Skip to content

The test-evidence gate is copied at five sites (198 lines) and policed by a 194-line extractor; the copies have already drifted three ways #247

Description

@monkopedia-coder

Code-health suggestion (proactive — staged for your review)

Category: duplication — five hand-maintained copies of one gate, and a 194-line harness whose only
job is to keep them in lockstep

Where: .github/workflows/{compiler-check,release,root-build,samples-minimal}.yml (five gate
sites) and .github/test-evidence-gate/verify.sh:129-322

The shape

The "assert the tests actually ran" gate exists five times, copied:

$ grep -c 'sum_attr() {' .github/workflows/*.yml
compiler-check.yml:1   release.yml:1   root-build.yml:2   samples-minimal.yml:1

# the five blocks, from the shared comment header through the second `fi`:
compiler-check.yml   :131-166   36 lines
release.yml          :101-139   39 lines
root-build.yml       :129-167   39 lines
root-build.yml       :246-284   39 lines
samples-minimal.yml  :118-162   45 lines
                              -----------
                                198 lines

The 16-line explanatory comment above each is byte-identical at all five sites (verified:
5 blocks, 1 distinct string after whitespace normalisation).

Because there are five copies, verify.sh cannot test the gate — it has to test five of them, in
place. That is why it carries an awk state machine that extracts each site's arithmetic and verdict
out of the YAML text, a run_block/at_sites/verdict_at/at_verdict driver, and three mutation
loops. Lines 129-322 — 194 of the file's 326 — exist only because the block is copied. The file
is right to do this (its own comment: "Proving a copy is not proving the gates"), but the cost is
real and it grows with each new gate site.

The copies have already drifted, three ways

This is the evidence that it is a defect rather than a taste preference — none of these was a
deliberate divergence in all five places at once:

axis state
delete-before-measure present at 4 of 5 sites. grep -n 'Clear stale test reports' .github/workflows/*.ymlcompiler-check:110, release:72, root-build:109, root-build:224. samples-minimal.yml has none — it runs nativeTest and then globs build/test-results/**/*.xml with nothing having cleared it. Harmless on a fresh hosted runner; it is exactly the guard the other four carry for the case where it is not fresh.
per-module vs summed release.yml:90-146 asserts per module; compiler-check.yml:121-166 sums across compiler/**, so compiler/gradle's 3 tests cover for compiler/plugin's 19 (measured: compiler/plugin/src/test = 19 @Test, compiler/gradle/src/test = 3). Already filed as #230 item 3 — cited here as evidence of drift, not re-filed. release.yml:78-83 documents the divergence in its own comment.
discovered vs hardcoded module list release.yml:94 discovers its modules (mods=(compiler/*/src/test)) specifically to avoid the #231 class. root-build.yml:150 and :267 hardcode for mod in krapper krapper_model feature-tests / featuregen cppfixture, so adding a module to the ./gradlew line without adding it to the loop leaves it unasserted.

Three divergences across five copies, and the newest copy (release.yml) is the one that got two of
the three right — which is the normal life cycle of a copied block, not a lapse by anyone.

Proposed: collapse the five into one composite action, e.g.
.github/actions/assert-tests-executed/action.yml, with inputs for the report glob and the module
discovery pattern, and have each job call it. Fold the delete-before-measure step into the same
action so it cannot be forgotten at a sixth site.

That also simplifies verify.sh rather than complicating it: with one implementation, the awk
extractor, the two block families and the three mutation loops collapse into sourcing the action's
own script and running it on the seven fixtures — and the thing under test then is the thing that
runs in CI, which is the property lines 92-107 argue for and currently have to reconstruct. Keep the
fixtures and every negative control; they are the valuable half.

Sequencing note: this touches the same five sites as #230 item 3, and doing that item inside the
composite action is strictly less work than doing it at one site and leaving four. Worth deciding
which lands first.

Why: 198 duplicated lines plus 194 lines of machinery to police them, guarding the repo's most
load-bearing CI invariant, with three measured divergences already. The current design detects drift;
this one removes the surface that drifts.

Effort: small refactor (the action) plus a larger simplification of verify.sh.

Refs #230, Refs #231, Refs #198, Refs #243.

Filed by the nightly code-health pass. To act on it: relabel agent-workable (it'll flow into triage→work). To decline: close it — it won't be suggested again.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    health-suggestionProactive code-health suggestion from the nightly analysis pass (staged for review)needs-decisionRequires user input on direction, scope, or design

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions