From ebe6b559eff3385db7e456a0c76c02ea1ecca3d2 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Thu, 27 Aug 2026 18:34:03 -0700 Subject: [PATCH] Prime changed native caches before test fanout (#16918) --- .../install-node-dependencies/action.yml | 5 +-- .github/workflows/pr.yml | 35 ++++++++++++++++--- config/scripts/pr-code-change-scope.mjs | 23 +++++++++++- config/scripts/pr-code-change-scope.test.mjs | 31 ++++++++++++++-- .../scripts/pr-workflow-parallelism.test.mjs | 12 +++++++ ...ws-pty-native-capability-workflow.test.mjs | 5 +++ 6 files changed, 101 insertions(+), 10 deletions(-) diff --git a/.github/actions/install-node-dependencies/action.yml b/.github/actions/install-node-dependencies/action.yml index e6d6f8f15e3..b67ce13cc1b 100644 --- a/.github/actions/install-node-dependencies/action.yml +++ b/.github/actions/install-node-dependencies/action.yml @@ -30,6 +30,7 @@ runs: - name: Setup pnpm uses: pnpm/action-setup@v6 with: + version: 10.24.0 run_install: false - name: Setup Node.js @@ -117,7 +118,7 @@ runs: node_modules/.pnpm/node-pty@*/node_modules/node-pty/build node_modules/.pnpm/windows-native-registry@*/node_modules/windows-native-registry/build node_modules/.pnpm/@vscode+windows-process-tree@*/node_modules/@vscode/windows-process-tree/build - key: native-modules-${{ runner.os }}-${{ steps.native-cache-scope.outputs.scope }}-${{ runner.arch }}-${{ inputs.native-runtime }}-node${{ steps.default-node.outputs.node-version }}${{ steps.requested-node.outputs.node-version }}-${{ hashFiles('pnpm-lock.yaml', 'config/patches/node-pty@1.1.0.patch', 'config/patches/@vscode__windows-process-tree@0.8.0.patch') }} + key: native-modules-${{ runner.os }}-${{ steps.native-cache-scope.outputs.scope }}-${{ runner.arch }}-${{ inputs.native-runtime }}-node${{ steps.default-node.outputs.node-version }}${{ steps.requested-node.outputs.node-version }}-${{ hashFiles('pnpm-lock.yaml', '.github/actions/install-node-dependencies/action.yml', 'config/scripts/ensure-native-runtime.mjs', 'config/scripts/rebuild-native-deps.mjs', 'config/patches/node-pty@1.1.0.patch', 'config/patches/@vscode__windows-process-tree@0.8.0.patch') }} - name: Restore compiled native modules without saving if: inputs.native-runtime != 'none' && inputs.persist-native-cache == 'false' @@ -127,7 +128,7 @@ runs: node_modules/.pnpm/node-pty@*/node_modules/node-pty/build node_modules/.pnpm/windows-native-registry@*/node_modules/windows-native-registry/build node_modules/.pnpm/@vscode+windows-process-tree@*/node_modules/@vscode/windows-process-tree/build - key: native-modules-${{ runner.os }}-${{ steps.native-cache-scope.outputs.scope }}-${{ runner.arch }}-${{ inputs.native-runtime }}-node${{ steps.default-node.outputs.node-version }}${{ steps.requested-node.outputs.node-version }}-${{ hashFiles('pnpm-lock.yaml', 'config/patches/node-pty@1.1.0.patch', 'config/patches/@vscode__windows-process-tree@0.8.0.patch') }} + key: native-modules-${{ runner.os }}-${{ steps.native-cache-scope.outputs.scope }}-${{ runner.arch }}-${{ inputs.native-runtime }}-node${{ steps.default-node.outputs.node-version }}${{ steps.requested-node.outputs.node-version }}-${{ hashFiles('pnpm-lock.yaml', '.github/actions/install-node-dependencies/action.yml', 'config/scripts/ensure-native-runtime.mjs', 'config/scripts/rebuild-native-deps.mjs', 'config/patches/node-pty@1.1.0.patch', 'config/patches/@vscode__windows-process-tree@0.8.0.patch') }} - name: Prepare native runtime if: inputs.native-runtime != 'none' diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 5eadeb9be81..9fbe35a173f 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -27,6 +27,7 @@ jobs: runs-on: ubuntu-latest outputs: should_run: ${{ steps.filter.outputs.should_run }} + native_cache_changed: ${{ steps.filter.outputs.native_cache_changed }} static_analysis: ${{ steps.filter.outputs.static_analysis }} typecheck: ${{ steps.filter.outputs.typecheck }} git_compatibility: ${{ steps.filter.outputs.git_compatibility }} @@ -442,10 +443,36 @@ jobs: src/shared/startup-shell-portability.live-shell.test.ts \ src/shared/posix-command-path-lookup.test.ts + # Cache-key input changes would otherwise make every shard compile the same + # native addon concurrently. Prime each Node ABI once before the matrix fans out. + test_native_cache: + name: prepare test native cache node ${{ matrix.node }} + needs: [code_paths] + if: needs.code_paths.outputs.native_cache_changed == 'true' + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + node: ['24', '26'] + + steps: + - name: Checkout + uses: actions/checkout@v6 + with: + persist-credentials: false + + - uses: ./.github/actions/install-node-dependencies + with: + native-runtime: node + node-version: ${{ matrix.node }} + test: name: tests node ${{ matrix.node }} ${{ matrix.shard }}/${{ matrix.shard_total }} - needs: [code_paths] - if: needs.code_paths.outputs.test == 'true' + needs: [code_paths, test_native_cache] + if: >- + always() && + needs.code_paths.outputs.test == 'true' && + (needs.test_native_cache.result == 'success' || needs.test_native_cache.result == 'skipped') runs-on: ubuntu-latest strategy: fail-fast: false @@ -695,7 +722,7 @@ jobs: node_modules/.pnpm/node-pty@*/node_modules/node-pty/build node_modules/.pnpm/windows-native-registry@*/node_modules/windows-native-registry/build node_modules/.pnpm/@vscode+windows-process-tree@*/node_modules/@vscode/windows-process-tree/build - key: native-modules-${{ runner.os }}-${{ steps.deps.outputs.native-cache-scope }}-${{ runner.arch }}-node-node${{ steps.deps.outputs.node-version }}-${{ hashFiles('pnpm-lock.yaml', 'config/patches/node-pty@1.1.0.patch', 'config/patches/@vscode__windows-process-tree@0.8.0.patch') }} + key: native-modules-${{ runner.os }}-${{ steps.deps.outputs.native-cache-scope }}-${{ runner.arch }}-node-node${{ steps.deps.outputs.node-version }}-${{ hashFiles('pnpm-lock.yaml', '.github/actions/install-node-dependencies/action.yml', 'config/scripts/ensure-native-runtime.mjs', 'config/scripts/rebuild-native-deps.mjs', 'config/patches/node-pty@1.1.0.patch', 'config/patches/@vscode__windows-process-tree@0.8.0.patch') }} - name: Test Windows-specific boundaries run: >- @@ -748,7 +775,7 @@ jobs: node_modules/.pnpm/node-pty@*/node_modules/node-pty/build node_modules/.pnpm/windows-native-registry@*/node_modules/windows-native-registry/build node_modules/.pnpm/@vscode+windows-process-tree@*/node_modules/@vscode/windows-process-tree/build - key: native-modules-${{ runner.os }}-${{ steps.deps.outputs.native-cache-scope }}-${{ runner.arch }}-electron-node${{ steps.deps.outputs.node-version }}-${{ hashFiles('pnpm-lock.yaml', 'config/patches/node-pty@1.1.0.patch', 'config/patches/@vscode__windows-process-tree@0.8.0.patch') }} + key: native-modules-${{ runner.os }}-${{ steps.deps.outputs.native-cache-scope }}-${{ runner.arch }}-electron-node${{ steps.deps.outputs.node-version }}-${{ hashFiles('pnpm-lock.yaml', '.github/actions/install-node-dependencies/action.yml', 'config/scripts/ensure-native-runtime.mjs', 'config/scripts/rebuild-native-deps.mjs', 'config/patches/node-pty@1.1.0.patch', 'config/patches/@vscode__windows-process-tree@0.8.0.patch') }} - name: Prepare Electron native runtime run: node config/scripts/ensure-native-runtime.mjs --runtime=electron diff --git a/config/scripts/pr-code-change-scope.mjs b/config/scripts/pr-code-change-scope.mjs index 8ccc864aabb..c51b1c18a5c 100644 --- a/config/scripts/pr-code-change-scope.mjs +++ b/config/scripts/pr-code-change-scope.mjs @@ -116,6 +116,19 @@ const NATIVE_RUNTIME_PREFIXES = [ 'config/patches/@vscode__windows-process-tree' ] +const NATIVE_CACHE_FILES = new Set([ + 'package.json', + 'pnpm-lock.yaml', + '.github/actions/install-node-dependencies/action.yml', + 'config/scripts/ensure-native-runtime.mjs', + 'config/scripts/rebuild-native-deps.mjs' +]) + +const NATIVE_CACHE_PREFIXES = [ + 'config/patches/node-pty@', + 'config/patches/@vscode__windows-process-tree' +] + const SHARED_PACKAGE_PREFIXES = [ 'electron.vite.config.ts', 'config/electron-builder', @@ -222,7 +235,11 @@ export function classifyPrJobs(changedFiles) { shouldRun && (forceAll || ALWAYS_ON_CODE_JOBS.has(job) || jobDetector(job)(changedFiles)) ]) ) - return { should_run: shouldRun, ...jobs } + return { + should_run: shouldRun, + native_cache_changed: shouldRun && (emptyDiff || changedFiles.some(isNativeCacheInputPath)), + ...jobs + } } function jobDetector(job) { @@ -274,6 +291,10 @@ function isDesktopIrrelevantPath(file) { return matchesPrefix(file, DESKTOP_IRRELEVANT_PREFIXES) } +function isNativeCacheInputPath(file) { + return NATIVE_CACHE_FILES.has(file) || matchesPrefix(file, NATIVE_CACHE_PREFIXES) +} + function isGlobalForcePath(file) { return GLOBAL_FORCE_FILES.has(file) || matchesPrefix(file, GLOBAL_FORCE_PREFIXES) } diff --git a/config/scripts/pr-code-change-scope.test.mjs b/config/scripts/pr-code-change-scope.test.mjs index 0ce697d1511..223929fc213 100644 --- a/config/scripts/pr-code-change-scope.test.mjs +++ b/config/scripts/pr-code-change-scope.test.mjs @@ -207,6 +207,22 @@ describe('per-job path classification', () => { expect(classifyPrJobs(['pnpm-lock.yaml']).git_compatibility).toBe(true) }) + it('primes native caches only when their immutable inputs change', () => { + expect(classifyPrJobs([]).native_cache_changed).toBe(true) + expect(classifyPrJobs(['README.md']).native_cache_changed).toBe(false) + expect(classifyPrJobs(['src/main/index.ts']).native_cache_changed).toBe(false) + for (const file of [ + 'package.json', + 'pnpm-lock.yaml', + '.github/actions/install-node-dependencies/action.yml', + 'config/scripts/ensure-native-runtime.mjs', + 'config/scripts/rebuild-native-deps.mjs', + 'config/patches/node-pty@1.1.0.patch' + ]) { + expect(classifyPrJobs([file]).native_cache_changed, file).toBe(true) + } + }) + it('keeps unit-test-only diffs out of packaging', () => { expectClassification(['src/main/git/git-status.test.ts'], { git_compatibility: true @@ -238,7 +254,7 @@ describe('PR Checks skip wiring', () => { expect(classify.run).toContain('--merge-base "$BASE_SHA" "$HEAD_SHA"') expect(classify.run).toContain('node config/scripts/pr-code-change-scope.mjs') expect(classify.run).toContain('tee -a "$GITHUB_OUTPUT"') - for (const jobName of ['should_run', ...expensiveJobs]) { + for (const jobName of ['should_run', 'native_cache_changed', ...expensiveJobs]) { expect(prWorkflow.jobs.code_paths.outputs[jobName], jobName).toBe( `\${{ steps.filter.outputs.${jobName} }}` ) @@ -250,13 +266,22 @@ describe('PR Checks skip wiring', () => { expect(prWorkflow.jobs.root_directory_guard.needs).toBeUndefined() }) - it('gates each expensive job on its own classifier output', () => { - for (const jobName of expensiveJobs) { + it('gates each expensive job on its classifier and cache prerequisite', () => { + for (const jobName of expensiveJobs.filter((jobName) => jobName !== 'test')) { expect(prWorkflow.jobs[jobName].needs, jobName).toEqual(['code_paths']) expect(prWorkflow.jobs[jobName].if, jobName).toBe( `needs.code_paths.outputs.${jobName} == 'true'` ) } + expect(prWorkflow.jobs.test.needs).toEqual(['code_paths', 'test_native_cache']) + expect(prWorkflow.jobs.test.if).toContain("needs.code_paths.outputs.test == 'true'") + expect(prWorkflow.jobs.test.if).toContain("needs.test_native_cache.result == 'success'") + expect(prWorkflow.jobs.test.if).toContain("needs.test_native_cache.result == 'skipped'") + expect(prWorkflow.jobs.test_native_cache.needs).toEqual(['code_paths']) + expect(prWorkflow.jobs.test_native_cache.if).toBe( + "needs.code_paths.outputs.native_cache_changed == 'true'" + ) + expect(prWorkflow.jobs.test_native_cache.strategy.matrix.node).toEqual(['24', '26']) }) it('skips e2e detection on docs-only PRs without dropping the draft gate', () => { diff --git a/config/scripts/pr-workflow-parallelism.test.mjs b/config/scripts/pr-workflow-parallelism.test.mjs index a61244d42d3..88bf4165d63 100644 --- a/config/scripts/pr-workflow-parallelism.test.mjs +++ b/config/scripts/pr-workflow-parallelism.test.mjs @@ -63,6 +63,13 @@ describe('PR workflow parallelism', () => { for (const testFile of nativeShellContractFiles) { expect(testStep.run).toContain(`--exclude=${testFile}`) } + const primerInstall = workflow.jobs.test_native_cache.steps.find( + (step) => step.uses === './.github/actions/install-node-dependencies' + ) + expect(workflow.jobs.test_native_cache.strategy.matrix.node).toEqual(['24', '26']) + expect(primerInstall.with['native-runtime']).toBe('node') + expect(primerInstall.with['node-version']).toBe('${{ matrix.node }}') + expect(workflow.jobs.test.needs).toContain('test_native_cache') }) it('runs real-shell coverage once outside the general shards', () => { @@ -208,6 +215,8 @@ describe('PR workflow parallelism', () => { expect(pnpmIndex).toBeLessThan(nodeIndex) expect(pnpmIndex).toBeLessThan(requestedNodeIndex) + const packageManagerVersion = /^pnpm@([^+]+)/.exec(packageJson.packageManager)?.[1] + expect(steps[pnpmIndex].with.version).toBe(packageManagerVersion) expect(steps[nodeIndex].with.cache).toBe('pnpm') expect(steps[nodeIndex].if).toBe("inputs.node-version == ''") expect(steps[requestedNodeIndex].if).toBe("inputs.node-version != ''") @@ -316,6 +325,9 @@ describe('PR workflow parallelism', () => { expect(cacheStep.with.key).toContain( 'config/patches/@vscode__windows-process-tree@0.8.0.patch' ) + expect(cacheStep.with.key).toContain('.github/actions/install-node-dependencies/action.yml') + expect(cacheStep.with.key).toContain('config/scripts/ensure-native-runtime.mjs') + expect(cacheStep.with.key).toContain('config/scripts/rebuild-native-deps.mjs') expect(cacheStep.with.path).toContain('node-pty@*/node_modules/node-pty/build') expect(cacheStep.with.path).toContain('windows-native-registry@') expect(cacheStep.with.path).toContain('@vscode+windows-process-tree@') diff --git a/config/scripts/windows-pty-native-capability-workflow.test.mjs b/config/scripts/windows-pty-native-capability-workflow.test.mjs index 8d4b7d568fd..e1742a43bb0 100644 --- a/config/scripts/windows-pty-native-capability-workflow.test.mjs +++ b/config/scripts/windows-pty-native-capability-workflow.test.mjs @@ -50,6 +50,11 @@ describe('packaged Windows PTY native capability routing', () => { expect(nodeCacheSave.uses).toBe('actions/cache/save@v5') expect(nodeCacheSave.with.key).toContain('-node-node') expect(electronCache.with.key).toContain('-electron-node') + for (const cache of [nodeCacheSave, electronCache]) { + expect(cache.with.key).toContain('.github/actions/install-node-dependencies/action.yml') + expect(cache.with.key).toContain('config/scripts/ensure-native-runtime.mjs') + expect(cache.with.key).toContain('config/scripts/rebuild-native-deps.mjs') + } expect(ensureNativeRuntime).toContain("runPnpm(['exec', 'node-gyp', 'rebuild']") expect(ensureNativeRuntime).toContain("resolve(moduleDir, 'scripts', 'post-install.js')") expect(build.run).toBe('pnpm run build:release:parallel')