Skip to content

ci: compat-matrix safe-slice hardening — zero-test guard, neutral soft-fail check, honest summary - #3366

Merged
bpamiri merged 2 commits into
developfrom
peter/3302-compat-matrix-safe-slice
Aug 5, 2026
Merged

ci: compat-matrix safe-slice hardening — zero-test guard, neutral soft-fail check, honest summary#3366
bpamiri merged 2 commits into
developfrom
peter/3302-compat-matrix-safe-slice

Conversation

@bpamiri

@bpamiri bpamiri commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

What / Why

Safe slice of the #3302 CI-integrity hardening: three workflow-plumbing fixes in
.github/workflows/compat-matrix.yml that are shippable before the burn-down gate clears.
None of these can turn the weekly banner permanently red, so they need no debt gate.

The core flip — removing continue-on-error: true from the tests job — is NOT in this PR.
It remains explicitly gated on PR #3365 merging plus one fully-green workflow_dispatch on
develop. The rustcfml job is untouched (intentionally informational lane, baseline-diff pass
criteria).

The three slices

  1. Per-leg zero-test guard (run-tests step). After the JUnit conversion, the step now reads
    totalSpecs from the leg's result file. If the leg returned HTTP 200/417 but reported fewer
    than 4,000 testcases (every engine runs the same ~4,700-spec core suite), it emits ::error::
    and sets the leg fail flag — so a compile-wiped leg (one bad CFC zeroing the whole directory
    compile, e.g. the adobe2025 tests="0" mode) fails loudly instead of rendering as a pass.
    SOFT_FAIL_DBS (oracle) is respected: a tripped guard on a soft-fail DB warns without failing
    the job.
  2. Neutral aggregate check (publish-results job). fail_on: nothing on
    EnricoMi/publish-unit-test-result-action, so oracle soft-fail debt stops pinning a red
    "Wheels Test Results" check to whatever SHA the matrix was dispatched on (verified live: red
    on innocent PR head SHAs, marking them UNSTABLE). Annotations, PR comments, JSON output, and
    the per-leg JUnit artifacts are all unchanged — only the check conclusion is neutralized. Leg
    pass/fail gating stays where it belongs, in the tests job's OVERALL_STATUS exit.
  3. Summary-grid honesty (test-matrix-summary job). Zero-test legs render as
    :warning: N tests instead of :white_check_mark:; failures on soft-fail DBs render as
    :warning: N instead of :x: N; the Oracle column header is annotated (soft-fail) with a
    footnote pointing at ci: Oracle datasource broken across all engines — Invalid URL, missing DBMS_LOCK, constraint cleanup #2663.

Acceptance criteria (from the assessment)

  • (i) A workflow_dispatch with an intentionally failing hard-DB leg concludes failure at run
    level — pending the gated continue-on-error flip; this PR does not change run-level
    propagation.
  • (ii) A leg reporting totalSpecs=0 fails the engine job — covered by slice 1.
  • (iii) A dispatch on a SHA whose only failures are SOFT_FAIL_DBS leaves no failing check-run on
    that SHA — covered by slice 2 (re-verify via gh api commits/<sha>/check-runs).
  • (iv) The next scheduled Sunday run on clean develop is fully green.

Validation

  • python3 -c "import yaml; yaml.safe_load(...)" — YAML parses clean.
  • actionlint — finding set byte-identical to develop baseline (21 pre-existing informational
    shellcheck notes, zero new findings; only inner-script line offsets shifted).
  • Bash/python cell + guard logic smoke-tested locally against four result shapes: healthy
    (4712 specs / 0 fail → pass/checkmark), compile-wiped (0 specs → guard trips / :warning: 0 tests), failing (6 fail → :x: 6, :warning: 6 on soft-fail DBs), unparseable JSON
    (→ guard trips / :warning:).
  • Full validation requires 2–3 workflow_dispatch runs (~20 min each); the maintainer session
    will trigger those after review. No test spec files and no changelog fragment — CI-only change.

Residual scope

Removing continue-on-error: true from the tests job (line 18) is the remaining #3302 work and
stays gated on PR #3365 merging plus one green full-matrix dispatch on develop.

Refs #3302

🤖 Generated with Claude Code

Three workflow-plumbing fixes in compat-matrix.yml, shippable before the
continue-on-error flip (which stays gated on PR #3365 merging plus one
green dispatch on develop):

- run-tests: per-leg zero-test guard — an HTTP 200/417 leg reporting
  totalSpecs below 4000 (suite runs ~4,700) now emits ::error:: and sets
  the leg fail flag, so a compile-wiped leg fails loudly instead of
  rendering as a pass. SOFT_FAIL_DBS is respected.
- publish-results: fail_on: nothing on publish-unit-test-result-action,
  so oracle soft-fail debt stops pinning a red aggregate 'Wheels Test
  Results' check to innocent dispatch SHAs. Annotations, PR comments and
  artifacts are unchanged; leg gating stays in the tests job.
- test-matrix-summary: zero-test legs render as ':warning: N tests'
  instead of a checkmark, soft-fail DB failures render as :warning:, and
  the Oracle column is annotated as soft-fail with a footnote.

The rustcfml job is untouched (intentionally informational).

Refs #3302

Signed-off-by: Peter Amiri <petera@pai.com>

@wheels-bot wheels-bot Bot 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.

Wheels Bot — Reviewer

TL;DR — This PR ships the shippable slice of the #3302 CI-integrity work: a per-leg zero-test guard, a neutral aggregate check (fail_on: nothing), and an honest summary grid, all confined to .github/workflows/compat-matrix.yml. The gating continue-on-error flip is correctly left out and gated on #3365. I traced the shell/Python logic and found no correctness, cross-engine, or security defects. Verdict: comment — a few minor, non-blocking observations, nothing that should hold the merge.

Correctness

The three slices hold up under scrutiny:

  • Zero-test guard (lines 442–464): TOTAL_SPECS defaults to -1, is only recomputed for HTTP 200/417, and the guard [ "$TOTAL_SPECS" -lt "$MIN_SPECS" ] is always fed a numeric value (Python print(-1) on parse failure, plus the || echo "-1" belt-and-braces). A transient failure (HTTP 000/5xx) leaves SPECS_OK=true and falls through to the existing HTTP_CODE != 200 fail path — no regression. The pass condition stays [ "$HTTP_CODE" = "200" ] && [ "$SPECS_OK" = true ], so 417 remains a failure exactly as before.
  • Summary split (lines 709–710): FAIL="${STATS% *}" / SPECS="${STATS#* }" correctly split the two-token Python output for every shape ("0 4712", "-1 -1", "6 4712", "0 0"), and the elif ladder orders -1 (missing) → 0+low-specs (compile-wiped) → soft-fail → hard fail sensibly.
  • Neutral check (line 646): fail_on: nothing is a valid EnricoMi/publish-unit-test-result-action value; it only changes the check conclusion, leaving annotations/comments/JSON/artifacts intact, and gating stays in the tests job's OVERALL_STATUS.

One thing to keep visible (already acknowledged in the PR body under acceptance criterion (ii)): because the tests job still carries continue-on-error: true (line 18, intentionally out of scope here), a tripped guard fails the step but not the run. So criterion (ii) — "a leg reporting totalSpecs=0 fails the engine job" — is only fully realized once the gated flip lands with #3365. The PR is honest about this; noting it so the follow-up isn't forgotten.

Conventions

Minor, non-blocking:

  • MIN_SPECS=4000 is a hard floor asserting every engine × db leg runs ≥4000 of the ~4,700 core specs. The buffer looks safe today, but if any engine ever legitimately skips a large block of specs this would false-trip a healthy leg. The comment at lines 442–446 ("Revisit the floor if per-DB spec subsets ever ship") already flags the assumption, and the value is duplicated with a "keep in sync" note at line 690 — acceptable given YAML has no cross-job variable sharing.
  • The new Python blocks interpolate '$RESULT_FILE' (line 453) and '$FILE' (line 704) directly into the -c source, whereas the pre-existing JUnit block (lines 380–383) passes the path via os.environ instead. Both paths are built from controlled engine/db names so there's no injection risk, but matching the os.environ style would keep the file internally consistent.
  • The ::error:: annotation at line 463 also fires for soft-fail (oracle) legs when a 417/low-spec result trips the guard. Since oracle is non-blocking, an error-level annotation on it is cosmetically a little misleading, though harmless (workflow-command annotations never set job status). Consider gating the ::error:: emission on !IS_SOFT_FAIL if you want the annotation severity to match the gating.

Cross-engine

N/A — this change is entirely GitHub Actions YAML plus embedded Bash/Python; no CFML touched, so the Lucee/Adobe/BoxLang invariants don't apply.

Tests

No spec files, correctly. This is a CI-workflow change with no runtime CFML surface, so there is no TDD obligation. The PR documents local smoke-testing against four result shapes (healthy / compile-wiped / failing / unparseable), which is the right validation level for shell-in-YAML.

Docs

No changelog fragment is required — changelog.d/ fragments are for user-facing fix/feat changes, and a CI workflow tweak is not user-facing. Correct call.

Commits

