ci: split PR baseline and cron supplemental e2e coverage#1248
Merged
Conversation
Restore the schedule-gate pattern (matching node_version above) so PRs run only `test`, while `test:commonjs` and `test:no-isolate` keep running on the daily cron and workflow_dispatch. Cuts PR e2e job count from 10 → 4 (2 os × 2 node × 1 script). Full matrix on schedule unchanged: 2 × 3 × 3 − 2 excludes = 16 jobs.
Rsdoctor Bundle Diff AnalysisFound 12 projects in monorepo, 0 projects with changes. 📊 Quick Summary
Generated by Rsdoctor GitHub Action |
PR (and push) runs `test` only on mac/windows × Node 20/24 (4 jobs). Scheduled cron runs `test:commonjs` and `test:no-isolate` only on mac/windows × Node 20/22/24 (12 jobs). The two triggers are now complementary with no overlap. Drop the Node 20 + test:no-isolate exclude; that combination now only runs on the scheduled cron where occasional flakes are acceptable.
fi3ework
changed the title
Detect release PRs by `startsWith(pull_request.title, 'release:')` and
expand the e2e matrix to the union of the PR baseline and the cron
supplemental suites:
PR/push mac/win × [20,24] × [test] 4 jobs
schedule mac/win × [20,22,24] × [commonjs, no-isolate] 12 jobs
release PR mac/win × [20,22,24] × [test, commonjs, no-isolate] 16 jobs
(exclude test × Node 22)
Node 22 only carries the supplemental scripts on release PRs because
`test` already runs on Node 20/24 in the standard PR matrix.
Expose `is_release_pr` from the existing `prepare` job and consume it via `needs.prepare.outputs.is_release_pr == 'true'` from the three matrix expressions. Aligns with the established `prepare.outputs.changed` pattern and removes three copies of the same `startsWith(...)` literal.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4e33da7a57
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Add a trigger × axis table comment and fold each conditional expression to multi-line via YAML folded scalars (`>-`) so each branch lands on its own line. No semantic change — `prettier --check` and YAML round-trip both confirm identical expressions.
Restore the warning that worker reuse on Node 20 can hit upstream vm native crashes for `test:no-isolate`. The combination was previously fully excluded; under the new schedule / release-PR scopes it runs by design, so the WHY is documented inline instead.
Collaborator
|
What if the full test is triggered when the main pushed? I'm afraid we won't receive the notification in time during the scheduled task. |
Per @9aoy's feedback on #1248: scheduled cron failures don't surface in a timely way. Move the supplemental matrix (Node 22 + commonjs + no-isolate) onto the push-to-main trigger so any regression that slips past the PR baseline is caught immediately on merge. - Drop the schedule trigger and its conditional branches in lint/ut/e2e. - Merge push and release-PR into a single "full" matrix arm; PR baseline arm unchanged.
- Add prepare.outputs.is_full_run alongside is_release_pr; collapse the three matrix axes from a repeated `(event == 'push' || is_release_pr)` predicate to a single output reference. - Drop the always() + explicit success-result checks on e2e.if. Those were only needed while lint had `if: github.event_name != 'schedule'` (which could leave lint skipped); now that lint always runs, default `needs:` behavior already gates e2e on lint/ut success. - Trim the matrix-comment block: drop the PR-narration paragraph and keep only the table and the Node 20 + no-isolate flakiness note.
2 tasks
Absorbed inline into is_full_run; comment documents the trigger semantics.
9aoy
approved these changes
May 13, 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.
Summary
Background
The e2e matrix currently fires too many jobs on every PR — all three scripts (
test,test:commonjs,test:no-isolate) run across every PR node combination, costing 10 jobs even for trivial changes. The intended shape is a fast baseline on every PR, the full supplemental sweep on every merge tomain, and the same full sweep as a final gate on release PRs.Per @9aoy's feedback: a nightly cron's failure surfaces too late. Folding the supplemental sweep into the push-to-
maintrigger keeps the PR matrix tight while still catching any regression the baseline misses, with timely notification on merge.Implementation
Drive all three axes off
github.event_nameand a newprepare.outputs.is_release_pr(matched viastartsWith(github.event.pull_request.title, 'release:')):testonly) — down from 10.test× Node 22 excluded so the baseline footprint matches the PR matrix).schedule:trigger and its conditional branches inlint/ut/e2eare removed; push-to-mainis now the single supplemental trigger.test:no-isolatecan hit upstream vm native crashes under worker reuse; tolerated on push / release-PR runs.User Impact
None — internal CI cost reduction and notification-latency improvement. PR e2e job count: 10 → 4. Supplemental regression coverage moves from "next nightly cron" to "immediately on merge to main".
Validation
Scoped to the workflow YAML file — no runtime/test source touched. Full build/test/e2e suites skipped as not applicable.
prettier --checkpasses on the file.Checklist