Skip to content

Commit ebb222d

Browse files
committed
test: harden fixture cleanup
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: edcaaa35-9a42-4351-98fa-55bcf8f1968e
1 parent 6764006 commit ebb222d

1 file changed

Lines changed: 53 additions & 13 deletions

File tree

src/test/integration/environmentFixture.ts

Lines changed: 53 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import { waitForCondition } from '../testUtils';
1717
const OWNERSHIP_FILE_NAME = '.python-envs-test-owner.json';
1818
const COMMAND_TIMEOUT_MS = 180_000;
1919
const DISCOVERY_TIMEOUT_MS = 60_000;
20+
const API_REMOVAL_SETTLE_TIMEOUT_MS = 10_000;
2021

2122
export interface EnvironmentFixtureProvider {
2223
readonly environmentDirectory: string;
@@ -74,16 +75,13 @@ export async function createEnvironmentFixture(
7475
let projectSettingAdded = false;
7576
let environmentCreated = false;
7677
let environment: PythonEnvironment | undefined;
77-
let disposed = false;
78+
let disposePromise: Promise<void> | undefined;
7879
let markerWritten = false;
7980
let projectRootCreated = false;
8081

81-
const dispose = async (): Promise<void> => {
82-
if (disposed) {
83-
return;
84-
}
85-
disposed = true;
82+
const cleanup = async (): Promise<void> => {
8683
const cleanupErrors: Error[] = [];
84+
let environmentRemovalPending = false;
8785

8886
if (markerWritten && (environmentCreated || (await pathExists(prefix)))) {
8987
let ownershipVerified = false;
@@ -94,27 +92,59 @@ export async function createEnvironmentFixture(
9492
cleanupErrors.push(toError(error));
9593
}
9694
if (ownershipVerified && environment) {
95+
let apiRemovalSettled = false;
96+
const apiRemoval = api
97+
.removeEnvironment(environment, { runHeadless: true })
98+
.finally(() => {
99+
apiRemovalSettled = true;
100+
});
97101
try {
98102
await withTimeout(
99-
api.removeEnvironment(environment, { runHeadless: true }),
103+
apiRemoval,
100104
COMMAND_TIMEOUT_MS,
101105
`${request.name} API environment removal timed out`,
102106
);
103107
} catch (error) {
104108
cleanupErrors.push(toError(error));
109+
if (!apiRemovalSettled) {
110+
try {
111+
await withTimeout(
112+
apiRemoval,
113+
API_REMOVAL_SETTLE_TIMEOUT_MS,
114+
`${request.name} API environment removal did not settle after timing out`,
115+
);
116+
} catch (settleError) {
117+
if (apiRemovalSettled) {
118+
cleanupErrors.push(toError(settleError));
119+
} else {
120+
environmentRemovalPending = true;
121+
cleanupErrors.push(
122+
new Error(
123+
`${request.name} direct cleanup was skipped because API removal is still running`,
124+
),
125+
);
126+
}
127+
}
128+
}
105129
}
106130
}
107131
if (ownershipVerified && (await pathExists(prefix))) {
108132
cleanupErrors.push(
109133
new Error(`${request.name} API removal left the environment on disk: ${prefix.fsPath}`),
110134
);
111-
try {
112-
await request.provider.remove(prefix);
113-
await assertPathMissing(prefix, `${request.name} environment was not removed`);
114-
} catch (error) {
115-
cleanupErrors.push(toError(error));
135+
if (!environmentRemovalPending) {
136+
try {
137+
await request.provider.remove(prefix);
138+
await assertPathMissing(prefix, `${request.name} environment was not removed`);
139+
} catch (error) {
140+
cleanupErrors.push(toError(error));
141+
}
116142
}
117143
}
144+
if (ownershipVerified && !(await pathExists(prefix))) {
145+
environmentCreated = false;
146+
environment = undefined;
147+
}
118148
}
119149

120150
try {
@@ -143,7 +173,7 @@ export async function createEnvironmentFixture(
143173
}
144174
}
145175

146-
if (projectRootCreated && (await pathExists(projectUri))) {
176+
if (!environmentRemovalPending && projectRootCreated && (await pathExists(projectUri))) {
147177
try {
148178
if (markerWritten) {
149179
await verifyOwnership(projectUri, markerUri, token, prefix);
@@ -161,6 +191,16 @@ export async function createEnvironmentFixture(
161191
}
162192
};
163193

194+
const dispose = (): Promise<void> => {
195+
if (!disposePromise) {
196+
disposePromise = cleanup().catch((error) => {
197+
disposePromise = undefined;
198+
throw error;
199+
});
200+
}
201+
return disposePromise;
202+
};
203+
164204
try {
165205
await assertPathMissing(projectUri, `Fixture directory already exists: ${projectUri.fsPath}`);
166206
await vscode.workspace.fs.createDirectory(projectUri);

0 commit comments

Comments
 (0)