-
Notifications
You must be signed in to change notification settings - Fork 0
test: Run tests in parallel #7
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
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.
Performed full review of b147318...e5b6d59
Analysis
- Forced parallel test execution ("-n auto") may cause excessive resource contention, particularly problematic on lightweight development machines or when tests spin up Docker containers
- No configuration mechanism to control/limit parallelism levels based on available system resources or environment needs
- Adding pytest-xdist as a first-class dependency unnecessarily increases the install footprint for end-users who don't need multiprocessing capabilities
- The multiprocessing approach lacks safeguards to prevent resource exhaustion in CI environments
Tip
Help
Slash Commands:
/review- Request a full code review/review latest- Review only changes since the last review/describe- Generate PR description. This will update the PR body or issue comment depending on your configuration/help- Get help with Mesa commands and configuration options
0 files reviewed | 1 comments | Edit Agent Settings • Read Docs
| dependencies = [ | ||
| "cloudpickle>=3.1.1", | ||
| "pytest>=9.0.2", | ||
| "pytest-xdist>=3.5.0", |
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.
Adding pytest-xdist to the main dependencies list means every consumer of the plugin now installs xdist/execnet even though the library code never imports or interacts with xdist—it’s only used here to speed up this repository’s own test run. That extra dependency surface can bloat installs and introduce compatibility issues for end-users who don’t need parallel pytest workers. Could we keep xdist in the dev/test dependency group (and still invoke it from CI with uv run --group dev pytest -n auto), or make it an optional extra instead of a hard requirement?
Prompt for Agent
Task: Address review feedback left on GitHub.
Repository: mesa-dot-dev/pytest-in-docker#7
File: pyproject.toml#L28
Action: Open this file location in your editor, inspect the highlighted code, and resolve the issue described below.
Feedback:
Adding `pytest-xdist` to the main `dependencies` list means every consumer of the plugin now installs xdist/execnet even though the library code never imports or interacts with xdist—it’s only used here to speed up this repository’s own test run. That extra dependency surface can bloat installs and introduce compatibility issues for end-users who don’t need parallel pytest workers. Could we keep xdist in the dev/test dependency group (and still invoke it from CI with `uv run --group dev pytest -n auto`), or make it an optional extra instead of a hard requirement?
Mesa DescriptionThis PR configures the test suite to run in parallel, which will speed up test execution for both local development and CI builds. By leveraging the Changes
Description generated by Mesa. Update settings |
No description provided.