Repository navigation
ci(e2e): start the managed-SSH lane for every helper only its specs import - #30233
Merged
Merged
Conversation
Contributor
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 51 seconds. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
ELI5
Some test helper files are used only by the managed-SSH end-to-end specs. If one of those helpers changes, nothing else runs it, so the managed-SSH lane has to start. Five of them were missing from the hand-written list. This adds them, plus a test that finds such helpers from the import graph and fails when one is missing.
What Changed
Before: a change to any of these five helpers started no e2e lane at all:
orcad-convert-teardown.tsorcad-upgrade-retained-state.tsreserve-local-port.tsssh-remote-only-browser-fixture.tswindows-host-orcad-processes.tsAfter: these five join
MANAGED_SSH_HARNESSinpr-e2e-source-routing.mjs, so changing one starts the managed-SSH Docker lane.New
pr-e2e-managed-ssh-harness-route.test.mjs:ssh-orcad-*specs reach;It fails on main with exactly those five files.
The import walker moved out of the layout-oracle contract test into
config/scripts/e2e-test-import-closure.mjs, so both tests use one copy. It now normalizes paths to/, so the contract also holds on Windows. Before,path.relativereturned backslashes there and the walk found nothing.Why
A hand-kept regex can't keep up with new helpers. The import graph is the ground truth for what a helper change can break.
The contract covers only helpers that only managed-SSH specs use. A shared helper like
orca-app.tsis reached by every spec, and routing it here would start the 2×17 min lane for most e2e changes. The round-2 proposal also suggested requiring routes for every product file the specs import. I measured that and dropped it: those are mostly shared type and constant modules (store/types.ts,shared/constants.ts, …), which would turn the lane on for most PRs. The folder fallback route (#30210) covers the remote product code instead.Linked Issue
Round 2 SSH hardening, slice S1, the import-graph route contract (internal).
Visual Proof
N/A: CI routing only.
Testing
The new contract test fails on main and passes here.
These suites pass:
pr-e2e-route-contracts,ci-e2e-job-selection,pr-e2e-gate-contract,pr-unit-support-routing,pr-auxiliary-preflight-admissionandwsl-e2e-lane-contract.The full static-analysis job ran locally.
Automated tests added/updated
Review
Codex review (gpt-6.1-sol, high):
Agent skill upstream boundary
Notes
No product, wire or platform impact.
Checklist
N/Avisual proof with reason