chore(ci): run the performance gate only when a change can move it - #34
Closed
chh-ay wants to merge 1 commit into
Closed
chore(ci): run the performance gate only when a change can move it#34chh-ay wants to merge 1 commit into
chh-ay wants to merge 1 commit into
Conversation
The matched zero-regression benchmark occupies the single self-hosted runner for up to 90 minutes and every other pull request queues behind it, yet it ran for changeset-only and docs-only diffs. Classify performance-relevant paths the way `docs_required` already classifies documentation, and gate the job on the result. Unknown, mixed, and empty change sets keep taking the expensive lane: a false negative ships a regression, a false positive costs one benchmark run. `unit-coverage` installed Chromium, Firefox and WebKit with system dependencies, but its only browser consumer is the `bench:verify` render smoke, which launches Chromium alone. Also close two holes found while gating: - The job now refuses fork pull requests. It is the only non-ephemeral runner in the workflow, and a persistent host exposes its filesystem, caches and network position regardless of a read-only token. Required CI fails such a pull request rather than accepting the skip, so a fork cannot alter `bench/results/render-baseline.json` and be validated against its own baseline after merge. - `preflight` joins the job's `needs`, because the `needs` context exposes direct dependencies only and the condition would otherwise evaluate against an undefined context and skip on every run. Contract tests cover the trigger lists, the dependency requirement for any job reading another job's outputs, the fork rejection, and the absence of `pull_request_target`.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Owner
Author
|
Superseded by #35. The repository has zero registered runners for the self-hosted label, so scoping the job still leaves performance-sensitive PRs — including this workflow change itself — permanently queued. #35 removes the unreliable required job while preserving the matched protocol as a fail-closed local command. |
36 tasks
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.
What
The matched zero-regression benchmark occupies the single self-hosted runner for up to 90 minutes and every other pull request queues behind it — yet it ran for changeset-only and docs-only diffs. This gates it on whether the diff can move a measurement, using the same path-classification mechanism
docs_requiredalready uses.While gating it, two holes turned up and are closed here.
Changes
Scope the benchmark to relevant diffs.
requiresPerformanceRun()joinsrequiresDocumentationBuild()inscripts/ci-paths.ts,preflightemitsperf_required, andcontrolled-performanceis gated on it. Unknown, mixed, and empty change sets keep taking the expensive lane: a false negative ships a regression, a false positive costs one benchmark run.bench/results/render-baseline.jsondeliberately requires a run — it is the reference the gate compares against, so changing it must re-measure.Install one browser instead of three.
unit-coverageinstalled Chromium, Firefox and WebKit with system dependencies. Its only browser consumer is thebench:verifyrender smoke, which runsrender-driver.ts --smokeand launches Chromium alone.Refuse fork code on the self-hosted runner.
controlled-performanceis the only non-ephemeral runner in the workflow. A persistent host exposes its filesystem, caches and network position regardless of a read-only token, so the job now requires a same-repository event. Required CI fails a fork pull request that touches performance-sensitive paths rather than accepting the skip — deferring to a post-merge run would let a fork alter the baseline and be validated against it after the merge already happened. The message tells a maintainer to replay the commits on a same-repository branch.Fix a latent always-skip.
preflightjoins the job'sneeds. Theneedscontext exposes direct dependencies only, so the new condition would otherwise evaluate against an undefined context and skip on every run.Contract tests
pull_request_targetif:readsneeds.<job>.outputsmust list<job>inneeds— the generic form of the always-skip bug, which also protectsdocs-buildscripts/ci-paths.test.tsjoinstest:toolingVerification
Local, on this branch:
Not in scope
The benchmark's thresholds, sample counts, timeout, and runner are untouched. This changes when the gate runs, never what it asserts.