test(ci): make the slow-tier partition a partition, and check it (#672) - #707
Draft
willhea wants to merge 1 commit into
Draft
test(ci): make the slow-tier partition a partition, and check it (#672)#707willhea wants to merge 1 commit into
willhea wants to merge 1 commit into
Conversation
The slow tier is divided between CI jobs by listing every module by name in ci.yml, and that partition is restated a second time as CI_SLOW_MODULES in tests/conftest.py. Neither copy was derived from anything and neither was checked, so both drifted on their own schedule. tests/test_manifest_report_fixtures.py was listed in two jobs and ran eight times per CI run. History resolves which listing was intended: bfa5d60 added it to the general slow list on 2026-08-04, deliberately and with reasoning, as part of fixing a gate that ran nowhere; f1767df added it again to corpus-gates two hours later, inside a commit whose subject and body are entirely about resolving a merge conflict and which never mentions CI. The corpus-gates entry is removed as the accidental one. The module's 91 tests still run, once, via remaining-slow. Two guards, one per direction the existing coverage gate cannot see. test_every_slow_module_is_run_by_a_workflow catches a module that runs NOWHERE; it is satisfied by a single naming and cannot count them, which is why nothing reported the duplicate. test_no_module_is_run_by_two_ci_invocations counts, and test_every_ci_slow_module_is_named_by_a_slow_step checks the second copy's stated membership rule, which was previously a claim about ci.yml that nothing verified. NOT done: deriving CI_SLOW_MODULES from ci.yml, the other remedy the issue floats. That tuple drives the #288 skip ceiling rather than the CI partition, and seventeen of the modules CI runs are absent from it, so deriving it would switch skip ceilings on for seventeen modules at once. That is a change to what the suite enforces, and it is the decision #610 exists to make. The claim is therefore verified in the safe direction only; the converse stays #610's to settle, and the guard's docstring says so. Verification, per the issue's own instruction to confirm a new guard reddens on the tree as it stands rather than on a synthetic mutation: - The duplicate guard fails on unmodified develop, naming test_manifest_report_fixtures.py and both jobs. Removing the accidental entry turns it green. A real red-green, not a planted one. - The CI_SLOW_MODULES guard was mutated two ways. Deleting a module from a slow step reddens it, but reddens the pre-existing coverage gate too, so that mutation does not show it earning its place. Changing a step's marker from `-m slow` to `-m "not slow"`, leaving the module named, reddens this guard ALONE (1 failed, 22 passed): the module is still named by a workflow, so the older gate stays green. The two overlap partially; neither is redundant. - _slow_marker_selected is pinned directly on the four marker spellings these workflows use, because the trap it exists for cannot be reached live: the fast step is the one spelled `not slow and not browser` and it names no modules, so a helper reading the bare word would misclassify it with nothing going red. Simplifying it to `"slow" in expression` fails exactly that case. Fast suite 2015 passed / 4 skipped / 15 xfailed, ruff clean. CONTRIBUTING.md's "three steps", which the issue also flags, is corrected on a separate docs branch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addresses the partition half of #672, via option 3: keep the parallelism and the matrix, remove only the hand-maintenance. The matrix half is deliberately untouched.
The problem
The slow tier is divided between CI jobs by listing every module by name in
ci.yml, and that partition is restated a second time asCI_SLOW_MODULESintests/conftest.py. Neither copy was derived from anything, and neither was checked.It had already drifted.
tests/test_manifest_report_fixtures.pywas listed in two jobs, so it ran once per leg of each: eight times per CI run.The existing guard,
test_every_slow_module_is_run_by_a_workflow, is a good one and is one-directional by design. It catches a module that runs nowhere. It is satisfied by a single naming and cannot count them, which is why nothing in the repository reported the duplicate.Which listing was the accident
Resolved from history rather than guessed, because the module's name fits more than one job and #672 notes that nothing states the grouping rule.
bfa5d606remaining-slow), deliberately, as part of fixing a gate that ran nowhere. The commit argues the placement at length.f1767df9corpus-gates, inside a commit titled "Fix conflict-resolution regressions from merging develop into #520" whose body never mentions CI.So the
corpus-gatesentry is collateral from a merge resolution. That one line is removed. The module's 91 tests still run, once, viaremaining-slow, confirmed by running them.I had initially reasoned from the module's name that
corpus-gateswas the better home. History says otherwise, which is why it was worth checking.The two guards
test_no_module_is_run_by_two_ci_invocations— counts invocations per module._modules_run_by_workflowsreturns a set, which is what makes it structurally unable to see a duplicate.test_every_ci_slow_module_is_named_by_a_slow_step— checks the second copy's own stated membership rule ("named by a slow CI step"), previously a claim aboutci.ymlthat nothing verified. It matters because the tuple is read bypytest_runtest_logreportin every session, so a stale entry keeps applying a skip ceiling locally for a gate CI stopped running.What this deliberately does not do
The issue also floats deriving
CI_SLOW_MODULESfromci.yml. I'd advise against it and haven't done it.That tuple drives the #288 skip ceiling, not the CI partition. Seventeen of the forty modules CI runs are absent from it, so deriving it would switch skip ceilings on for seventeen modules at once. That is a change to what the suite enforces, not a de-duplication, and it is precisely the decision #610 exists to make. The issue itself frames this one as "about the duplication that makes the gap easy to create, not the gap itself."
So the claim is asserted in the safe direction only: everything the tuple names must really be named by a slow step. The converse stays open and stays #610's to settle. The guard's docstring says so, so a later reader does not tighten it to an equality and make that decision silently.
Verification
Per the issue's instruction to confirm a new guard reddens on the tree as it stands, since the tree already contained the defect.
The duplicate guard is a real red-green, not a planted one. On unmodified
developit fails, naming the module and both jobs:Removing the accidental entry turns it green.
The
CI_SLOW_MODULESguard was mutated twice, because the first mutation did not prove it earns its place.-m slow→-m "not slow", module still namedThe two guards overlap partially and neither is redundant.
_slow_marker_selectedis pinned directly, on the four marker spellings these workflows use, because the trap it exists for cannot be reached through the live workflows: the fast step is the one spellednot slow and not browser, and it names no modules, so a helper reading the bare word would classify it as a slow step with nothing going red. The wrongness would surface the first time someone added a module argument to that step. Simplifying the helper to"slow" in expressionfails exactly that case and no other.Gates. Fast suite 2015 passed / 4 skipped / 15 xfailed;
ruff checkandruff format --checkclean.Caveat: measured in a worktree lacking the gitignored fetched corpus, which reports 4 skips where the main checkout reports 1. None relate to this change.
Notes for the reviewer
CONTRIBUTING.md's "three steps", which Most checks on a pull request restate a verdict already given, and the hand-maintained split that produces them has drifted #672 also flags, is corrected on a separate docs branch (docs: correct three onboarding claims about running the tests #705) rather than here.tests/test_ci_workflow.py, as does fix(ci): install and audit the committed dependency graph (#678) #700. Both edit the module docstring's property count, so whichever merges second will need a trivial conflict resolution there — the two additions are independent and both belong.