From 884913d0656e3fc9e98066fc620722722caefd5f Mon Sep 17 00:00:00 2001 From: Alexandru Dima Date: Mon, 29 Jun 2026 10:44:22 +0200 Subject: [PATCH] build: avoid per-section Electron re-download in PR test runs (#323341) The GitHub Actions PR test workflows run integration/smoke tests out of sources, so each test section launches scripts/code.bat, which runs build/lib/preLaunch.ts. Unlike the Azure Pipelines product builds, the GitHub workflows did not set VSCODE_SKIP_PRELAUNCH, so preLaunch ran on every section and getElectron() unconditionally deleted and re-downloaded .build/electron each time. On Windows this races with file locks held by the just-exited Electron process and intermittently fails the whole job with the bare 'The system cannot find the path specified.' error. - Set VSCODE_SKIP_PRELAUNCH=1 on the unit/integration/remote test steps of the win32, linux and darwin PR workflows, matching Azure Pipelines (the workflows already prepare node_modules, out, built-in extensions and Electron in dedicated steps before the tests run). - Make getElectron() version-aware: skip the destructive re-download when the installed Electron already matches the expected version, falling back to a download on any detection failure. - Make scripts/code.bat fail fast with a clear message when preLaunch.ts fails instead of falling through to launch a missing executable. - Retry rimraf on EBUSY/EPERM (Windows file-lock codes), not just ENOTEMPTY. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/workflows/pr-darwin-test.yml | 6 ++++++ .github/workflows/pr-linux-test.yml | 3 +++ .github/workflows/pr-win32-test.yml | 6 ++++++ build/lib/preLaunch.ts | 20 ++++++++++++++++++++ build/lib/util.ts | 2 +- scripts/code.bat | 5 ++++- 6 files changed, 40 insertions(+), 2 deletions(-) diff --git a/.github/workflows/pr-darwin-test.yml b/.github/workflows/pr-darwin-test.yml index 971fda8191f1..281f019ad283 100644 --- a/.github/workflows/pr-darwin-test.yml +++ b/.github/workflows/pr-darwin-test.yml @@ -122,6 +122,8 @@ jobs: if: ${{ inputs.electron_tests && inputs.unit_and_integration_tests }} timeout-minutes: 15 run: ./scripts/test.sh --tfs "Unit Tests" + env: + VSCODE_SKIP_PRELAUNCH: '1' - name: 🧪 Run unit tests (node.js) if: ${{ inputs.electron_tests && inputs.unit_and_integration_tests }} @@ -161,6 +163,8 @@ jobs: if: ${{ inputs.electron_tests && inputs.unit_and_integration_tests }} timeout-minutes: 20 run: ./scripts/test-integration.sh --tfs "Integration Tests" + env: + VSCODE_SKIP_PRELAUNCH: '1' - name: 🧪 Run integration tests (Browser, Webkit) if: ${{ inputs.browser_tests && inputs.unit_and_integration_tests }} @@ -171,6 +175,8 @@ jobs: if: ${{ inputs.remote_tests && inputs.unit_and_integration_tests }} timeout-minutes: 20 run: ./scripts/test-remote-integration.sh + env: + VSCODE_SKIP_PRELAUNCH: '1' - name: Compile smoke tests if: ${{ inputs.smoke_tests }} diff --git a/.github/workflows/pr-linux-test.yml b/.github/workflows/pr-linux-test.yml index 13fdf65f0783..65dd148287a6 100644 --- a/.github/workflows/pr-linux-test.yml +++ b/.github/workflows/pr-linux-test.yml @@ -324,6 +324,7 @@ jobs: run: ./scripts/test.sh --tfs "Unit Tests" env: DISPLAY: ":10" + VSCODE_SKIP_PRELAUNCH: '1' - name: 🧪 Run unit tests (node.js) if: ${{ inputs.electron_tests && inputs.unit_and_integration_tests }} @@ -365,6 +366,7 @@ jobs: run: ./scripts/test-integration.sh --tfs "Integration Tests" env: DISPLAY: ":10" + VSCODE_SKIP_PRELAUNCH: '1' - name: 🧪 Run integration tests (Browser, Chromium) if: ${{ inputs.browser_tests && inputs.unit_and_integration_tests }} @@ -377,6 +379,7 @@ jobs: run: ./scripts/test-remote-integration.sh env: DISPLAY: ":10" + VSCODE_SKIP_PRELAUNCH: '1' - name: Compile smoke tests if: ${{ inputs.smoke_tests }} diff --git a/.github/workflows/pr-win32-test.yml b/.github/workflows/pr-win32-test.yml index 45681aff7077..f1524e55fcc7 100644 --- a/.github/workflows/pr-win32-test.yml +++ b/.github/workflows/pr-win32-test.yml @@ -130,6 +130,8 @@ jobs: timeout-minutes: 15 shell: pwsh run: .\scripts\test.bat --tfs "Unit Tests" + env: + VSCODE_SKIP_PRELAUNCH: '1' - name: 🧪 Run unit tests (node.js) if: ${{ inputs.electron_tests && inputs.unit_and_integration_tests }} @@ -181,6 +183,8 @@ jobs: timeout-minutes: 20 shell: pwsh run: .\scripts\test-integration.bat --tfs "Integration Tests" + env: + VSCODE_SKIP_PRELAUNCH: '1' - name: 🧪 Run integration tests (Browser, Chromium) if: ${{ inputs.browser_tests && inputs.unit_and_integration_tests }} @@ -193,6 +197,8 @@ jobs: timeout-minutes: 20 shell: pwsh run: .\scripts\test-remote-integration.bat + env: + VSCODE_SKIP_PRELAUNCH: '1' - name: Diagnostics after integration test runs if: ${{ inputs.unit_and_integration_tests && always() }} diff --git a/build/lib/preLaunch.ts b/build/lib/preLaunch.ts index 5e175afde288..df9ef7738c6e 100644 --- a/build/lib/preLaunch.ts +++ b/build/lib/preLaunch.ts @@ -33,9 +33,29 @@ async function ensureNodeModules() { } async function getElectron() { + // `npm run electron` deletes and re-downloads `.build/electron` on every + // invocation. When preLaunch runs repeatedly (e.g. once per integration test + // section) this is both wasteful and a source of flaky failures on Windows, + // where the just-exited Electron process can still hold file locks while the + // directory is being removed and re-extracted. Skip the refresh when the + // already-present Electron matches the expected version; any detection + // failure falls back to a (re)download to preserve the previous behavior. + if (await isExpectedElectronInstalled()) { + return; + } await runProcess(npm, ['run', 'electron']); } +async function isExpectedElectronInstalled(): Promise { + try { + const { electronVersion } = await import('./electron.ts'); + const installedVersion = (await fs.readFile(path.join(rootDir, '.build', 'electron', 'version'), 'utf8')).trim().replace(/^v/, ''); + return installedVersion === electronVersion; + } catch { + return false; + } +} + async function ensureCompiled() { if (!(await exists('out'))) { await runProcess(npm, ['run', 'compile']); diff --git a/build/lib/util.ts b/build/lib/util.ts index d463aec5e999..760768dd07fc 100644 --- a/build/lib/util.ts +++ b/build/lib/util.ts @@ -302,7 +302,7 @@ export function rimraf(dir: string): () => Promise { return c(); } - if (err.code === 'ENOTEMPTY' && ++retries < 5) { + if ((err.code === 'ENOTEMPTY' || err.code === 'EBUSY' || err.code === 'EPERM') && ++retries < 5) { return setTimeout(() => retry(), 10); } diff --git a/scripts/code.bat b/scripts/code.bat index 62cfd0b4c490..51b27cb4664e 100644 --- a/scripts/code.bat +++ b/scripts/code.bat @@ -7,7 +7,10 @@ pushd %~dp0\.. :: Get electron, compile, built-in extensions if "%VSCODE_SKIP_PRELAUNCH%"=="" ( - node build/lib/preLaunch.ts + node build/lib/preLaunch.ts || ( + echo Failed to prepare VS Code for launch ^(build/lib/preLaunch.ts^). 1>&2 + exit /b 1 + ) ) set "NAMESHORT="