fix(e2e): surface cleanup HTTP failures instead of swallowing them
CleanupRegistry's catch-all was masking every kind of teardown error, not just the intended "resource already gone" 404. A backend returning 500 on delete would leak orphans run after run without ever surfacing. Now treat 2xx and 404 as success, log any other status (and any thrown network error) to stderr with the resource label, and keep running the remaining items. The suite stays best-effort but no longer hides accumulating leaks. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -3,28 +3,40 @@ import { adminApi } from './api';
|
||||
|
||||
// Resources to delete after a test, in LIFO order. Tests register
|
||||
// their creations and the registry tears everything down in
|
||||
// `cleanupRegistered` — typically called from `test.afterEach`.
|
||||
// `run()` — typically called from `test.afterEach`.
|
||||
//
|
||||
// A non-2xx status (other than 404) is treated as a real failure and
|
||||
// logged to stderr. The previous shape silently swallowed every
|
||||
// error, so a backend that started returning 500 on cleanup would
|
||||
// have leaked orphans invisibly across runs. 404 stays tolerated —
|
||||
// the test may have already deleted the resource itself.
|
||||
|
||||
type Cleanup = (api: APIRequestContext) => Promise<void>;
|
||||
interface CleanupItem {
|
||||
label: string;
|
||||
path: string;
|
||||
}
|
||||
|
||||
export class CleanupRegistry {
|
||||
private items: Cleanup[] = [];
|
||||
private items: CleanupItem[] = [];
|
||||
|
||||
app(slugOrId: string): void {
|
||||
this.items.push(async (api) => {
|
||||
await api.delete(`/api/v1/admin/apps/${encodeURIComponent(slugOrId)}?force=true`);
|
||||
this.items.push({
|
||||
label: `app=${slugOrId}`,
|
||||
path: `/api/v1/admin/apps/${encodeURIComponent(slugOrId)}?force=true`
|
||||
});
|
||||
}
|
||||
|
||||
adminUser(userId: string): void {
|
||||
this.items.push(async (api) => {
|
||||
await api.delete(`/api/v1/admin/admins/${userId}`);
|
||||
this.items.push({
|
||||
label: `admin=${userId}`,
|
||||
path: `/api/v1/admin/admins/${userId}`
|
||||
});
|
||||
}
|
||||
|
||||
apiKey(keyId: string): void {
|
||||
this.items.push(async (api) => {
|
||||
await api.delete(`/api/v1/admin/api-keys/${keyId}`);
|
||||
this.items.push({
|
||||
label: `key=${keyId}`,
|
||||
path: `/api/v1/admin/api-keys/${keyId}`
|
||||
});
|
||||
}
|
||||
|
||||
@@ -36,12 +48,7 @@ export class CleanupRegistry {
|
||||
// caller that inspects the registry after a partial
|
||||
// teardown) doesn't see the items in a re-reversed order.
|
||||
for (const item of [...this.items].reverse()) {
|
||||
try {
|
||||
await item(api);
|
||||
} catch {
|
||||
// Best-effort cleanup — a missing resource (already
|
||||
// deleted by the test) shouldn't fail the suite.
|
||||
}
|
||||
await deleteAndReport(api, item);
|
||||
}
|
||||
} finally {
|
||||
await api.dispose();
|
||||
@@ -49,3 +56,22 @@ export class CleanupRegistry {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
async function deleteAndReport(
|
||||
api: APIRequestContext,
|
||||
item: CleanupItem
|
||||
): Promise<void> {
|
||||
try {
|
||||
const res = await api.delete(item.path);
|
||||
// 2xx and 404 are both "this resource is no longer here" — fine.
|
||||
if (!res.ok() && res.status() !== 404) {
|
||||
console.warn(
|
||||
`[cleanup] ${item.label} failed: HTTP ${res.status()} ${await res.text()}`
|
||||
);
|
||||
}
|
||||
} catch (err) {
|
||||
// Network-level failure (request never reached the server,
|
||||
// timeout, etc.). Log so a leak doesn't accumulate silently.
|
||||
console.warn(`[cleanup] ${item.label} failed: ${(err as Error).message}`);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user