Skip to content

docs(contributing): fix incorrect xdist reference in run_tests comment - #54956

Closed
waroffchange wants to merge 2 commits into
NousResearch:mainfrom
waroffchange:docs/fix-contributing-xdist-comment
Closed

docs(contributing): fix incorrect xdist reference in run_tests comment#54956
waroffchange wants to merge 2 commits into
NousResearch:mainfrom
waroffchange:docs/fix-contributing-xdist-comment

Conversation

@waroffchange

@waroffchange waroffchange commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Summary

The comment above \scripts/run_tests.sh\ in CONTRIBUTING.md described the runner as using \4 xdist workers:

\\�ash

Preferred — matches CI (hermetic env, 4 xdist workers); see AGENTS.md

scripts/run_tests.sh
\\

However, \scripts/run_tests.sh\ itself explicitly states:

Per-file isolation via \scripts/run_tests_parallel.py\ — each test file runs in its own freshly-spawned \python -m pytest \ subprocess. No xdist, no shared workers, no module-level leakage between files.

The runner uses a custom per-file subprocess parallel strategy, not pytest-xdist. Updated the comment to match the actual behavior.

Validation

Docs-only change. Verified by reading \scripts/run_tests.sh\ header and comparing against the corrected comment. No runtime code modified.

Checked open PRs for overlap: #44482 addresses \website/docs/developer-guide/contributing.md\ (the published docs site); this PR corrects the root \CONTRIBUTING.md\ file.

@alt-glitch alt-glitch added type/docs Documentation improvements comp/cli CLI entry point, hermes_cli/, setup wizard P3 Low — cosmetic, nice to have labels Jun 29, 2026
@teknium1 teknium1 mentioned this pull request Jul 14, 2026
Closed
teknium1
teknium1 reviewed Jul 15, 2026
View reviewed changes

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for correcting the stale runner description. The proposed per-file isolation wording matches the implementation in scripts/run_tests.sh:6-9 and scripts/run_tests_parallel.py:17-23.

Problems

  • The changed comment still says “see AGENTS.md,” but AGENTS.md:1281-1285 currently says the wrapper uses -n auto xdist workers. This leaves two directly linked contributor instructions in conflict.

Suggested changes

  • Update AGENTS.md:1281-1285 to describe the hermetic per-file subprocess runner, or remove the see AGENTS.md pointer from this comment until AGENTS.md is aligned.

Automated hermes-sweeper review.

Comment thread CONTRIBUTING.md
@@ -194,7 +194,7 @@ ln -sf "$(pwd)/venv/bin/hermes" ~/.local/bin/hermes
### Run tests

@teknium1 teknium1 Jul 15, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AGENTS.md:1281-1285 still describes this wrapper as using -n auto xdist workers, so this “see AGENTS.md” pointer leaves conflicting guidance. Please align that section too, or remove this pointer.

@teknium1 teknium1 added the sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users label Jul 15, 2026
@waroffchange

waroffchange commented Jul 15, 2026

Copy link
Copy Markdown
Contributor Author

Thanks — addressed the AGENTS.md conflict on this branch. AGENTS.md (Testing section) said the wrapper uses -n auto xdist workers + a subprocess-isolation plugin; per scripts/run_tests_parallel.py ("drop xdist entirely", one python -m pytest <file> per file) and scripts/run_tests.sh ("No xdist, no shared workers") that's stale. Updated it to describe the per-file subprocess isolation, so both contributor instructions now agree and the see AGENTS.md pointer stays valid.

@teknium1 teknium1 mentioned this pull request Jul 30, 2026
Merged
teknium1 pushed a commit that referenced this pull request Jul 30, 2026
@konsisumer @TutkuEroglu
@waroffchange @teknium1
docs: purge stale xdist/_enforce_test_timeout test-runner references …
8e1debd 
…repo-wide

The test runner moved to per-file subprocess isolation via
scripts/run_tests_parallel.py (hermetic `env -i`, worker count auto-scaled
from CPU count, FLAKY-retry policy) — no pytest-xdist, no SIGALRM per-test
timeout fixture. Docs still described the old runner in many places:

