ome / omero-test-infra

Test infrastructure for decoupled repositories
BSD 2-Clause "Simplified" License
3 stars 14 forks source link

Drop setup.py test form to run tests #60

Closed sbesson closed 7 months ago

sbesson commented 2 years ago

See https://github.com/ome/omero-demo-cleanup/commit/886c01d6ea58d6a74b76040883e8682aade780a3#r60744714 for the initial discussion

Probably most relevant is the recommendation from Pytest to avoid using setup.py test - see https://docs.pytest.org/en/latest/explanation/goodpractices.html#do-not-run-via-setuptools.

We still use this semantics in the omero-test-integration

https://github.com/ome/omero-test-infra/blob/d03e9429a89c118db8220bdc47066c20532e6c08/app-build#L20 https://github.com/ome/omero-test-infra/blob/d03e9429a89c118db8220bdc47066c20532e6c08/scripts-build#L11 https://github.com/ome/omero-test-infra/blob/d03e9429a89c118db8220bdc47066c20532e6c08/cli-build#L17

In addition to complying with the recommendations, dropping the setuptools form has the advantage of removing the need for the custom pytest command class copied and pasted across all plugins - https://github.com/search?q=org%3Aome+%22%28test_command%29%22&type=code.

As a proof of concept, https://github.com/ome/omero-demo-cleanup/blob/44482aa7b7200677ccf16899a0b47cc8c9d8f936/.omeroci/cli-build#L17-L18 uses the direct form pytest -sv test after setting ICE_CONFIG. Probably the biggest thing that is being lost is the installation of test dependencies like mox3 (currently via test_requires). I could imagine several solutions ranging from requirements-{dev,test}.txt to specifying some test requirements via extra_requires. Do we have a feeling for the most appropriate way to capture this?

sbesson commented 8 months ago

The upgrade of this testing stack to Rocky Linux 9 in #66 seems to have caused some unwanted regression related to the logging in the standard output.

Dropping the usage of setup.py test and calling pytest directly seems to restore compatibility with the previous behavior - see https://github.com/ome/omero-metadata/pull/85 for an example.

On the question above, note also that mox3 is being phased out from all test requirements since support for Python 3.11 was introduced. Are there some minimal steps/checks that would give us confidence in updating cli-build, app-build and scripts-build in this repository ?

joshmoore commented 8 months ago

Looking at https://github.com/ome/omero-metadata/pull/85/files, is the/another question whether or not we can start calling pytest by default from the original cli-build without needing to override?

sbesson commented 7 months ago

Discussed earlier today with @joshmoore @jburel @pwalczysko @khaledk2 @dominikl. No objection to looking into the replacement of setup.py test. Alongside the ongoing changes to omero-test-infra, next actions are: 1- open a PR in this repository dropping setup.py test in favor of pytest 2- start reviewing downstream repositories consuming this including one example of CLI plugin, script and Web apps to consume this branch and drop the custom Pytest(test_command) from setup.py