From ebfae6a5e93e00bd068ba3739ecf2b22655d32ae Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Mon, 12 Oct 2026 18:11:57 -0700 Subject: [PATCH] Speed up Mobile Checks by overlapping typechecks and tests (#28127) Overlap mobile typechecks with tests and avoid repeated filesystem probes and unused device substitutes in recording replays. Keep state isolated between loaders, give exhaustive snapshot cases an explicit execution deadline, and retire obsolete locale keys that blocked repository validation. --- .github/workflows/mobile.yml | 7 +- .../scripts/mobile-test-change-scope.test.mjs | 26 +++++++ ...native-chat-merged-snapshot-parity.test.ts | 15 ++-- ...peration-module-loader-work-budget.test.ts | 74 +++++++++++++++++++ .../rpc-recording/operation-module-loader.ts | 16 ++-- 5 files changed, 123 insertions(+), 15 deletions(-) create mode 100644 mobile/src/test-support/rpc-recording/operation-module-loader-work-budget.test.ts diff --git a/.github/workflows/mobile.yml b/.github/workflows/mobile.yml index 30dc63186e1..57ac9819d91 100644 --- a/.github/workflows/mobile.yml +++ b/.github/workflows/mobile.yml @@ -135,10 +135,10 @@ jobs: # drifted. This fails when a test file that checks today stops checking, when a test leaves # the program, and on @ts-nocheck; the baseline may only shrink. - name: Typecheck tests (ratchet) + id: test-types + background: true run: node scripts/check-tests-typecheck-ratchet.mjs - - wait: production-types - # This includes the bridged replay of the whole recording corpus, which used to be a second # step of its own behind RPC_FOUNDATION_BRIDGE=1. A gate nobody can forget to set is the point: # it fails when a divergence class grows, when a divergence lands in no class at all, or when @@ -168,6 +168,9 @@ jobs: ORCA_BACKGROUND_LAUNCH: '1' run: node ../config/scripts/run-mobile-ci-tests.mjs + # All checks read the completed install; compilers need not hold up test workers. + - wait: [production-types, test-types] + # Reports, never gates: the goldens are content-addressed JSON, so their raw diff is hashes. # This decodes which recorded behaviour the pull request moves into the job summary, and runs # after a red Test too, when a reviewer most wants it. diff --git a/config/scripts/mobile-test-change-scope.test.mjs b/config/scripts/mobile-test-change-scope.test.mjs index 5e4e75b7685..7bdb513fd62 100644 --- a/config/scripts/mobile-test-change-scope.test.mjs +++ b/config/scripts/mobile-test-change-scope.test.mjs @@ -79,3 +79,29 @@ it('skips only tests, after successful detection, and retains all other mobile g expect(steps.find((step) => step.name === name).if).toBeUndefined() } }) + +it('overlaps both typechecks with tests and collects their failures before the summary', () => { + const workflow = parse( + readFileSync(new URL('../../.github/workflows/mobile.yml', import.meta.url), 'utf8') + ) + const steps = workflow.jobs.verify.steps + const install = steps.findIndex((step) => step.name === 'Install dependencies') + const test = steps.findIndex((step) => step.name === 'Test') + const summary = steps.findIndex((step) => step.name === 'Summarize RPC recording changes') + const waits = steps.filter((step) => step.wait !== undefined) + expect(waits).toHaveLength(1) + const [wait] = waits + expect(wait.wait).toEqual(['production-types', 'test-types']) + expect(wait.if).toBeUndefined() + expect(steps.indexOf(wait)).toBeGreaterThan(test) + expect(steps.indexOf(wait)).toBeLessThan(summary) + for (const id of wait.wait) { + const step = steps.find((candidate) => candidate.id === id) + expect(step.background).toBe(true) + expect(step['continue-on-error']).toBeUndefined() + expect(step.if).toBeUndefined() + expect(steps.indexOf(step)).toBeGreaterThan(install) + expect(steps.indexOf(step)).toBeLessThan(test) + expect(step.run).toMatch(/^node /) + } +}) diff --git a/mobile/src/session/mobile-native-chat-merged-snapshot-parity.test.ts b/mobile/src/session/mobile-native-chat-merged-snapshot-parity.test.ts index ae934923c72..89b22ac3461 100644 --- a/mobile/src/session/mobile-native-chat-merged-snapshot-parity.test.ts +++ b/mobile/src/session/mobile-native-chat-merged-snapshot-parity.test.ts @@ -221,11 +221,16 @@ describe.each(['after-end', 'during-turn'] as const)( ]) }) - it.each([1, 2, 3])('from every subscribe point, %i row(s) per frame', (rowsPerFrame) => { - expect(compareFrames(journal, range(2, journal.rows.length), rowsPerFrame).differing).toEqual( - [] - ) - }) + // Exhaustive frame enumeration shares CI CPUs with the typechecks. + it.each([1, 2, 3])( + 'from every subscribe point, %i row(s) per frame', + (rowsPerFrame) => { + expect( + compareFrames(journal, range(2, journal.rows.length), rowsPerFrame).differing + ).toEqual([]) + }, + 10_000 + ) } ) diff --git a/mobile/src/test-support/rpc-recording/operation-module-loader-work-budget.test.ts b/mobile/src/test-support/rpc-recording/operation-module-loader-work-budget.test.ts new file mode 100644 index 00000000000..e9dc9921883 --- /dev/null +++ b/mobile/src/test-support/rpc-recording/operation-module-loader-work-budget.test.ts @@ -0,0 +1,74 @@ +import { mkdtempSync, mkdirSync, rmSync, writeFileSync, existsSync } from 'node:fs' +import type * as FsModule from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, expect, it, vi } from 'vitest' +import { nativeMountingSubstitutes } from './native-mounting-substitutes' +import type * as NativeSubstitutesModule from './native-mounting-substitutes' +import { operationModuleLoader } from './operation-module-loader' + +vi.mock('node:fs', async (original) => { + const fs = await original() + return { ...fs, existsSync: vi.fn(fs.existsSync) } +}) + +vi.mock('./native-mounting-substitutes', async (original) => { + const module = await original() + return { ...module, nativeMountingSubstitutes: vi.fn(module.nativeMountingSubstitutes) } +}) + +const roots: string[] = [] + +function sourceTree(files: Record): string { + const root = mkdtempSync(join(tmpdir(), 'rpc-loader-work-budget-')) + roots.push(root) + mkdirSync(join(root, 'mobile', 'src'), { recursive: true }) + for (const [name, source] of Object.entries(files)) { + writeFileSync(join(root, 'mobile', 'src', name), source) + } + return root +} + +afterEach(() => { + for (const root of roots.splice(0)) { + rmSync(root, { recursive: true, force: true }) + } + vi.clearAllMocks() +}) + +it('resolves a relative import once instead of probing disk on every exported member read', () => { + const root = sourceTree({ + 'value.ts': 'export const value = 42', + 'operation.ts': `import { value } from './value'; export const read = () => value` + }) + const operation = operationModuleLoader(root).load<{ read: () => number }>( + 'mobile/src/operation.ts' + ) + expect(operation.read()).toBe(42) + vi.mocked(existsSync).mockClear() + for (let index = 0; index < 1_000; index++) { + expect(operation.read()).toBe(42) + } + expect(existsSync).not.toHaveBeenCalled() +}) + +it('builds device substitutes only for a used loader and keeps their state per loader', async () => { + const root = sourceTree({ + 'operation.ts': ` + import * as clipboard from 'expo-clipboard' + export const read = () => clipboard.getStringAsync() + export const write = () => clipboard.setStringAsync('first recording') + ` + }) + const loaders = Array.from({ length: 64 }, () => operationModuleLoader(root)) + expect(nativeMountingSubstitutes).not.toHaveBeenCalled() + type ClipboardOperation = { read: () => Promise; write: () => Promise } + const first = loaders[0].load('mobile/src/operation.ts') + await first.write() + expect(await first.read()).toBe('first recording') + expect(nativeMountingSubstitutes).toHaveBeenCalledTimes(1) + const second = loaders[1].load('mobile/src/operation.ts') + expect(await second.read()).toBe('') + expect(await first.read()).toBe('first recording') + expect(nativeMountingSubstitutes).toHaveBeenCalledTimes(2) +}) diff --git a/mobile/src/test-support/rpc-recording/operation-module-loader.ts b/mobile/src/test-support/rpc-recording/operation-module-loader.ts index b2f75a4cb4f..ae0ec820b51 100644 --- a/mobile/src/test-support/rpc-recording/operation-module-loader.ts +++ b/mobile/src/test-support/rpc-recording/operation-module-loader.ts @@ -32,7 +32,7 @@ export function operationModuleLoader( declared: ReadonlyMap = new Map() ) { const cache = new Map() - const natives = new Map([...nativeMountingSubstitutes(), ...declared]) + let natives: Map | undefined const sharedModulePath = resolve(root, SHARED_MODULE) let mutationCount = 0 function pathFor(base: string): string { @@ -45,13 +45,12 @@ export function operationModuleLoader( return file } function imported(base: string, name: string): unknown { + // Most registered adapters are never mounted in this recording. + natives ??= new Map([...nativeMountingSubstitutes(), ...declared]) const native = natives.get(name) if (native !== undefined) { return native } - if (name.startsWith('.') && pathFor(resolve(dirname(base), name)) === sharedModulePath) { - return deliveryAmbiguity - } if (!name.startsWith('.')) { return new Proxy( {}, @@ -69,10 +68,11 @@ export function operationModuleLoader( } ) } - return new Proxy( - {}, - { get: (_target, key) => load(pathFor(resolve(dirname(base), name)))[String(key)] } - ) + const file = pathFor(resolve(dirname(base), name)) + if (file === sharedModulePath) { + return deliveryAmbiguity + } + return new Proxy({}, { get: (_target, key) => load(file)[String(key)] }) } function barrel(file: string, source: string): OperationModule { const parsed = ts.createSourceFile(file, source, ts.ScriptTarget.Latest, true)