Repository navigation
Fail the runtime boundary PR check on counts main would reject - #15208
Merged
Merged
Conversation
…increase PR mode only failed for a Deno.* API with a src/deno_ral replacement; every other increase was a warning on a green job, relying on a maintainer to read the job summary and push the baseline bump before merging. Twice a PR merged without it and the absolute push check turned main red (#15134, #15202). An increase the PR's own tools/runtime-boundary-baseline.json does not accept now fails the run. The per-line findings stay warnings, since the code is fine as written, and one ::error plus the job summary tell the contributor there is nothing for them to change: a maintainer updates the branch with main, runs --update-baseline --accept-increase and pushes the baseline to the PR. Once accepted, the findings become notices and the run passes, so a green PR no longer implies a red main. A PR label clearing the check was rejected: the merged tree would still carry the old baseline and main would fail anyway. Replayed #15202 (55a313c against its parent): exit 1 with the four tests/unit/zip.test.ts calls; with the baseline from 4bebcb4 applied, exit 0 and "Accepted in this PR: 870 -> 874".
The PR-mode failure used "accepted by this PR", which needs the PR to raise
the baseline above the base revision's value. When main sits below its
baseline (a count fell without --update-baseline), a PR adding a call and
running --accept-increase writes a value at or under the old one, so the
run stayed red although the push check on main would pass.
A finding now fails only when the count exceeds the PR's own baseline, the
same comparison the push check makes after merge. A count within it that
the PR did not raise is reported as a notice ("within the baseline, which
main sits below"); "accepted by this PR" keeps its meaning for counts the
PR raised.
The check counts Deno. occurrences in comments too, so the comment added with the headroom test raised the tests scope from 874 to 875 and failed the push check against the committed baseline.
Collaborator
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
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.
Follow-up to #15153 and #15183.
#15202 added four
Deno.*calls intests/unit/zip.test.ts. Its Runtime Boundary run was green, with warnings saying "a maintainer accepts the new count before merging" (run). It merged without the baseline update, and the push check onmainfailed from 55a313c (run) until 4bebcb4 raisedtests870 → 874.In pull request mode only a
Deno.*API thatsrc/deno_ralreplaces is an error. Every other increase is a warning and the job exits 0, so the maintainer step depends on someone reading the summary of a green check.Fix
A PR run now fails when a count is above the PR's own
tools/runtime-boundary-baseline.json, the comparison the push check onmainmakes after merge.A green PR no longer turns
mainred.The remedy is unchanged and stays with the maintainer: update the branch with
main, run--update-baseline --accept-increase, push the baseline to the PR.Per-line findings stay warnings, as the code is fine as written. They, one
::errorand the job summary tell the contributor there is nothing to change in the PR and that the check stays red until a maintainer accepts the count.A count the PR did not raise but that fits under its baseline (
mainbelow its baseline because nobody lowered it) is now a notice and does not fail, asmainwill not fail on it either.#15183 kept it a warning. With a failing check that would leave a PR red that
--accept-increasecannot fix, since it writes a value at or under the old baseline.A label to clear the check was considered and rejected: the merged tree would still carry the old baseline, and
mainwould fail the same way.Replaying #15202 (55a313c against its parent) with this branch exits 1 with the four
zip.test.tswarnings. With the baseline from 4bebcb4 applied it exits 0 and liststests870 → 874 under "Accepted in this PR".The PR-mode test from #15183 now checks the exit status: its unaccepted case exited 0 before the fix.
dev-docs/runtime-migration/README.mdrecords the decision for the PR 02c plan, and_TEMPLATE.mdtells plans that raise a count to run--accept-increasein their own PR.Checklist
I have (if applicable):
AI-assisted PR
Note: autonomous AI agents submitting PRs without human oversight are not permitted — see the Code of Conduct.