Conversation
There was a problem hiding this comment.
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.
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.