Skip to content

Commit d1826bb

Browse files
stephentoubCopilot
andcommitted
Make Windows test cleanup soft-fail to fully tolerate CLI 1.0.40-1 SQLite race
The previous tightening (30s rmDir retry; suppress(OSError) for unlink in central context.py) was not enough on Windows: the @github/copilot 1.0.40-1 CLI keeps the SQLite session-store.db handle open longer than 30 seconds in some cases, and three additional multi-client test files have their own configure_for_test methods with the same unlink-without-suppression pattern. - python/e2e/test_commands.py, test_multi_client.py, test_ui_elicitation_multi_client.py: wrap item.unlink(missing_ok=True) in contextlib.suppress(OSError), matching the change in testharness/context.py - nodejs/test/e2e/harness/sdkTestContext.ts: rmDir now warns instead of throwing if the temp dir cannot be removed after the 30s retry budget — temp dirs are reclaimed by the OS / GitHub Actions runner anyway, so a stale temp dir should not fail the entire test suite when 245/254 tests passed Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 5563d46 commit d1826bb

4 files changed

Lines changed: 30 additions & 18 deletions

File tree

nodejs/test/e2e/harness/sdkTestContext.ts

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import { fileURLToPath } from "url";
1111
import { afterAll, afterEach, beforeEach, onTestFailed, TestContext } from "vitest";
1212
import { CopilotClient, CopilotClientOptions } from "../../../src";
1313
import { CapiProxy } from "./CapiProxy";
14-
import { retry } from "./sdkTestHelper";
14+
import { retry, formatError } from "./sdkTestHelper";
1515

1616
export const isCI = process.env.GITHUB_ACTIONS === "true";
1717

@@ -111,8 +111,17 @@ function getTrafficCapturePath(testContext: TestContext): string {
111111
return join(SNAPSHOTS_DIR, testFileName, `${taskNameAsFilename}.yaml`);
112112
}
113113

114-
function rmDir(message: string, path: string): Promise<void> {
115-
// Use longer retries (30s total) to tolerate Windows holding SQLite
116-
// session-store.db open briefly after the CLI subprocess exits.
117-
return retry(message, () => rm(path, { recursive: true, force: true }), 30, 1000);
114+
async function rmDir(message: string, path: string): Promise<void> {
115+
// Use longer retries to tolerate Windows holding SQLite session-store.db
116+
// open briefly after the CLI subprocess exits. If the temp dir still can't
117+
// be removed (e.g. CLI background writer racing with cleanup), warn and
118+
// continue rather than failing the whole test run — the OS / CI runner
119+
// will reclaim the temp dir on shutdown.
120+
try {
121+
await retry(message, () => rm(path, { recursive: true, force: true }), 30, 1000);
122+
} catch (error) {
123+
console.warn(
124+
`WARN: ${message} failed; leaving temp dir for OS cleanup: ${formatError(error)}`
125+
);
126+
}
118127
}

python/e2e/test_commands.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
"""
88

99
import asyncio
10+
import contextlib
1011
import os
1112
import shutil
1213
import tempfile
@@ -106,7 +107,8 @@ async def configure_for_test(self, test_file: str, test_name: str):
106107
if item.is_dir():
107108
shutil.rmtree(item, ignore_errors=True)
108109
else:
109-
item.unlink(missing_ok=True)
110+
with contextlib.suppress(OSError):
111+
item.unlink(missing_ok=True)
110112

111113
def _get_env(self) -> dict:
112114
env = os.environ.copy()

python/e2e/test_multi_client.py

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
"""
66

77
import asyncio
8+
import contextlib
89
import os
910
import shutil
1011
import tempfile
@@ -110,19 +111,17 @@ async def configure_for_test(self, test_file: str, test_name: str):
110111
if self._proxy:
111112
await self._proxy.configure(abs_snapshot_path, self.work_dir)
112113

113-
# Clear temp directories between tests
114+
# Clear temp directories between tests; tolerate Windows holding the
115+
# SQLite session-store.db open briefly after the CLI subprocess exits.
114116
from pathlib import Path
115117

116-
for item in Path(self.home_dir).iterdir():
117-
if item.is_dir():
118-
shutil.rmtree(item, ignore_errors=True)
119-
else:
120-
item.unlink(missing_ok=True)
121-
for item in Path(self.work_dir).iterdir():
122-
if item.is_dir():
123-
shutil.rmtree(item, ignore_errors=True)
124-
else:
125-
item.unlink(missing_ok=True)
118+
for base_dir in (self.home_dir, self.work_dir):
119+
for item in Path(base_dir).iterdir():
120+
if item.is_dir():
121+
shutil.rmtree(item, ignore_errors=True)
122+
else:
123+
with contextlib.suppress(OSError):
124+
item.unlink(missing_ok=True)
126125

127126
def get_env(self) -> dict:
128127
env = os.environ.copy()

python/e2e/test_ui_elicitation_multi_client.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
"""
99

1010
import asyncio
11+
import contextlib
1112
import os
1213
import shutil
1314
import tempfile
@@ -113,7 +114,8 @@ async def configure_for_test(self, test_file: str, test_name: str):
113114
if item.is_dir():
114115
shutil.rmtree(item, ignore_errors=True)
115116
else:
116-
item.unlink(missing_ok=True)
117+
with contextlib.suppress(OSError):
118+
item.unlink(missing_ok=True)
117119

118120
def _get_env(self) -> dict:
119121
env = os.environ.copy()

0 commit comments

Comments
 (0)