- AGENTS.md: "-n auto xdist workers, in-tree subprocess-isolation plugin"
  clause replaced with the current per-file-subprocess description; the
  `::test_x` single-test example now shows file + -k (runner is
  file-granular).
- CONTRIBUTING.md: "hermetic env, 4 xdist workers" comment corrected;
  `tests/conftest.py::_enforce_test_timeout` reference redirected to the
  win32 timeout-method shim in `tests/conftest.py::pytest_configure`.
- skills/autonomous-ai-agents/hermes-agent/references/contributor-guide.md
  and windows-quirks.md: same corrections (the bundled skill mirrors the
  contributor docs); Windows workaround no longer installs pytest-xdist
  or passes -n 0.
- website/docs + zh-Hans i18n mirrors: same fixes in adding-providers.md
  and the bundled-skill doc pages.
- skills/software-development/python-debugpy/SKILL.md (+ zh-Hans mirror):
  "-p no:xdist"/"-n 0" pdb advice rewritten for the captured per-file
  subprocess runner.
- skills/creative/comfyui/tests/README.md: parent-repo "-n auto by
  default" rationale updated to past tense.

Combined salvage of PR #38295 (konsisumer), PR #51354 (TutkuEroglu,
redirected to the current conftest truth and the relocated
references/contributor-guide.md), and PR #54956 (waroffchange).

Co-authored-by: TutkuEroglu <rrandqua@gmail.com>
Co-authored-by: waroffchange <116298975+waroffchange@users.noreply.github.com>
@teknium1 teknium1 mentioned this pull request Jul 30, 2026
Closed
@teknium1

teknium1 commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Merged via the round-4 closeout PR #74614 (commit 953ff9c, co-authored credit). The '4 xdist workers' claim in CONTRIBUTING.md is fixed (without the stray parenthesis) along with every other stale runner reference repo-wide, co-crediting #38295 (earliest) and #51354. Thanks @waroffchange!

@teknium1 teknium1 closed this Jul 30, 2026
SSC-ENG pushed a commit to SSC-Engineering/hermes-agent that referenced this pull request Jul 30, 2026
@konsisumer @TutkuEroglu
@waroffchange @SSC-ENG
docs: purge stale xdist/_enforce_test_timeout test-runner references …
cad8397 
…repo-wide

The test runner moved to per-file subprocess isolation via
scripts/run_tests_parallel.py (hermetic `env -i`, worker count auto-scaled
from CPU count, FLAKY-retry policy) — no pytest-xdist, no SIGALRM per-test
timeout fixture. Docs still described the old runner in many places:

- AGENTS.md: "-n auto xdist workers, in-tree subprocess-isolation plugin"
  clause replaced with the current per-file-subprocess description; the
  `::test_x` single-test example now shows file + -k (runner is
  file-granular).
- CONTRIBUTING.md: "hermetic env, 4 xdist workers" comment corrected;
  `tests/conftest.py::_enforce_test_timeout` reference redirected to the
  win32 timeout-method shim in `tests/conftest.py::pytest_configure`.
- skills/autonomous-ai-agents/hermes-agent/references/contributor-guide.md
  and windows-quirks.md: same corrections (the bundled skill mirrors the
  contributor docs); Windows workaround no longer installs pytest-xdist
  or passes -n 0.
- website/docs + zh-Hans i18n mirrors: same fixes in adding-providers.md
  and the bundled-skill doc pages.
- skills/software-development/python-debugpy/SKILL.md (+ zh-Hans mirror):
  "-p no:xdist"/"-n 0" pdb advice rewritten for the captured per-file
  subprocess runner.
- skills/creative/comfyui/tests/README.md: parent-repo "-n auto by
  default" rationale updated to past tense.

Combined salvage of PR NousResearch#38295 (konsisumer), PR NousResearch#51354 (TutkuEroglu,
redirected to the current conftest truth and the relocated
references/contributor-guide.md), and PR NousResearch#54956 (waroffchange).

Co-authored-by: TutkuEroglu <rrandqua@gmail.com>
Co-authored-by: waroffchange <116298975+waroffchange@users.noreply.github.com>
randlee pushed a commit to randlee/hermes-agent that referenced this pull request Aug 11, 2026
@konsisumer @TutkuEroglu
@waroffchange @teknium1
docs: purge stale xdist/_enforce_test_timeout test-runner references …
ab66de7 
…repo-wide

The test runner moved to per-file subprocess isolation via
scripts/run_tests_parallel.py (hermetic `env -i`, worker count auto-scaled
from CPU count, FLAKY-retry policy) — no pytest-xdist, no SIGALRM per-test
timeout fixture. Docs still described the old runner in many places:

- AGENTS.md: "-n auto xdist workers, in-tree subprocess-isolation plugin"
  clause replaced with the current per-file-subprocess description; the
  `::test_x` single-test example now shows file + -k (runner is
  file-granular).
- CONTRIBUTING.md: "hermetic env, 4 xdist workers" comment corrected;
  `tests/conftest.py::_enforce_test_timeout` reference redirected to the
  win32 timeout-method shim in `tests/conftest.py::pytest_configure`.
- skills/autonomous-ai-agents/hermes-agent/references/contributor-guide.md
  and windows-quirks.md: same corrections (the bundled skill mirrors the
  contributor docs); Windows workaround no longer installs pytest-xdist
  or passes -n 0.
- website/docs + zh-Hans i18n mirrors: same fixes in adding-providers.md
  and the bundled-skill doc pages.
- skills/software-development/python-debugpy/SKILL.md (+ zh-Hans mirror):
  "-p no:xdist"/"-n 0" pdb advice rewritten for the captured per-file
  subprocess runner.
- skills/creative/comfyui/tests/README.md: parent-repo "-n auto by
  default" rationale updated to past tense.

Combined salvage of PR NousResearch#38295 (konsisumer), PR NousResearch#51354 (TutkuEroglu,
redirected to the current conftest truth and the relocated
references/contributor-guide.md), and PR NousResearch#54956 (waroffchange).

Co-authored-by: TutkuEroglu <rrandqua@gmail.com>
Co-authored-by: waroffchange <116298975+waroffchange@users.noreply.github.com>
33hodl pushed a commit to 33hodl/hermes-agent that referenced this pull request Aug 12, 2026
@konsisumer @TutkuEroglu
@waroffchange @teknium1
docs: purge stale xdist/_enforce_test_timeout test-runner references …
fbb3aa0 
…repo-wide

The test runner moved to per-file subprocess isolation via
scripts/run_tests_parallel.py (hermetic `env -i`, worker count auto-scaled
from CPU count, FLAKY-retry policy) — no pytest-xdist, no SIGALRM per-test
timeout fixture. Docs still described the old runner in many places:

- AGENTS.md: "-n auto xdist workers, in-tree subprocess-isolation plugin"
  clause replaced with the current per-file-subprocess description; the
  `::test_x` single-test example now shows file + -k (runner is
  file-granular).
- CONTRIBUTING.md: "hermetic env, 4 xdist workers" comment corrected;
  `tests/conftest.py::_enforce_test_timeout` reference redirected to the
  win32 timeout-method shim in `tests/conftest.py::pytest_configure`.
- skills/autonomous-ai-agents/hermes-agent/references/contributor-guide.md
  and windows-quirks.md: same corrections (the bundled skill mirrors the
  contributor docs); Windows workaround no longer installs pytest-xdist
  or passes -n 0.
- website/docs + zh-Hans i18n mirrors: same fixes in adding-providers.md
  and the bundled-skill doc pages.
- skills/software-development/python-debugpy/SKILL.md (+ zh-Hans mirror):
  "-p no:xdist"/"-n 0" pdb advice rewritten for the captured per-file
  subprocess runner.
- skills/creative/comfyui/tests/README.md: parent-repo "-n auto by
  default" rationale updated to past tense.

Combined salvage of PR NousResearch#38295 (konsisumer), PR NousResearch#51354 (TutkuEroglu,
redirected to the current conftest truth and the relocated
references/contributor-guide.md), and PR NousResearch#54956 (waroffchange).

Co-authored-by: TutkuEroglu <rrandqua@gmail.com>
Co-authored-by: waroffchange <116298975+waroffchange@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@teknium1 teknium1 teknium1 left review comments

Assignees

No one assigned

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard P3 Low — cosmetic, nice to have sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users type/docs Documentation improvements

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@waroffchange @teknium1 @alt-glitch