Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Drop setup.py test form to run tests #60

Closed
sbesson opened this issue Nov 24, 2021 · 3 comments · Fixed by #68
Closed

Drop setup.py test form to run tests #60

sbesson opened this issue Nov 24, 2021 · 3 comments · Fixed by #68
Assignees

Comments

@sbesson
Copy link
Member

sbesson commented Nov 24, 2021

See ome/omero-demo-cleanup@886c01d#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

python $DIR/setup.py test

python setup.py test -t test -i ${OMERO_DIST}/etc/ice.config -v

python setup.py test -t test -i ${OMERO_DIST}/etc/ice.config -vs

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
Copy link
Member Author

sbesson commented Jan 22, 2024

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 ome/omero-metadata#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
Copy link
Member

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 sbesson self-assigned this Jan 23, 2024
@sbesson
Copy link
Member Author

sbesson commented Jan 23, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants