From beea65a29b65d4436368f4582415440b2c80f31d Mon Sep 17 00:00:00 2001 From: dprevost-perso Date: Mon, 21 Sep 2026 08:28:24 -0400 Subject: [PATCH 1/2] fix: give each edgedriver e2e test its own driver cache dir Tests previously shared one cache path (EDGEDRIVER_CACHE_DIR or os.tmpdir()) and deleted+redownloaded into it between tests, racing against whatever process the prior test hadn't fully torn down. Each test now gets a fresh mkdtemp() dir instead, removing the shared-file race entirely. Also drops the EDGE_BINARY_PATH cleanup block, which was dead code: that env var locates the Edge browser binary (see finder.ts), not a driver cache path. Co-Authored-By: Claude Sonnet 5 --- packages/e2e/tests/edgedriver.e2e.test.ts | 52 +++++++++++++---------- 1 file changed, 30 insertions(+), 22 deletions(-) diff --git a/packages/e2e/tests/edgedriver.e2e.test.ts b/packages/e2e/tests/edgedriver.e2e.test.ts index ae16b3e..b16c959 100644 --- a/packages/e2e/tests/edgedriver.e2e.test.ts +++ b/packages/e2e/tests/edgedriver.e2e.test.ts @@ -4,39 +4,46 @@ import { remote } from 'webdriverio' import fs from 'node:fs/promises' import path from 'node:path' import os from 'node:os' -import { beforeEach, describe, it } from 'vitest' +import { beforeEach, afterEach, describe, it } from 'vitest' import { start, download, findEdgePath } from 'edgedriver' describe('Edgedriver E2E Tests', () => { - const port = 4444 + /** + * Give every test its own driver cache dir instead of the shared default + * (EDGEDRIVER_CACHE_DIR or os.tmpdir()). Tests used to all download into + * the same /tmp/msedgedriver and delete-then-redownload it between tests, + * which raced against whatever process the previous test hadn't fully + * torn down yet. + */ + let cacheDir = '' beforeEach(async () => { - const tempDir = process.env.EDGEDRIVER_CACHE_DIR || os.tmpdir() - const edgedriverCachePath = tempDir ? path.resolve(tempDir, 'msedgedriver') : '' - if (edgedriverCachePath && await fs.lstat(edgedriverCachePath).catch(() => false)) { - console.log(`Removing existing Edge binary at ${edgedriverCachePath}`) - await fs.unlink(edgedriverCachePath) - } + cacheDir = await fs.mkdtemp(path.join(os.tmpdir(), 'edgedriver-e2e-')) - const edgedriverPath = process.env.EDGE_BINARY_PATH ? path.resolve(process.env.EDGE_BINARY_PATH, 'msedgedriver') : '' - if (edgedriverPath && await fs.lstat(edgedriverPath).catch(() => false)) { - console.log(`Removing existing Edge binary at ${edgedriverPath}`) - await fs.unlink(edgedriverPath) + /** + * Force-kill any leftover msedgedriver/Edge processes from a previous test. + * cp.kill() below isn't guaranteed to have fully torn down the Edge browser + * process it spawned by the time the next test starts; a lingering instance + * here can push memory over the CI runner's limit and get OOM-killed + * (surfaces as a bare "Killed" + exit 137 in the next test). + */ + for (const pattern of ['msedgedriver', 'microsoft-edge']) { + try { + execSync(`pkill -9 -f ${pattern}`) + } catch { + // no matching process running, nothing to clean up + } } + }) - try { - // Kill pending processes - execSync(`kill -9 $(lsof -t -i :${port})`) - console.log(`Successfully killed process on port ${port}`) - } catch { - console.log(`No process found running on port ${port}, or insufficient permissions.`) - } + afterEach(async () => { + await fs.rm(cacheDir, { recursive: true, force: true }) }) it('start edgedriver manually', async () => { const port = 4444 - const cp = await start({ port }) + const cp = await start({ port, cacheDir }) try { await waitPort({ port: 4444 }) @@ -57,11 +64,12 @@ describe('Edgedriver E2E Tests', () => { await browser.deleteSession() } finally { cp.kill() + await new Promise((resolve) => cp.once('exit', resolve)) } }) it('start specific edgedriver', async () => { - const binary = await download() + const binary = await download(undefined, cacheDir) const browser = await remote({ automationProtocol: 'webdriver', @@ -80,7 +88,7 @@ describe('Edgedriver E2E Tests', () => { }) it('start with missing architecture', async () => { - const binary = await download('152.0.4191.77') + const binary = await download('152.0.4191.77', cacheDir) const browser = await remote({ automationProtocol: 'webdriver', From f6442df45d0b84e948573079b0432d7ad39f0ebb Mon Sep 17 00:00:00 2001 From: dprevost-perso Date: Mon, 21 Sep 2026 16:55:41 -0400 Subject: [PATCH 2/2] fix: actually kill the driver process before deleting its cache dir afterEach only deleted the per-test cache dir; it never confirmed the driver process was dead first. On Windows that's a hard EPERM (can't unlink a running .exe); on Linux/macOS it silently "succeeds" while the process keeps running and consuming memory - the same mechanism likely behind the original Ubuntu OOM. Kill is now cross-platform (taskkill on Windows, pkill elsewhere), runs in afterEach too (not just beforeEach), and fs.rm gets retries since Windows can lag briefly releasing the file handle even after the process exits. Co-Authored-By: Claude Sonnet 5 --- packages/e2e/tests/edgedriver.e2e.test.ts | 45 +++++++++++++++-------- 1 file changed, 29 insertions(+), 16 deletions(-) diff --git a/packages/e2e/tests/edgedriver.e2e.test.ts b/packages/e2e/tests/edgedriver.e2e.test.ts index b16c959..a451251 100644 --- a/packages/e2e/tests/edgedriver.e2e.test.ts +++ b/packages/e2e/tests/edgedriver.e2e.test.ts @@ -8,6 +8,27 @@ import { beforeEach, afterEach, describe, it } from 'vitest' import { start, download, findEdgePath } from 'edgedriver' +/** + * Force-kill any leftover msedgedriver/Edge processes. Sessions started via + * `wdio:edgedriverOptions.binary` have their driver process managed + * internally by WebdriverIO - deleteSession() ends the WebDriver session but + * doesn't guarantee that process has actually exited yet. A lingering + * instance either blocks deleting its own binary on Windows (EPERM: file + * still in use) or, on Linux/macOS, silently keeps running and consuming + * memory into the next test (surfaces as a bare "Killed" + exit 137). + */ +function killDriverProcesses() { + const isWindows = process.platform === 'win32' + const patterns = isWindows ? ['msedgedriver.exe', 'msedge.exe'] : ['msedgedriver', 'microsoft-edge'] + for (const pattern of patterns) { + try { + execSync(isWindows ? `taskkill /IM ${pattern} /F /T` : `pkill -9 -f ${pattern}`, { stdio: 'ignore' }) + } catch { + // no matching process running, nothing to clean up + } + } +} + describe('Edgedriver E2E Tests', () => { /** * Give every test its own driver cache dir instead of the shared default @@ -20,25 +41,17 @@ describe('Edgedriver E2E Tests', () => { beforeEach(async () => { cacheDir = await fs.mkdtemp(path.join(os.tmpdir(), 'edgedriver-e2e-')) - - /** - * Force-kill any leftover msedgedriver/Edge processes from a previous test. - * cp.kill() below isn't guaranteed to have fully torn down the Edge browser - * process it spawned by the time the next test starts; a lingering instance - * here can push memory over the CI runner's limit and get OOM-killed - * (surfaces as a bare "Killed" + exit 137 in the next test). - */ - for (const pattern of ['msedgedriver', 'microsoft-edge']) { - try { - execSync(`pkill -9 -f ${pattern}`) - } catch { - // no matching process running, nothing to clean up - } - } + killDriverProcesses() }) afterEach(async () => { - await fs.rm(cacheDir, { recursive: true, force: true }) + killDriverProcesses() + /** + * maxRetries/retryDelay: Windows can lag briefly between a process + * exiting and the OS actually releasing its handle on the binary, + * even after killDriverProcesses() above returns. + */ + await fs.rm(cacheDir, { recursive: true, force: true, maxRetries: 5, retryDelay: 200 }) }) it('start edgedriver manually', async () => {