Single commit ci: compat-matrix safe-slice hardening for #3302 and PR title ci: compat-matrix safe-slice hardening — zero-test guard, neutral soft-fail check, honest summary both conform to commitlint.config.js: valid type ci, subject not ALL-CAPS, header under the 100-char limit (~97, counting the em dash as one char).

Security

No concerns. No secrets in the diff; the interpolated Python paths are derived from the fixed engine/db matrix values, not from untrusted input.

Review follow-up for the #3302 safe slice: the per-engine step summary
still rendered a compile-wiped leg (0 failures, 0 testcases) as a pass
while the run-tests guard in the same job failed it with ::error::.
Read totalSpecs alongside the failure count and render sub-floor legs
as ':warning: N tests (zero-test guard)', mirroring the matrix grid.

Refs #3302

Signed-off-by: Peter Amiri <petera@pai.com>

@wheels-bot wheels-bot Bot 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.

Wheels Bot — Reviewer

TL;DR — This PR ships the shippable slice of the #3302 CI-integrity work: a per-leg zero-test guard, a neutral aggregate check (fail_on: nothing), and an honest summary grid — all in .github/workflows/compat-matrix.yml, with the load-bearing continue-on-error flip explicitly deferred. I traced the guard/propagation and shell/python plumbing and found it correct; only cosmetic nits remain. Verdict: comment (nothing blocking; runtime behavior still awaits the author's pending workflow_dispatch validation, so I'm not approving as a courtesy).

Correctness

No blocking findings. Confirmed the load-bearing paths:

  • Guard propagates to job failure. A tripped guard sets DB_STATUS="fail" and, for a non-soft-fail DB, OVERALL_STATUS=1, which exit 1s the step (compat-matrix.yml:476-481, :503-506). Oracle warns without gating — matches the PR body's claim.
  • totalSpecs is a real field, not a guess. The pre-existing JUnit conversion already reads it from the same $RESULT_FILE: tests=str(int(d.get('totalSpecs', 0))) (compat-matrix.yml:421). So the guard keys off a field the endpoint is already known to emit — no risk of every leg silently reporting 0 and tripping the floor.
  • Two-token parse is sound. print(int(fail+error), int(specs)) yields "6 4712"; FAIL="${STATS% *}" / SPECS="${STATS#* }" split correctly, and the -1 -1 fallback (compat-matrix.yml:714, :535) splits the same way. Unparseable JSON → -1, and -1 -lt 4000 trips the guard — the fail-safe direction.
  • Branch ordering is safe. In all three summary blocks the FAIL = -1 (error) arm is evaluated before the FAIL = 0 (guard) arm, so an unparseable result can't be misrendered as a low-spec warning.

Conventions

  • Nit (compat-matrix.yml:542): echo "..." >> "$GITHUB_STEP_SUMMARY" quotes the redirect target while every surrounding line leaves it bare (>> $GITHUB_STEP_SUMMARY). Harmless (the var has no spaces), but worth matching the local style for consistency.

Tests

No spec files, correctly — CI workflow YAML isn't exercised by the core suite. The PR body documents local smoke-testing of the four result shapes (healthy / compile-wiped / failing / unparseable) and notes full validation needs 2–3 workflow_dispatch runs. That deferred live validation is the reason for a comment rather than approve; the static logic itself is clean.

Docs

  • MIN_SPECS=4000 is now duplicated across four sites (:447, :518, :696, and the footnote at :739), each with a "keep in sync" comment. Acceptable for GitHub Actions YAML (no shared-env mechanism across jobs), and the comments are the right mitigation — flagging only so a future editor updates all four. The 4000 floor against a ~4,700-spec suite leaves ~700 headroom; the inline comment already calls out "Revisit the floor if per-DB spec subsets ever ship," which is the correct caveat.

Commits

Both commits conform to commitlint.config.js: ci: compat-matrix safe-slice hardening for #3302 and ci: align per-engine summary with the zero-test guard — valid ci type, headers well under 100 chars, not ALL-CAPS. ci is correctly used as the type (not a scope).

Nice work — the guard is fail-safe in the right direction and the summary honesty changes are a genuine integrity improvement. Ship after the pending dispatch runs confirm the live shapes.

@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Wheels Test Results

     32 files    9 856 suites   21m 39s ⏱️
131 789 tests 131 319 ✅ 397 💤 38 ❌ 35 🔥
133 721 runs  133 251 ✅ 397 💤 38 ❌ 35 🔥

For more details on these failures and errors, see this check.

Results for commit 195a465.

@bpamiri
bpamiri merged commit c5f4558 into develop Aug 5, 2026
10 of 15 checks passed
@bpamiri
bpamiri deleted the peter/3302-compat-matrix-safe-slice branch August 5, 2026 02:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant