From 25c3ac400b6a36dbfccd23b4813e25537b2ea39d Mon Sep 17 00:00:00 2001 From: OrcaWin Date: Sun, 27 Sep 2026 00:28:02 -0700 Subject: [PATCH] ci: overlap shell setup, localization extraction, and mobile route preparation (#23368) Co-authored-by: m4air --- .github/workflows/pr.yml | 54 +++++++++++-------- .../ci-background-step-barriers.test.mjs | 34 ++++++++++-- .../mobile-web-app-route-snapshot.test.mjs | 42 ++++++++++++++- .../scripts/pr-workflow-parallelism.test.mjs | 6 ++- config/scripts/run-mobile-web-app-checks.mjs | 44 ++++++++++++--- 5 files changed, 144 insertions(+), 36 deletions(-) diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 60ea206cc73..03fc47bbcb3 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -245,6 +245,23 @@ jobs: - name: Enforce runtime Electron-import ratchet run: pnpm run check:runtime-electron-ratchet + # Why: extraction writes sorted evidence to an isolated temporary path, + # so feature PRs need one normalized AST pass rather than a three-OS matrix. + - name: Verify localization extraction + id: localization-extraction + background: true + env: + BASE_SHA: ${{ github.event.pull_request.base.sha }} + HEAD_SHA: ${{ github.event.pull_request.head.sha }} + run: | + # Detection failures run the full check; renames retain the removed input path. + if git diff --name-only --no-renames -z --merge-base "$BASE_SHA" "$HEAD_SHA" > "$RUNNER_TEMP/localization-changes" && + scope="$(node config/scripts/localization-extraction-change-scope.mjs "$RUNNER_TEMP/localization-changes")" && [ "$scope" = false ]; then + echo "Localization extraction inputs are unchanged." + else + pnpm run verify:localization-extraction + fi + # Why both: the ratchet proves nothing reachable from the runtime imports electron, # which is a property of the import graph. This proves the Node artifact it enables # actually boots, pairs, creates a worktree and round-trips a real PTY. @@ -276,23 +293,6 @@ jobs: background: true run: pnpm run verify:localization-catalogs - # Why: extraction writes sorted evidence to an isolated temporary path, - # so feature PRs need one normalized AST pass rather than a three-OS matrix. - - name: Verify localization extraction - id: localization-extraction - background: true - env: - BASE_SHA: ${{ github.event.pull_request.base.sha }} - HEAD_SHA: ${{ github.event.pull_request.head.sha }} - run: | - # Detection failures run the full check; renames retain the removed input path. - if git diff --name-only --no-renames -z --merge-base "$BASE_SHA" "$HEAD_SHA" > "$RUNNER_TEMP/localization-changes" && - scope="$(node config/scripts/localization-extraction-change-scope.mjs "$RUNNER_TEMP/localization-changes")" && [ "$scope" = false ]; then - echo "Localization extraction inputs are unchanged." - else - pnpm run verify:localization-extraction - fi - - name: Verify localization coverage run: pnpm run verify:localization-coverage @@ -507,6 +507,8 @@ jobs: # fish-color-scheme-child-stdin.node-pty.test.ts (#9993) needs it. Noble ships # 3.7, so the PPA is what makes that lane real. - name: Install zsh and fish + id: shells + background: true run: | # Why the update/PPA/fish steps are tolerant: a repo the runner image already # ships can lack a Release file for this suite, and a failed add-apt-repository @@ -555,6 +557,12 @@ jobs: # first install command in this step to prove the lane really installs them. timeout 300 sudo apt-get install -y zsh fish + - uses: ./.github/actions/install-node-dependencies + with: + native-runtime: node + + - wait: shells + # Separate from the install so the failure names the contract, not an apt error. # ORCA_REQUIRE_FISH re-checks this at test time; this step just fails in seconds # instead of after a full dependency install. @@ -570,10 +578,6 @@ jobs: exit 1 fi - - uses: ./.github/actions/install-node-dependencies - with: - native-runtime: node - - name: Test real shell contracts run: | pnpm exec vitest run --config config/vitest.config.ts --maxWorkers=1 \ @@ -699,6 +703,11 @@ jobs: # The entry lives in mobile/ so one React resolves; without this every RN import is nothing. - uses: ./.github/actions/install-mobile-dependencies + - name: Prepare mobile route snapshot + id: mobile-routes + background: true + run: node config/scripts/run-mobile-web-app-checks.mjs --prepare-route-snapshot "$RUNNER_TEMP/mobile-routes.json" + # Why the runner's Google Chrome and not a downloaded chromium: same reason as the orcad # browser job -- Ubuntu 24.04 only ships an AppArmor userns profile for the Chrome .deb. # Why fail instead of skip: a silently skipped render check is the failure this job exists @@ -718,7 +727,7 @@ jobs: run: pnpm run build:mobile-web # Browser checks must observe both a finished install and the built bundle. - - wait: webkit + - wait: [webkit, mobile-routes] # The bundling tests skip themselves where mobile dependencies are absent, which is how they # stay green in the sharded `test` job. This is the job that installs them, so here a missing @@ -733,6 +742,7 @@ jobs: - name: Builder, override census and render checks env: ORCA_MOBILE_WEB_APP_DEPS_REQUIRED: '1' + ORCA_MOBILE_WEB_PREPARED_ROUTE_SNAPSHOT: ${{ runner.temp }}/mobile-routes.json run: | node config/scripts/run-mobile-web-app-checks.mjs diff --git a/config/scripts/ci-background-step-barriers.test.mjs b/config/scripts/ci-background-step-barriers.test.mjs index ccfe63a2916..8bf3d90ef23 100644 --- a/config/scripts/ci-background-step-barriers.test.mjs +++ b/config/scripts/ci-background-step-barriers.test.mjs @@ -22,6 +22,7 @@ describe('CI background step barriers', () => { pr.jobs.static_analysis, pr.jobs.mobile_web_app, pr.jobs.package, + pr.jobs.shell_contracts, mobile.jobs.verify, cloud.jobs.security ]) { @@ -67,15 +68,42 @@ describe('CI background step barriers', () => { it('waits for WebKit and the bundle before any browser tests', () => { const steps = pr.jobs.mobile_web_app.steps - assertJoinedBefore(steps, 'webkit', (step) => - step.run?.includes('run-mobile-web-app-checks.mjs') + assertJoinedBefore( + steps, + 'webkit', + (step) => step.name === 'Builder, override census and render checks' ) const build = steps.findIndex((step) => step.name === 'Build and verify the app bundle') expect(steps[build].background).toBeUndefined() - expect(build).toBeLessThan(steps.findIndex((step) => step.wait === 'webkit')) + expect(build).toBeLessThan(steps.findIndex((step) => [step.wait].flat().includes('webkit'))) expect(steps.findIndex((step) => step.id === 'webkit')).toBeLessThan(build) }) + it('joins shell installation before checking fish and running live shell tests', () => { + const steps = pr.jobs.shell_contracts.steps + assertJoinedBefore(steps, 'shells', (step) => step.name === 'Require fish 4+') + assertJoinedBefore(steps, 'shells', (step) => step.name === 'Test real shell contracts') + const install = steps.findIndex((step) => step.uses?.endsWith('/install-node-dependencies')) + expect(install).toBeGreaterThan(steps.findIndex((step) => step.id === 'shells')) + expect(install).toBeLessThan(steps.findIndex((step) => step.wait === 'shells')) + }) + + it('prepares fresh mobile routes after dependencies and before the browser tests', () => { + const steps = pr.jobs.mobile_web_app.steps + assertJoinedBefore( + steps, + 'mobile-routes', + (step) => step.name === 'Builder, override census and render checks' + ) + const prepare = steps.findIndex((step) => step.id === 'mobile-routes') + expect(prepare).toBeGreaterThan( + steps.findIndex((step) => step.uses?.endsWith('/install-mobile-dependencies')) + ) + expect(prepare).toBeLessThan( + steps.findIndex((step) => step.name === 'Build and verify the app bundle') + ) + }) + it('joins package setup before reading outputs and preserves isolated native probes', () => { const steps = pr.jobs.package.steps for (const [id, consumer] of [ diff --git a/config/scripts/mobile-web-app-route-snapshot.test.mjs b/config/scripts/mobile-web-app-route-snapshot.test.mjs index 901cdfcaaf0..a4dc897271e 100644 --- a/config/scripts/mobile-web-app-route-snapshot.test.mjs +++ b/config/scripts/mobile-web-app-route-snapshot.test.mjs @@ -1,7 +1,11 @@ import { existsSync, writeFileSync } from 'node:fs' import { describe, expect, it } from 'vitest' import { readRouteSnapshot } from './mobile-web-app-route-snapshot.mjs' -import { withRouteSnapshot } from './run-mobile-web-app-checks.mjs' +import { + prepareRouteSnapshot, + withPreparedRouteSnapshot, + withRouteSnapshot +} from './run-mobile-web-app-checks.mjs' import { PAGE_ROUTE_MODULES } from './mobile-web-app-page-route-modules.mjs' const collect = async (entries) => ({ @@ -70,3 +74,39 @@ describe('snapshot validation', () => { } ) }) + +it('consumes a separately prepared snapshot and removes it after success', async () => { + await withRouteSnapshot(async (file) => { + const result = await withPreparedRouteSnapshot(file, async (prepared) => { + expect(prepared).toBe(file) + return 'verified' + }) + expect(result).toBe('verified') + expect(existsSync(file)).toBe(false) + }, collect) +}) + +it('rejects incomplete prepared snapshots before launching tests', async () => { + await withRouteSnapshot(async (file) => { + writeFileSync(file, JSON.stringify({ version: 1, routes: [] })) + let launched = false + await expect( + withPreparedRouteSnapshot(file, async () => { + launched = true + }) + ).rejects.toThrow('missing') + expect(launched).toBe(false) + expect(existsSync(file)).toBe(false) + }, collect) +}) + +it('removes previous evidence before a failed preparation', async () => { + await withRouteSnapshot(async (file) => { + await expect( + prepareRouteSnapshot(file, async () => { + throw new Error('unresolved import') + }) + ).rejects.toThrow('unresolved import') + expect(existsSync(file)).toBe(false) + }, collect) +}) diff --git a/config/scripts/pr-workflow-parallelism.test.mjs b/config/scripts/pr-workflow-parallelism.test.mjs index 26b8cf6b439..fb93165d83c 100644 --- a/config/scripts/pr-workflow-parallelism.test.mjs +++ b/config/scripts/pr-workflow-parallelism.test.mjs @@ -517,9 +517,11 @@ describe('PR workflow parallelism', () => { // The bundling tests skip themselves without mobile/node_modules, which is what keeps the // sharded `test` job green. Only this env var stops that skip from spreading to the one job // that installs them, so a typo here would leave the whole job passing vacuously. - const step = workflow.jobs.mobile_web_app.steps.find((entry) => - entry.run?.includes('node config/scripts/run-mobile-web-app-checks.mjs') + const step = workflow.jobs.mobile_web_app.steps.find( + (entry) => entry.name === 'Builder, override census and render checks' ) + expect(step.run).toContain('node config/scripts/run-mobile-web-app-checks.mjs') + expect(step.run).not.toContain('--prepare-route-snapshot') expect(step.env[MOBILE_WEB_APP_DEPENDENCIES_REQUIRED_ENV]).toBe('1') expect(mobileWebCheckArgs).toEqual([ 'run', diff --git a/config/scripts/run-mobile-web-app-checks.mjs b/config/scripts/run-mobile-web-app-checks.mjs index 3f099dddf24..c6257676911 100644 --- a/config/scripts/run-mobile-web-app-checks.mjs +++ b/config/scripts/run-mobile-web-app-checks.mjs @@ -4,6 +4,7 @@ import { dirname, join } from 'node:path' import { pathToFileURL } from 'node:url' import { createRequire } from 'node:module' import { mobileWebAppModuleClosure } from './build-mobile-web-app-bundle.mjs' +import { readRouteSnapshot } from './mobile-web-app-route-snapshot.mjs' import { PAGE_ROUTE_MODULES } from './mobile-web-app-page-route-modules.mjs' import { spawnProcess } from './script-child-process.mjs' @@ -16,17 +17,35 @@ export const mobileWebCheckArgs = [ 'config/scripts/build-mobile-web-app-bundle.test.mjs' ] +export async function prepareRouteSnapshot(file, collect = mobileWebAppModuleClosure) { + rmSync(file, { force: true }) + const routes = [] + for (const route of new Set(PAGE_ROUTE_MODULES.values())) { + const closure = await collect(['app/_layout', 'app/h/_layout', route]) + routes.push({ route, closure }) + } + writeFileSync(file, JSON.stringify({ version: 1, routes })) +} + +export async function withPreparedRouteSnapshot(file, run) { + try { + for (const route of new Set(PAGE_ROUTE_MODULES.values())) { + if (!readRouteSnapshot(route, file)) { + throw new Error(`Prepared mobile route snapshot is missing ${route}`) + } + } + return await run(file) + } finally { + rmSync(file, { force: true }) + } +} + export async function withRouteSnapshot(run, collect = mobileWebAppModuleClosure) { const directory = mkdtempSync(join(tmpdir(), 'orca-route-snapshot-')) try { - const routes = [] - for (const route of new Set(PAGE_ROUTE_MODULES.values())) { - const closure = await collect(['app/_layout', 'app/h/_layout', route]) - routes.push({ route, closure }) - } const file = join(directory, 'routes.json') - writeFileSync(file, JSON.stringify({ version: 1, routes })) - return await run(file) + await prepareRouteSnapshot(file, collect) + return await withPreparedRouteSnapshot(file, run) } finally { rmSync(directory, { recursive: true, force: true }) } @@ -56,5 +75,14 @@ function runTests(file) { } if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) { - await withRouteSnapshot(runTests) + if (process.argv[2] === '--prepare-route-snapshot') { + if (process.argv.length !== 4) { + throw new Error('Expected --prepare-route-snapshot FILE') + } + await prepareRouteSnapshot(process.argv[3]) + } else if (process.env.ORCA_MOBILE_WEB_PREPARED_ROUTE_SNAPSHOT) { + await withPreparedRouteSnapshot(process.env.ORCA_MOBILE_WEB_PREPARED_ROUTE_SNAPSHOT, runTests) + } else { + await withRouteSnapshot(runTests) + } }