Skip to content

ci(bench): run every benchmark harness module in the CI Gate (#1679) - #1681

Merged
DecisionNerd merged 6 commits into
mainfrom
test/1679-bench-harness-ci
Sep 30, 2026
Merged

DecisionNerd merged 6 commits into
mainfrom
test/1679-bench-harness-ci

Conversation

@DecisionNerd

@DecisionNerd DecisionNerd commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Closes #1679

What changes

  • All 41 benchmarks/tests modules now run in the CI Gate. Before this PR, 8 ran, from an explicit list in Rust Tests. benchmarks/tests is now a package, and a new Benchmark Harness Tests job runs unittest discover -v -s tests -t . -p 'test_*.py' from benchmarks/. A new module runs without being wired anywhere.

    • The job follows rust_tests, because the four GDC suites build Rust runners against graphforge-api.
    • It runs in parallel with Rust Tests, the CI Gate's longest job, instead of adding to it.
    • Every benchmarks/ path except benchmarks/algorithms/ already falls through the classifier to enable_all, and no harness module reads benchmarks/algorithms/. New classifier assertions pin this for test, harness and schema edits.
  • Policy (scripts/ci/test-ci-storage-policy.py) requires four things, and negative fixtures cover each one:

    • exactly one lane runs unittest from benchmarks/, and it runs the exact discovery command, not a module list;
    • that lane is gated on rust_tests;
    • that lane feeds CI Gate;
    • every test_*.py under benchmarks/tests can be reached through packages.

    The dependency-cache inventory moves from 11 to 12 for the new job's registry cache.

  • test_smoke: test_checked_in_fixtures_are_discoverable failed on g500-ladder-qualification.schema.json. docs: move evidence out of the tree, prune orphan dev docs, give ADRs a lifecycle (#1625) #1628 moved that schema from docs/ into benchmarks/schemas/, next to its only validator (progressive_storage_qualification.py). The move itself was right, and moving the file back would break docs: stop committing per-issue evidence to the docs tree, prune unreferenced development docs, and give ADRs a lifecycle #1625's docs-tree rule. Every other document in benchmarks/schemas/ has a top-level graphforge-*-schema/N identity, and discover_fixtures requires one, so this PR adds "schema": "graphforge-g500-ladder-qualification-schema/4". JSON Schema 2020-12 treats an unknown keyword as an annotation, so validation doesn't change, and test_progressive_storage_qualification and test_native_ladder_controller still pass. make -C benchmarks smoke failed for the same reason and now passes.

  • test_measurement_tools: this was the only pytest module in a unittest-only locked environment. It is rewritten as unittest subtests covering the same 16 cases, with no new dependency. The conversion exposed a second bug that pytest had been masking. The stub sudo runs cat >/dev/null, and run-f1.sh calls it without redirecting stdin, so the fio case hung whenever the process inherited an open stdin. Every tool invocation now passes stdin=subprocess.DEVNULL.

  • make -C benchmarks smoke-python now uses the same package discovery. Its old -s tests form loaded modules twice, as test_x and as tests.test_x.

Evidence

Exact CI command, run from a fresh worktree with no benchmarks/target, CARGO_BUILD_JOBS=4, ext4 TMPDIR:
PYTHONPATH=harness uv run --locked python -m unittest discover -v -s tests -t . -p 'test_*.py'
Result: Ran 528 tests in 263.539s, OK, 41 modules, 0 skipped. The 528 is the issue's 453, plus 69 from test_tck_perf (#1680), 1 new in test_benchexec_authority, and 5 more from test_measurement_tools (6 methods instead of 1 import error). The per-module sum matches the discovery total, so nothing is loaded twice.

Mutations:

  • a new benchmarks/tests/test_zz_mutation_probe.py with a failing test was discovered: Ran 529, FAILED (failures=1), rc=1;
  • benchmarks/tests/probe/test_probe.py in a directory without __init__.py fails the policy;
  • removing tests/__init__.py fails both the policy and discovery;
  • going back to a module list fails the policy;
  • dropping the CI Gate edge fails the policy;
  • removing the watchdog restore in run-perf.sh fails all 6 test_measurement_tools watchdog subtests.

Gates: make pre-push-fast, python3 scripts/ci/test-ci-storage-policy.py, bash scripts/ci/test-classify-changes.sh, make gate-registry-check, scripts/check-workflows.sh, bash scripts/ci/test-require-gates.sh, python3 scripts/ci/benchmark-measurement-policy.py and python3 scripts/ci/test-benchmark-measurement-policy.py all pass.

🤖 Generated with Claude Code


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

DecisionNerd and others added 4 commits September 30, 2026 16:04
test_measurement_tools imported pytest, which the benchmark project's locked
environment does not provide, so it could not run beside the other harness
modules. Express the same 16 cases as unittest subtests.

Close stdin for every tool invocation: the stub sudo reads stdin, and the fio
case hung whenever the runner inherited an open one. pytest had been masking
that by closing stdin itself.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
#1628 moved g500-ladder-qualification.schema.json from docs/ into
benchmarks/schemas/, beside its only validator. Every other document there
carries a top-level graphforge-*-schema/N identity, and discover_fixtures
requires one, so the benchmark smoke and test_smoke have failed since the
move. Nothing ran them. Add the identity rather than relax the rule; JSON
Schema 2020-12 treats the extra keyword as an annotation, so validation is
unchanged.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Only eight of the 41 benchmarks/tests modules ran in CI, from an explicit
list in the Rust Tests job. Make benchmarks/tests a package and run the whole
tree with unittest discovery in a Benchmark Harness Tests job, so a new module
runs without being wired anywhere. The GDC suites build Rust runners against
graphforge-api, so the job follows the Rust test gate. It runs beside Rust
Tests instead of lengthening the CI Gate's critical path.

The CI storage policy now requires that lane to run discovery from benchmarks/,
follow the Rust test gate, and feed CI Gate, and it requires every test_*.py to
be reachable through packages. Negative fixtures cover each of these. The
benchmark smoke target uses the same package discovery, which stops loading
modules twice under two names.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Harness, test and schema edits under benchmarks/ enable rust_tests, which the
Benchmark Harness Tests lane follows. Now that benchmarks/tests is a package,
ruff sorts its imports as first-party, so regroup them.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository: CurateLabs/graphforge/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 958e6fc5-3428-494b-a9e5-ec251c187137

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.

Warning

Billing warning: we have not been able to collect payment for this subscription for more than 72 hours. Please update the payment method or pay any pending invoices in Billing to avoid service interruption.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added ci-cd CI/CD configuration changes tooling Developer tooling and automation release:none No release note or version impact labels Sep 30, 2026
@blacksmith-sh

This comment has been minimized.

…1679)

test_progressive_host_run defaulted its scratch parent to
/home/ubuntu/graphforge-ladder-test, a path on one development host. On a CI
runner, setUpModule and 13 native ladder controller tests failed with
PermissionError the first time the suite ran there. Default to a directory
under the temporary root instead. scripts/test_environment.py checks that root
is native storage and exports it in CI. GF_LADDER_TEST_ROOT still overrides,
and the retained evidence root stays untouched.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added the documentation Improvements or additions to documentation label Sep 30, 2026
…ch root (#1679)

test_native_ladder_controller imported WORK_PARENT from
test_progressive_host_run but not the setUpModule that creates it, so it passed
only when that module ran first or a previous run had left the directory
behind. Discovery runs it first, and on a fresh CI runner 13 tests failed with
FileNotFoundError. Share both module fixtures, which also extends the
top-level leak check to this module.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@DecisionNerd
DecisionNerd added this pull request to the merge queue Sep 30, 2026
Merged via the queue into main with commit 2a9a637 Sep 30, 2026
27 checks passed
@DecisionNerd
DecisionNerd deleted the test/1679-bench-harness-ci branch September 30, 2026 17:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci-cd CI/CD configuration changes documentation Improvements or additions to documentation release:none No release note or version impact tooling Developer tooling and automation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test(bench): 33 of 40 benchmark harness test modules never run in CI; two are already broken

1 participant