-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Run tests under the appropriate 'extra' dependencies #1517
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is really good
tests/README.md
Outdated
@@ -185,6 +185,29 @@ For executing this test, first make sure you are in the correct environment as d | |||
pytest tests/unit/test_notebooks_python.py::test_sar_single_node_runs | |||
``` | |||
|
|||
### Test execution with tox |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One question, when we deprecate the ADO pipeline, are we going to use tox as the main test tool instead of pytest?
If that's the case, above in the text we have content showing the behavior with pytest:
pytest tests/smoke -m "smoke and not spark and gpu" --durations 0
will we still need this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was hoping people can use tox
as another option to run tests (especially when they need to debug a build issue in the pipeline), I want to keep the option here for people to use either pytest or tox. I myself runs pytest most of the time due to habit 😄. So I think we can keep these nice documentations on pytest.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm!
Codecov Report
@@ Coverage Diff @@
## staging #1517 +/- ##
============================================
- Coverage 74.23% 62.12% -12.11%
============================================
Files 84 84
Lines 8369 8397 +28
============================================
- Hits 6213 5217 -996
- Misses 2156 3180 +1024
Continue to review full report at Codecov.
|
Description
To improve developer experience working with this repo, we will begin migrating our DevOps pipelines into Github Action, leveraging popular DevOps tools like tox and flake8. This will be a sequence of updates to our DevOps infrastructures:
Propose and draft the CI pipeline in Github ActionSetup self-hosted machines to run our GPU workloadsCreate feature parity with the existing CI pipelines (pr-gates & nightly-build)In this PR:
Problem
Our recommender package has several extra dependencies to address the different compute/development environments:
recommender[spark,gpu,example,dev]
. We should run our tests against the specific extra dependency (currently, we always installrecommenders[all]
to run any ANY test markers)For example:
A user
pip install recommenders[gpu,examples,dev]
should be able to run recommender utilities tested bypytest -m "gpu and notebooks and not spark
Solution
New
tox-env
is introduced to the pipeline. One can map the extra dependencies with the tests type running the following command:tox -e {TOX_ENV} -- {PYTEST_PARAM}
where
For example:
tox -e cpu -- tests/unit -m "not notebook and not spark and not gpu
(runs the unit tests without extra dependency)tox -e gpu -- tests/unit -m "gpu and notebook"
(runs the gpu notebook tests)tox -e spark -- tests/unit -m "spark and notebook"
(runs the spark notebook tests)tox -e all -- tests/unit
(to run all of the unit tests)For more details, please see the changes in
tox.ini
.Related Issues
#1517
Checklist:
staging branch
and not tomain branch
.