Set up OMERO server test infrastructure for tests - #112
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #112 +/- ##
=======================================
Coverage 36.21% 36.21%
=======================================
Files 14 14
Lines 947 947
=======================================
Hits 343 343
Misses 604 604 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
I think in theory https://github.com/nektos/act is supposed to let you test actions locally? I've never tried it though. |
|
Based on the hints from Josh and Will, I think we could use something like this: but then in the run: use Still not sure it's easier to have it in it's own section or as part of the rest of the tests. |
I've had mixed results with The design of omero-test-infra is intended to make (local) testing a two liner: Where PHASE is the type of thing you want to happen with your code base. Do you want it installed in the OMERO.web container? in the OMERO.server container? Or somewhere else? There are then callbacks that runs the pytests within the right container (or externally to the container if necessary) and gives feedback. |
Without the -d the compose call never finishes and the pytest workflow stops in the OMERO set up step. I had just taken the instructions from the omero-test-infra
Not sure about that, tbh. A big part of the test imho would be the OMERO browser widget, the ROI uploader widget (the interactive functionality in general), all of which would require both Qt and a remote OMERO instance. |
hehe, I was thinking about the ROI uploader specifically -- the widget is just a button that executes something. That something uses OMERO and can be tested independently of the button. Tests of the button can test that the button calls what it's supposed to, but don't need to actually execute anything. |
|
Yosh, by the time my train has passed through the tunnels and my internet is back up I find the commits already reviewed ^^ Thanks a lot for looking over it but please don't feel pressured to invest much time in my WIP at this stage :) |
Co-authored-by: Peter Sobolewski <76622105+psobolewskiPhD@users.noreply.github.com>
|
Hehe, I saw failed CI so my detective sense started tingling. |
|
@jo-mueller I started looking at this and I think it's very close! |
59135ed to
b94763b
Compare
b94763b to
5b1a577
Compare
|
@jo-mueller Note: I've not done anything locally, just dry-code and CI. So we may need to follow with the contributor guide on testing. That may be Edit: there is no real rush on this PR, we should finish the stuff we want for the next release, do the release, and then merge this and then over time build out tests +/- ai code assist. |
| # ensure the ports are bound correctly | ||
| # see also: https://github.com/ome/omero-test-infra/issues/77 | ||
| echo 'OMERO_SERVER_SSL=4064:' >> .omero/.env | ||
| echo 'OMERO_WEB_PORT=4080:' >> .omero/.env |
There was a problem hiding this comment.
I don't really get why we need to specify this and didn't have much to go on other than the issue, but without echoing the env vars, the ports are unreachable.
There was a problem hiding this comment.
The ports are by default randomly generated with the docker-compose set up in order to avoid (local) collisions.
|
|
||
| - name: Test with OMERO server | ||
| if: runner.os == 'Linux' | ||
| run: pytest ./tests/server --color=yes --cov=napari_omero --cov-report=xml |
There was a problem hiding this comment.
I think it's simpler to have server tests and local tests -- we may end up not even running the server ones on every linux case.
|
I kinda wonder if we should do the OMERO server tests in a separate CI job? 🤔 |
|
@jo-mueller I made the change I suggested above: putting the server tests in their own job. Plus some cleanups. |
Relevant for #95
This PR will expand the tests of the plugins by adding a testing OMERO instance to the test CI. WIP since trial and error more or less happens on github (not sure if it's possible to do that locally).