Skip to content

fix(tests): forward pytest flags without requiring -- separator - #42195

Closed
liuhao1024 wants to merge 1 commit into
NousResearch:mainfrom
liuhao1024:fix/run-tests-pytest-passthrough
Closed

fix(tests): forward pytest flags without requiring -- separator#42195
liuhao1024 wants to merge 1 commit into
NousResearch:mainfrom
liuhao1024:fix/run-tests-pytest-passthrough

Conversation

@liuhao1024

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes scripts/run_tests.sh so that pytest flags (e.g. -q, -k, --tb=long) passed after test paths work without requiring a literal -- separator. Previously, scripts/run_tests.sh tests/foo.py -q would fail with an argparse error.

Related Issue

Fixes #42189

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • scripts/run_tests_parallel.py: Use parse_known_args instead of parse_args so unrecognized pytest flags are captured and forwarded to each per-file pytest invocation
  • scripts/run_tests.sh: Update usage comment to show that -- is now optional
  • tests/scripts/test_run_tests_parallel.py: Add 6 tests verifying argument parsing for various invocation styles (with/without separator, multiple flags, mixed runner+pytest flags)

How to Test

  1. Run scripts/run_tests.sh tests/test_run_tests_parallel.py -q — should succeed (previously failed with argparse error)
  2. Run scripts/run_tests.sh tests/test_run_tests_parallel.py -- -q — should still work (backward compat)
  3. Run scripts/run_tests.sh -j 2 tests/test_run_tests_parallel.py -k "test_" --tb=short — runner's -j parsed, pytest flags forwarded
  4. Run python -m pytest tests/scripts/test_run_tests_parallel.py -v — all 6 tests pass

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

Code Intelligence

⚠️ GitNexus unavailable — grep-based fallback used.

  • Checked: scripts/run_tests_parallel.py (argparse logic), scripts/run_tests.sh (usage docs)
  • Blast radius: LOW — test runner argument parsing only, no production code affected
  • Related patterns: parse_known_args is the standard argparse approach for mixed known/unknown flags

Use parse_known_args instead of parse_args in run_tests_parallel.py so
that pytest flags (e.g. -q, -k, --tb=long) passed after test paths
without a literal '--' separator are captured as unrecognized args and
forwarded to each per-file pytest invocation.

Closes NousResearch#42189
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused test-runner fix. This is already implemented on current main by the stronger merged fix in #54008.

  • Automated hermes-sweeper review verified scripts/run_tests_parallel.py:670-732 routes bare pytest flags while preserving values for flags such as -k, and retains explicit -- passthrough.
  • tests/test_run_tests_parallel.py:228-279 covers bare -q, value-taking -k, explicit --, and positional discovery roots against the real runner.
  • The implementation merged as 2523917680191b4375ceb1457658fe924ab1c73e (fix(tests): bare pytest flags pass through run_tests.sh without a '--' separator (#54008)) and is contained in release tag v2026.7.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P3 Low — cosmetic, nice to have sweeper:implemented-on-main Sweeper: behavior already present on current main type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: scripts/run_tests.sh rejects pytest flags passed without --

3 participants