diff --git a/.github/workflows/mobile.yml b/.github/workflows/mobile.yml index 8382bb73b5d..537751f7223 100644 --- a/.github/workflows/mobile.yml +++ b/.github/workflows/mobile.yml @@ -38,6 +38,7 @@ on: - '.github/actions/install-node-dependencies/**' - '.github/workflows/mobile-ios-release.yml' - 'config/scripts/mobile-release-check-scope*' + - 'config/scripts/mobile-test-change-scope*' - 'config/scripts/pr-code-change-scope.mjs' - 'config/scripts/mobile-recording-pin-checkout.test.mjs' # Why main too: a behaviour-change branch legitimately pins its own last fenced commit, and that @@ -126,7 +127,21 @@ jobs: # one of the 103 goldens inside the C1 page closure changes the verdict it is pinned to. It is # ~3 min of test time on its own, and Vitest runs it on a worker beside the rest of the suite, # so folding it in costs a fraction of that in wall time and one step less to skip. + - name: Detect mobile test inputs + id: test-scope + shell: bash + working-directory: . + run: | + if ! git diff --name-only --no-renames -z HEAD^1 HEAD > "$RUNNER_TEMP/mobile-test-changes"; then + echo 'should_run=true' >> "$GITHUB_OUTPUT" + elif ! node config/scripts/mobile-test-change-scope.mjs "$RUNNER_TEMP/mobile-test-changes"; then + echo 'should_run=true' >> "$GITHUB_OUTPUT" + fi + - name: Test + if: steps.test-scope.outputs.should_run != 'false' + env: + ORCA_BACKGROUND_LAUNCH: '1' run: pnpm test - name: Test iOS release version resolution diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 68b0971ae6a..fc060cea125 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -710,17 +710,12 @@ jobs: # one `pr-code-change-scope.mjs` fires this job on, so naming a test into the family is all # it takes to have it run. Quoted because these are vitest filename filters, matched as # substrings against the discovered files, and the shell must not touch them. - # - # Cost: 18 files in 25-30s wall, of which the frame-budget sweep is 2.5s. That sweep encodes - # 111 noise JPEGs in Chromium, so it is the one step here whose cost grows with its viewport - # set; adding rows to that set is a decision about this job's runtime. + # Route censuses share fresh dependency lists for this invocation; scratch builds stay independent. - name: Builder, override census and render checks env: ORCA_MOBILE_WEB_APP_DEPS_REQUIRED: '1' run: | - pnpm exec vitest run --config config/vitest.config.ts \ - 'config/scripts/mobile-web-app-' \ - 'config/scripts/build-mobile-web-app-bundle.test.mjs' + node config/scripts/run-mobile-web-app-checks.mjs cross-version-wire: name: cross-version wire compatibility diff --git a/config/scripts/build-mobile-web-app-bundle.mjs b/config/scripts/build-mobile-web-app-bundle.mjs index 51178e9bed5..1bfad89b6e7 100644 --- a/config/scripts/build-mobile-web-app-bundle.mjs +++ b/config/scripts/build-mobile-web-app-bundle.mjs @@ -1,4 +1,5 @@ import { readFile } from 'node:fs/promises' +import { readRouteSnapshot } from './mobile-web-app-route-snapshot.mjs' import { realpathSync } from 'node:fs' import { basename, extname, join, resolve } from 'node:path' import { createRequire } from 'node:module' @@ -466,7 +467,10 @@ const isScriptOutput = (path) => path.endsWith('.js') * wrap every route, and their imports are part of the page as surely as the route module's. */ export async function mobileWebAppRouteClosure(routeModule) { - return await mobileWebAppModuleClosure(['app/_layout', 'app/h/_layout', routeModule]) + return ( + readRouteSnapshot(routeModule) ?? + (await mobileWebAppModuleClosure(['app/_layout', 'app/h/_layout', routeModule])) + ) } /** diff --git a/config/scripts/mobile-release-check-scope.test.mjs b/config/scripts/mobile-release-check-scope.test.mjs index faf90039feb..cd7c204f817 100644 --- a/config/scripts/mobile-release-check-scope.test.mjs +++ b/config/scripts/mobile-release-check-scope.test.mjs @@ -66,13 +66,13 @@ it('runs Ruby checks when the changed-file evidence is empty', () => { expect(shouldRunMobileReleaseChecks([])).toBe(true) }) -it('gates only Ruby steps and keeps ordinary mobile validation unconditional', () => { +it('gates Ruby independently and retains mobile static validation', () => { expect(steps[0].with['fetch-depth']).toBe(2) expect(detector['working-directory']).toBe('.') expect(steps.indexOf(detector)).toBeGreaterThan( steps.findIndex((step) => step.uses === './.github/actions/install-node-dependencies') ) - const gated = steps.filter((step) => step.if !== undefined) + const gated = steps.filter((step) => step.if !== undefined && step.name !== 'Test') expect(gated.map((step) => step.name)).toEqual([ 'Setup Ruby and fastlane', 'Test iOS release version resolution', @@ -82,13 +82,7 @@ it('gates only Ruby steps and keeps ordinary mobile validation unconditional', ( for (const step of gated) { expect(step.if).toBe("steps.ruby-scope.outputs.should_run != 'false'") } - for (const name of [ - 'Typecheck', - 'Typecheck tests (ratchet)', - 'Test', - 'Lint', - 'Check formatting' - ]) { + for (const name of ['Typecheck', 'Typecheck tests (ratchet)', 'Lint', 'Check formatting']) { expect(steps.find((step) => step.name === name)?.if).toBeUndefined() expect(steps.some((step) => step.name === name)).toBe(true) } @@ -108,7 +102,7 @@ it('gates only Ruby steps and keeps ordinary mobile validation unconditional', ( ) }) -function fixture() { +function fixture(detectorStep = detector) { const directory = mkdtempSync(join(tmpdir(), 'mobile-release-scope-')) directories.push(directory) const git = (...args) => { @@ -130,7 +124,8 @@ function fixture() { for (const file of [ 'package.json', 'config/scripts/pr-code-change-scope.mjs', - 'config/scripts/mobile-release-check-scope.mjs' + 'config/scripts/mobile-release-check-scope.mjs', + 'config/scripts/mobile-test-change-scope.mjs' ]) { copyFileSync(join(root, file), join(directory, file)) } @@ -152,7 +147,7 @@ function fixture() { const output = join(directory, 'github-output') const result = runProcessSync({ program: 'bash', - args: ['-e', '-c', detector.run], + args: ['-e', '-c', detectorStep.run], cwd: directory, env: { ...process.env, GITHUB_OUTPUT: output, RUNNER_TEMP: directory } }) @@ -189,3 +184,32 @@ describe.skipIf(process.platform === 'win32')('the Linux workflow detector comma expect(repo.detect()).toBe('should_run=true\n') }) }) + +describe.skipIf(process.platform === 'win32')('the mobile test detector command', () => { + const testDetector = steps.find((step) => step.id === 'test-scope') + it('skips release-only changes while retaining source changes', () => { + const repo = fixture(testDetector) + repo.write('mobile/fastlane/Fastfile', 'default_platform(:android)\n') + repo.commit() + expect(repo.detect()).toBe('should_run=false\n') + repo.write('mobile/src/view.tsx', 'export const view = 2\n') + repo.commit() + expect(repo.detect()).toBe('should_run=false\nshould_run=true\n') + }) + it('retains tests after source moves into a documentation directory', () => { + const repo = fixture(testDetector) + repo.git('mv', 'mobile/src/view.tsx', 'mobile/README.md') + repo.commit() + expect(repo.detect()).toBe('should_run=true\n') + }) + it('retains tests when the diff is unavailable', () => { + expect(fixture(testDetector).detect()).toBe('should_run=true\n') + }) + it('retains tests when the classifier cannot execute', () => { + const repo = fixture(testDetector) + repo.write('mobile/fastlane/Fastfile', 'default_platform(:android)\n') + repo.commit() + rmSync(join(repo.directory, 'config/scripts/mobile-test-change-scope.mjs')) + expect(repo.detect()).toBe('should_run=true\n') + }) +}) diff --git a/config/scripts/mobile-test-change-scope.mjs b/config/scripts/mobile-test-change-scope.mjs new file mode 100644 index 00000000000..cdd04d6edf5 --- /dev/null +++ b/config/scripts/mobile-test-change-scope.mjs @@ -0,0 +1,34 @@ +import { appendFileSync, readFileSync } from 'node:fs' +import { pathToFileURL } from 'node:url' +import { isDocsOnlyPath } from './pr-code-change-scope.mjs' + +const NON_TEST_FILES = new Set([ + 'mobile/Gemfile', + 'mobile/Gemfile.lock', + 'mobile/README.md', + '.github/workflows/mobile-ios-release.yml', + 'config/scripts/pr-code-change-scope.mjs', + 'config/scripts/pr-code-change-scope.test.mjs', + 'config/scripts/mobile-release-check-scope.mjs', + 'config/scripts/mobile-release-check-scope.test.mjs' +]) + +export function shouldRunMobileTests(files) { + return ( + files.length === 0 || + files.some( + (file) => + file.split('/').includes('..') || + (!isDocsOnlyPath(file) && + !NON_TEST_FILES.has(file) && + !file.startsWith('mobile/docs/') && + !file.startsWith('mobile/fastlane/')) + ) + ) +} + +if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) { + const input = readFileSync(process.argv[2], 'utf8') + const files = input.endsWith('\0') ? input.slice(0, -1).split('\0') : [] + appendFileSync(process.env.GITHUB_OUTPUT, `should_run=${shouldRunMobileTests(files)}\n`) +} diff --git a/config/scripts/mobile-test-change-scope.test.mjs b/config/scripts/mobile-test-change-scope.test.mjs new file mode 100644 index 00000000000..8859daca448 --- /dev/null +++ b/config/scripts/mobile-test-change-scope.test.mjs @@ -0,0 +1,60 @@ +import { readFileSync } from 'node:fs' +import { parse } from 'yaml' +import { expect, it } from 'vitest' +import { shouldRunMobileTests } from './mobile-test-change-scope.mjs' + +it.each([ + 'docs/ci.md', + 'mobile/docs/release.md', + 'mobile/README.md', + 'mobile/Gemfile.lock', + 'mobile/fastlane/Fastfile', + '.github/workflows/mobile-ios-release.yml', + 'config/scripts/pr-code-change-scope.mjs', + 'config/scripts/mobile-release-check-scope.test.mjs' +])('skips only known non-test inputs: %s', (file) => { + expect(shouldRunMobileTests([file])).toBe(false) + expect(shouldRunMobileTests([file, 'mobile/src/changed.ts'])).toBe(true) +}) + +it.each([ + 'mobile/src/view.tsx', + 'mobile/app/_layout.tsx', + 'mobile/scripts/check.ts', + 'mobile/rpc-foundation/goldens/recording.json', + 'mobile/vitest.config.ts', + 'mobile/vitest.setup.ts', + 'mobile/pnpm-lock.yaml', + 'pnpm-lock.yaml', + 'src/main/runtime/runtime-rpc.ts', + 'src/shared/protocol-version.ts', + '.github/workflows/mobile.yml', + '.github/actions/install-node-dependencies/action.yml', + 'config/scripts/mobile-test-change-scope.mjs', + 'unknown-input', + 'mobile/docs/../../src/source.ts' +])('retains source, fixtures, toolchain, selector and unknown changes: %s', (file) => { + expect(shouldRunMobileTests([file])).toBe(true) +}) + +it('retains tests on missing evidence and a move out of the source tree', () => { + expect(shouldRunMobileTests([])).toBe(true) + expect(shouldRunMobileTests(['mobile/src/deleted.ts', 'mobile/docs/moved.ts'])).toBe(true) +}) + +it('skips only tests, after successful detection, and retains all other mobile gates', () => { + const workflow = parse( + readFileSync(new URL('../../.github/workflows/mobile.yml', import.meta.url), 'utf8') + ) + const steps = workflow.jobs.verify.steps + expect(workflow.on.pull_request.paths).toContain('config/scripts/mobile-test-change-scope*') + const detector = steps.find((step) => step.id === 'test-scope') + expect(detector.run).toContain('git diff --name-only --no-renames -z HEAD^1 HEAD') + expect(detector.run.match(/should_run=true/g)).toHaveLength(2) + expect(steps.find((step) => step.name === 'Test').if).toBe( + "steps.test-scope.outputs.should_run != 'false'" + ) + for (const name of ['Typecheck', 'Typecheck tests (ratchet)', 'Lint', 'Check formatting']) { + expect(steps.find((step) => step.name === name).if).toBeUndefined() + } +}) diff --git a/config/scripts/mobile-web-app-route-snapshot.mjs b/config/scripts/mobile-web-app-route-snapshot.mjs new file mode 100644 index 00000000000..248481ee0d4 --- /dev/null +++ b/config/scripts/mobile-web-app-route-snapshot.mjs @@ -0,0 +1,25 @@ +import { readFileSync } from 'node:fs' + +export function readRouteSnapshot(route, file = process.env.ORCA_MOBILE_WEB_ROUTE_SNAPSHOT) { + if (!file) { + return undefined + } + // The test runner creates this file for one immutable checkout and deletes it on exit. + const snapshot = JSON.parse(readFileSync(file, 'utf8')) + if (snapshot.version !== 1 || !Array.isArray(snapshot.routes)) { + throw new Error('Invalid mobile route snapshot') + } + const entry = snapshot.routes.find((item) => item.route === route) + if (!entry) { + return undefined + } + if ( + !Array.isArray(entry.closure?.modules) || + !Array.isArray(entry.closure?.local) || + !entry.closure.modules.every((item) => typeof item === 'string') || + !entry.closure.local.every((item) => typeof item === 'string') + ) { + throw new Error(`Invalid mobile route closure: ${route}`) + } + return entry.closure +} diff --git a/config/scripts/mobile-web-app-route-snapshot.test.mjs b/config/scripts/mobile-web-app-route-snapshot.test.mjs new file mode 100644 index 00000000000..901cdfcaaf0 --- /dev/null +++ b/config/scripts/mobile-web-app-route-snapshot.test.mjs @@ -0,0 +1,72 @@ +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 { PAGE_ROUTE_MODULES } from './mobile-web-app-page-route-modules.mjs' + +const collect = async (entries) => ({ + modules: [...entries, 'node_modules/react/index.js'], + local: entries +}) + +it('collects each real route once and shares isolated results for one invocation', async () => { + const collected = [] + let snapshotFile + await withRouteSnapshot( + async (file) => { + snapshotFile = file + for (const route of PAGE_ROUTE_MODULES.values()) { + const expected = await collect(['app/_layout', 'app/h/_layout', route]) + expect(readRouteSnapshot(route, file)).toEqual(expected) + readRouteSnapshot(route, file).local.length = 0 + expect(readRouteSnapshot(route, file)).toEqual(expected) + } + expect(readRouteSnapshot('new-route', file)).toBeUndefined() + }, + async (entries) => { + collected.push(entries[2]) + return collect(entries) + } + ) + expect(collected).toEqual([...new Set(PAGE_ROUTE_MODULES.values())]) + expect(existsSync(snapshotFile)).toBe(false) +}) + +it('removes the snapshot after a failed suite and propagates the failure', async () => { + let snapshotFile + await expect( + withRouteSnapshot(async (file) => { + snapshotFile = file + throw new Error('failed assertion') + }, collect) + ).rejects.toThrow('failed assertion') + expect(existsSync(snapshotFile)).toBe(false) +}) + +it('never launches tests after dependency collection fails', async () => { + await expect( + withRouteSnapshot( + () => { + throw new Error('must not launch') + }, + async () => { + throw new Error('unresolved import') + } + ) + ).rejects.toThrow('unresolved import') +}) + +describe('snapshot validation', () => { + it('leaves ordinary builds and scratch routes uncached', () => { + expect(readRouteSnapshot('any', '')).toBeUndefined() + }) + it.each(['{}', '{', '{"version":1,"routes":[{"route":"bad","closure":{}}]}'])( + 'rejects damaged snapshots: %s', + async (bytes) => { + await withRouteSnapshot(async (file) => { + writeFileSync(file, bytes) + expect(() => readRouteSnapshot('bad', file)).toThrow() + }, collect) + } + ) +}) diff --git a/config/scripts/pr-code-change-scope.mjs b/config/scripts/pr-code-change-scope.mjs index b560bdf45ab..ceb75a66bfc 100644 --- a/config/scripts/pr-code-change-scope.mjs +++ b/config/scripts/pr-code-change-scope.mjs @@ -113,6 +113,9 @@ const ORCAD_BROWSER_PREFIXES = [ // import, and the shell policy the render check runs the page under. const MOBILE_WEB_APP_PREFIXES = [ 'config/scripts/build-mobile-web-app', + 'config/scripts/run-mobile-web-app-checks', + 'config/scripts/script-child-process.mjs', + 'src/shared/child-process/', 'config/scripts/verify-mobile-web-app-bundle', 'config/scripts/mobile-web-app-', 'config/scripts/mobile-web-bundle-', diff --git a/config/scripts/pr-code-change-scope.test.mjs b/config/scripts/pr-code-change-scope.test.mjs index ead750bcce9..06ef8e3879e 100644 --- a/config/scripts/pr-code-change-scope.test.mjs +++ b/config/scripts/pr-code-change-scope.test.mjs @@ -211,7 +211,10 @@ describe('per-job path classification', () => { 'config/scripts/run-headless-linux-pairing-docker.mjs', 'config/scripts/static-appimage-package-contract.cjs' ]) { - expectClassification([file], { package: true }) + expectClassification([file], { + package: true, + mobile_web_app: file === 'config/scripts/script-child-process.mjs' + }) } }) @@ -223,7 +226,10 @@ describe('per-job path classification', () => { 'config/docker/daemon-shutdown-descendants/run-case.sh', 'config/scripts/run-daemon-shutdown-descendants-docker.mjs' ]) { - expectClassification([file], { package: true }) + expectClassification([file], { + package: true, + mobile_web_app: file === 'config/scripts/script-child-process.mjs' + }) } for (const file of [ 'src/main/daemon/terminal-host.ts', @@ -288,6 +294,9 @@ describe('per-job path classification', () => { it('runs the mobile web app job for the builder, the page source and the shell policy', () => { for (const file of [ 'config/scripts/build-mobile-web-app-bundle.mjs', + 'config/scripts/run-mobile-web-app-checks.mjs', + 'config/scripts/script-child-process.mjs', + 'src/shared/child-process/run-process.ts', 'config/scripts/mobile-web-app-route-manifest.mjs', 'mobile/web-entry/index.tsx', 'mobile/app/h/[hostId]/index.tsx', diff --git a/config/scripts/pr-workflow-parallelism.test.mjs b/config/scripts/pr-workflow-parallelism.test.mjs index 6aaa4b25018..26b8cf6b439 100644 --- a/config/scripts/pr-workflow-parallelism.test.mjs +++ b/config/scripts/pr-workflow-parallelism.test.mjs @@ -1,6 +1,7 @@ import { existsSync, globSync, readFileSync } from 'node:fs' import { parse } from 'yaml' import { describe, expect, it } from 'vitest' +import { mobileWebCheckArgs } from './run-mobile-web-app-checks.mjs' import { MOBILE_WEB_APP_DEPENDENCIES_REQUIRED_ENV } from './mobile-web-app-bundle-dependencies.mjs' const workflow = parse(readFileSync('.github/workflows/pr.yml', 'utf8')) @@ -517,8 +518,15 @@ describe('PR workflow parallelism', () => { // 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('build-mobile-web-app-bundle.test.mjs') + entry.run?.includes('node config/scripts/run-mobile-web-app-checks.mjs') ) expect(step.env[MOBILE_WEB_APP_DEPENDENCIES_REQUIRED_ENV]).toBe('1') + expect(mobileWebCheckArgs).toEqual([ + 'run', + '--config', + 'config/vitest.config.ts', + 'config/scripts/mobile-web-app-', + 'config/scripts/build-mobile-web-app-bundle.test.mjs' + ]) }) }) diff --git a/config/scripts/run-mobile-web-app-checks.mjs b/config/scripts/run-mobile-web-app-checks.mjs new file mode 100644 index 00000000000..3f099dddf24 --- /dev/null +++ b/config/scripts/run-mobile-web-app-checks.mjs @@ -0,0 +1,60 @@ +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +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 { PAGE_ROUTE_MODULES } from './mobile-web-app-page-route-modules.mjs' +import { spawnProcess } from './script-child-process.mjs' + +const require = createRequire(import.meta.url) +export const mobileWebCheckArgs = [ + 'run', + '--config', + 'config/vitest.config.ts', + 'config/scripts/mobile-web-app-', + 'config/scripts/build-mobile-web-app-bundle.test.mjs' +] + +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) + } finally { + rmSync(directory, { recursive: true, force: true }) + } +} + +function runTests(file) { + return new Promise((resolve, reject) => { + const child = spawnProcess({ + program: process.execPath, + args: [ + join(dirname(require.resolve('vitest/package.json')), 'vitest.mjs'), + ...mobileWebCheckArgs, + ...process.argv.slice(2) + ], + env: { ...process.env, ORCA_BACKGROUND_LAUNCH: '1', ORCA_MOBILE_WEB_ROUTE_SNAPSHOT: file }, + stdio: 'inherit' + }) + child.once('error', reject) + child.once('close', (code, signal) => { + if (code === 0 && signal === null) { + resolve() + } else { + reject(new Error(`Mobile web checks failed (code=${code}, signal=${signal})`)) + } + }) + }) +} + +if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) { + await withRouteSnapshot(runTests) +}