Fix failures after nvidia-nat-eval isolation#1615
Fix failures after nvidia-nat-eval isolation#1615rapids-bot[bot] merged 2 commits intoNVIDIA:developfrom
nvidia-nat-eval isolation#1615Conversation
Signed-off-by: Will Killian <wkillian@nvidia.com>
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes. 💤 Files selected but had no reviewable changes (1)
You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThese changes introduce per-project virtual environment handling in the test runner, replacing shared VENV_DIR usage with individual .venv directories per project. A new CLI argument (extra_flags) is added to propagate custom pytest flags through the execution pipeline, and pre-commit configuration is extended with uv-lock entries for two additional packages. Changes
Sequence Diagram(s)sequenceDiagram
actor CLI
participant main as main()
participant make_env as make_env()
participant uv as uv tools
participant pytest as pytest
participant cleanup as cleanup
CLI->>main: run_tests.py with project_dir, extra_flags
loop For each project
main->>make_env: project_dir
make_env->>make_env: Set VIRTUAL_ENV, UV_PROJECT_ENVIRONMENT
make_env->>CLI: Return env dict
main->>uv: uv sync (with env)
uv-->>main: Sync complete
alt Tests exist
main->>pytest: uv run pytest + coverage + extra_flags
pytest-->>main: Test results
else No tests
main->>main: Exit early
end
main->>cleanup: Remove project .venv
cleanup-->>main: Cleanup complete
end
main-->>CLI: Return exit code
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
ci/scripts/run_tests.py (1)
274-274: Document the--separator requirement in theextra_flagshelp text.With
nargs="*", argparse will not consume values starting with-as positional arguments — they are interpreted as unrecognized options and the call will fail. Most useful pytest flags (e.g.-v,-x,--tb=short) start with-, so callers must use--to separate them. The current help string doesn't mention this.📝 Proposed fix
- parser.add_argument("extra_flags", nargs="*", default=[], help="Extra flags to pass to pytest") + parser.add_argument("extra_flags", + nargs="*", + default=[], + help="Extra flags to pass to pytest (use '--' to separate from script flags, " + "e.g. run_tests.py --project foo -- -v --tb=short)")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ci/scripts/run_tests.py` at line 274, Update the argparse help for the positional "extra_flags" to mention that flags starting with "-" must be passed after a "--" separator (because nargs="*" won't capture option-like tokens); edit the parser.add_argument call that defines extra_flags (nargs="*", default=[], help="...") to include a concise note like "If passing pytest flags that start with '-', put them after '--' (e.g. -- -v -x)" so callers know to use the separator.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ci/scripts/run_tests.py`:
- Line 175: Replace the explicit str() call inside the f-string used when
appending the coverage argument: in the cmd.append call that currently builds
f"--cov={str(source_dir)}", use the f-string conversion flag instead (i.e.,
f"--cov={source_dir!s}") so Ruff RUF010 is satisfied while preserving the same
string conversion behavior; update the cmd.append expression where source_dir is
referenced.
---
Nitpick comments:
In `@ci/scripts/run_tests.py`:
- Line 274: Update the argparse help for the positional "extra_flags" to mention
that flags starting with "-" must be passed after a "--" separator (because
nargs="*" won't capture option-like tokens); edit the parser.add_argument call
that defines extra_flags (nargs="*", default=[], help="...") to include a
concise note like "If passing pytest flags that start with '-', put them after
'--' (e.g. -- -v -x)" so callers know to use the separator.
Signed-off-by: Will Killian <wkillian@nvidia.com>
|
/merge |
Description
uv.lockfilesexamples/notebooks(already underuv.lock)uv.lockprecommit check forevalandfastmcpdaskrather than unconditionally.venvper project/example with automatic cleanupprediction_trieback tonvidia-nat-coreCloses
By Submitting this PR I confirm:
Summary by CodeRabbit