Allow expected-failures entries to name a single check#406
Merged
Conversation
A baseline entry names a scenario, but a scenario is many checks:
server-stateless is 29. Excusing the one or two a spec change breaks
means baselining the whole scenario, which stops enforcing the rest.
An entry may now be '<scenario>:<check-id>', in which case every failing
check in that scenario is judged on its own. Bare scenario entries are
unchanged, and YAML parses '- a:b' as a plain string, so every existing
baseline keeps working untouched.
loadExpectedFailures now owns the entry grammar and returns parsed
BaselineEntry values, so evaluateBaseline only matches and cannot throw.
Rejecting a bad entry and interpreting a good one no longer live in
separate layers.
A check id matches all of its occurrences, since ids repeat within a run;
they collapse to one verdict, most-severe first. Absent and SKIPPED are
tolerated, matching how the runner already treats them as green.
Move collapseDuplicateChecks to src/checks/collapse.ts with its tests:
expected-failures.ts needs the reducer and cannot import dpop.ts's auth
server stack. No behavior change.
Reject two configs that previously coerced to an entry matching nothing:
a mapping ('- scenario: check-id', with a space) and an empty list item.
commit: |
pcarleton
approved these changes
Jul 20, 2026
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.
Closes #404.
Motivation and Context
An entry names a scenario, but a scenario is many checks.
server-statelessis 29. Spec PR #3002 (picked up here in #403) breaks 2 of them, so today the only way to stay green is:That excuses all 29. The other 27 stop being enforced until the entry comes out. typescript-sdk#2513 has exactly this entry, with a comment listing by hand which checks are really affected.
Now you can name them:
27 checks stay enforced. Anything else in
server-statelessthat breaks is still an unexpected failure.Both forms work, and they mix:
The check id is the column the runner already prints, so you copy it out of the failing run.
Two configs are rejected rather than silently misread:
How Has This Been Tested?
Against the v2 typescript-sdk server,
--scenario server-stateless --spec-version 2026-07-28:Naming both exits 0. Naming one exits 1 on the other:
To show what the coarse entry costs, I broke an unrelated fixture the scenario depends on:
- server-statelessexits 0 and hides the regression, the per-check baseline exits 1 and names it.Plus unit tests for stale-per-check, repeated ids, and both rejected configs.
Breaking Changes
None for baseline files — YAML parses
- a:bas a plain string, so existing baselines and the action input are untouched.loadExpectedFailuresnow returns parsedBaselineEntry[]rather thanstring[]. Internal only: the package ships a CLI (bin, nomain/exports), andsrc/index.tsneeds no change.Types of changes
Checklist
Additional context
A check id matches every occurrence of that id, since ids repeat within a run. Occurrences collapse to one verdict, most-severe first, so baselining a repeated id excuses all of them — coarser than ideal, still far narrower than the scenario. Absent and SKIPPED checks are tolerated, matching how the runner already treats them as green.
loadExpectedFailuresowns the entry grammar andevaluateBaselineonly matches, so rejecting a bad entry and interpreting a good one don't live in separate layers.collapseDuplicateChecksmoves fromscenarios/client/auth/dpop.tstosrc/checks/collapse.tswith its tests —expected-failures.tsneeds the reducer and can't pull in dpop.ts's auth server stack. No behavior change.