Repository navigation
Compliance: a SKIPPED case is not an executed one, and not a fixed one - #24933
Merged
Merged
Conversation
The executed set I added in #24835 counted skips as executions, so the very confusion it was built to prevent survived in a second form. PHPUnit emits testStarted for a skipped case AND then testIgnored. The set was built from testStarted alone, so once --SKIPIF-- started being honoured (#24888) a case that became skipped left the failing set, stayed in the executed set, and read as FIXED. Measured on the first post-SKIPIF collect (runs 30472500277 / 30472527097 at 90c654c): failing fell 428 -> 370, while executed ROSE 8112 -> 8121 and exactly ZERO cases stopped executing. The 58 cases that stopped failing were skips wearing a fix. shard-compliance.sh writes <suite>-<i>-of-<N>.skipped from testIgnored, and .executed is now started MINUS ignored compliance-baseline.sh --diff splits FIXED / SKIPPED / DROPPED / NEWLY EXECUTED SKIPPED and DROPPED are kept apart because they are different problems: a skipped case declares itself inapplicable to this host/profile, an unyielded case was gated out by VMTest::providePHPTests(). Both are coverage losses, neither is a repair, and a skip guard that started firing wrongly would look identical to a correct one. The zero-executed guard deliberately still keys on testStarted: it means "the filter matched nothing", and a shard whose cases all legitimately skip is not that failure. Verified on a real mixed teamcity log (4 started, 2 skipped, 2 ran, 1 failed — the two skips correctly excluded from "ran"), and on synthetic fixtures for all four classifications. Degrades conservatively: with no .skipped files a skipped case falls into DROPPED, never into FIXED. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Jul 30, 2026
PurHur
added a commit
that referenced
this pull request
Jul 30, 2026
) The compiler gate has produced no master verdict at all. Measured over the 60 most recent runs of compiler-gate.yml: cancelled 31 in flight 29 completed 0 Every master push shared one concurrency group with cancel-in-progress, and master takes ~12 commits/hour against a gate that needs ~25 min — so each run was cancelled by the next merge before it could reach a conclusion. The gate appeared to be running constantly and never reported anything. That is the same failure as a check that reports without running, which this repo has now hit four times (#24888 dead SKIPIF, #24726 quarantine from two different commits, #24835 baseline that could not tell fixed from dropped, #24933 skips counted as executions). Cancelled-forever is absence wearing a busy spinner. Master runs are now keyed on the SHA, so each merge gets its own verdict. PR runs keep superseding each other on the ref, which is correct — once a branch is pushed again the previous tree is nobody's concern. Cost: master runs may overlap. With the paths filter (lib, ext, bin, patches, the sweep scripts) and ~56% of commits touching lowering, that is roughly 7 runs/hour at ~25 min each, so ~3 concurrent. GitHub queues the excess. Found while investigating why #24988 merged with a red gate. That failure was `aot-smoke: 7 passed, 1 failed` — `echo` hitting rc=124, a 900s COMPILE TIMEOUT under CI load, not a wrong result. Verified on current master (e5bf674, which contains that patch): aot-smoke 8 passed, 0 failed locally, including echo. So the red was infrastructure slowness, and master is intact — but nobody could have known that from the gate, because it never finishes. Co-authored-by: PurHur <tedyyyyy@gmail.com> 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
2 participants
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.
The executed set added in #24835 counted skips as executions, so the very confusion it was built to prevent survived in a second form.
The bug
PHPUnit emits
testStartedfor a skipped case and thentestIgnored. The set was built fromtestStartedalone. Once--SKIPIF--started being honoured (#24888), a case that became skipped left the failing set, stayed in the executed set, and read as FIXED.How it surfaced
The first post-SKIPIF collect — runs 30472500277 / 30472527097 at
90c654c46:a3f32662b)I predicted the ~19 SKIPIF false-failures would leave the executed set. They didn't — it went up. That mismatch is what exposed the defect; the 58 cases that stopped failing were skips wearing a fix.
Fix
SKIPPED and DROPPED are kept apart because they are different problems: a skipped case declares itself inapplicable to this host/profile; an unyielded case was gated out by
VMTest::providePHPTests(). Both are coverage losses, neither is a repair, and a skip guard that started firing wrongly would look identical to a correct one.The zero-executed guard deliberately still keys on
testStarted— it means "the filter matched nothing", and a shard whose cases all legitimately skip is not that failure.Verified
.skippedfiles present, a skipped case falls into DROPPED, never into FIXEDNote
This lands ahead of merge-base diffing (the agreed next structural change), because that will consume the same set logic and it needed to be correct first.
🤖 Generated with Claude Code