fix: update run_affected_tests.py to use -n 8#1014
Conversation
The pre-push hook script had 'auto' hardcoded as the xdist worker count, bypassing the pyproject.toml addopts change from #1013.
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Snapshot WarningsEnsure that dependencies are being submitted on PR branches. Re-running this action after a short time may resolve the issue. See the documentation for more information and troubleshooting advice. Scanned FilesNone |
There was a problem hiding this comment.
Pull request overview
This PR updates the local pre-push test runner script to align its pytest-xdist worker count with the repository’s configured default parallelism, preventing overly aggressive worker spawning during git push.
Changes:
- Update
scripts/run_affected_tests.pyto run unit tests with-n 8instead of-n auto.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Code Review
This pull request modifies the run_affected_tests.py script to change the pytest worker count from 'auto' to a hardcoded value of 8. The review feedback suggests removing the '-n' flag and its associated value entirely to allow pytest to use the centralized configuration defined in pyproject.toml, thereby avoiding configuration duplication and ensuring that future updates to the worker count are automatically respected.
| "unit", | ||
| "-n", | ||
| "auto", | ||
| "8", |
There was a problem hiding this comment.
This hardcoded value still overrides the pyproject.toml configuration mentioned in the PR description. To avoid configuration duplication and ensure that future updates to the worker count in pyproject.toml are automatically respected, consider removing both the "-n" flag and this value from the command list. This would allow pytest to fall back to the centralized addopts settings as intended by #1013, and also prevents similar issues with grep-based searches in the future. Note: Since I can only comment on modified lines, I am anchoring this to line 160, but the suggestion applies to line 159 as well.
Summary
The pre-push hook script
scripts/run_affected_tests.pyhad"auto"hardcoded as the xdist worker count (line 160), bypassing thepyproject.tomladdopts change from #1013.This was missed in #1013 because the value was split across two Python list elements (
"-n","auto") rather than a single string"-n auto", so the grep search didn't catch it.Changes
scripts/run_affected_tests.py: Change worker count from"auto"to"8"Why
Follow-up to #1013. Without this fix,
git pushstill triggers 32 pytest workers via the pre-push hook, causing the same crashes and slowness that #1013 aimed to fix.