chore: parallelize testing in ci with xdist#3906
chore: parallelize testing in ci with xdist#3906bcmyguest wants to merge 6 commits intoUnidata:mainfrom
Conversation
|
Obviously something wonky going on with the coverage but as a POC I think this works (tests run, and they run much faster). I think we would need to use ^ did this |
bcmyguest
changed the title
Removed '--cov-branch' from default pytest arguments.
For that part of the issue, you could mark the plotting tests, run the non-plotting tests in parallel then the plotting tests serially. MetPy probably has enough non-plotting tests this would speed things up |
|
@DWesl it actually seemed to be working alright - |
There was a problem hiding this comment.
Pull request overview
This PR adds parallel test execution to the CI pipeline using pytest-xdist to speed up testing. It updates the test dependencies to include pytest-xdist and pytest-cov, modifies the CI test runner to use parallel execution with coverage reporting, and updates the contributing documentation with instructions for running tests in parallel locally.
Changes:
- Added pytest-xdist and pytest-cov as test dependencies in pyproject.toml and pinned versions in ci-dev/test_requirements.txt
- Modified CI test runner to use
-n autofor parallel execution and integrated pytest-cov for coverage collection - Updated CONTRIBUTING.md with examples of running tests in parallel using pytest-xdist
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| pyproject.toml | Added pytest-xdist and pytest-cov to test dependencies |
| ci-dev/test_requirements.txt | Pinned specific versions for pytest-cov (6.2.1) and pytest-xdist (3.8.0) |
| CONTRIBUTING.md | Added documentation for running tests in parallel with xdist examples |
| .github/actions/run-tests/action.yml | Updated default pytest arguments to include parallel execution and coverage reporting, removed manual coverage commands |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -30,10 +30,8 @@ runs: | |||
| # By running coverage in "parallel" mode and "combining", we can clean up the path names | |||
There was a problem hiding this comment.
The comment about running coverage in "parallel" mode and "combining" is now outdated since coverage is being run via pytest-cov plugin rather than directly. This comment should be updated or removed to reflect the new approach.
| # By running coverage in "parallel" mode and "combining", we can clean up the path names | |
| # Coverage is collected via pytest-cov as configured in the pytest-args input |
| @@ -1,5 +1,8 @@ | |||
|
|
|||
There was a problem hiding this comment.
This blank line at the top of the file is unnecessary and should be removed for consistency with the previous version of the file.
| "pytest-xdist", | ||
| "pytest-cov", |
There was a problem hiding this comment.
When using pytest-xdist with pytest-cov, it's recommended to configure coverage for parallel execution. While pytest-cov should handle this automatically in recent versions, explicitly setting 'parallel = true' in the [run] section of .coveragerc (or adding [tool.coverage.run] with parallel = true in pyproject.toml) ensures accurate coverage collection across parallel workers. This is particularly important for consistent coverage reporting.
| description: 'Additional arguments to pass to pytest' | ||
| required: false | ||
| default: '--mpl -W error::metpy.deprecation.MetpyDeprecationWarning' | ||
| default: '--mpl -W error::metpy.deprecation.MetpyDeprecationWarning -n auto --cov --cov-report=xml' |
There was a problem hiding this comment.
The PR description mentions concerns about matplotlib not being thread-safe, which could make tests flaky with parallel execution. Consider adding pytest-xdist configuration to use 'loadscope' or 'loadfile' distribution mode instead of the default 'load' mode to reduce potential race conditions. This can be configured in pyproject.toml under [tool.pytest.ini_options] by adding 'addopts = "-n auto --dist=loadfile"' or similar. The '--dist=loadfile' mode ensures all tests in a file run on the same worker, which can help with matplotlib's thread safety issues.
2 participants
Description Of Changes
From an idea in here: #3901
Attempting to turn on parallel testing in CI to speed up testing. Locally testing it seemed to work but who knows once it gets in the wild.
Definite DISadvantage here is that it screws up pluggy, but that's only really relevant when running locally anyways and it's not enabled by default. Another possible issue is that matplotlib is not thread safe, so I am not sure how tests will interact with that, it could end up being more flaky than it's worth.
Checklist