conductor

commit 248bd2fd6de9fd7d522e73b19101f1ffca50cd0a
parent d0e35738733d0f1b0a88d4bb5586f26a129ce029
Author: finwo <finwo@pm.me>
Date:   Sat, 19 Sep 2026 18:18:07 +0200

Stop test helpers from killing each other's containers

Diffstat:
A
test/helpers/containers.js
 | 
57
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++
M
test/helpers/minio.js
 | 
15
++-------------
M
test/helpers/oidc.js
 | 
19
++-----------------

3 files changed, 61 insertions(+), 30 deletions(-)
diff --git a/test/helpers/containers.js b/test/helpers/containers.js
@@ -0,0 +1,57 @@
+// test/helpers/containers.js - cleaning up after runs that crashed
+//
+// A test run that is killed never reaches its cleanup hook and leaves its
+// containers behind, so each helper tidies up before it starts.
+//
+// Doing that by name prefix alone is wrong, and was: node --test runs
+// test files in parallel processes, two files each start their own MinIO,
+// and whichever starts second force-removes the container the first is
+// using. The first then waits out its whole timeout and fails with the
+// container gone from under it. That showed up as a flake under load,
+// which is exactly the wrong thing to go looking for.
+//
+// So age is the thing that makes a container stale, not its name. A
+// sibling that started seconds ago is left alone; a leftover from a run
+// that died half an hour ago is removed.
+
+import { execFile } from 'node:child_process';
+import { promisify } from 'node:util';
+
+const execFileAsync = promisify(execFile);
+
+// Comfortably longer than any test takes, comfortably shorter than the
+// gap between one run and the next.
+export const STALE_AGE = 30 * 60 * 1000;
+
+export async function reapStale(prefix, maxAgeMs = STALE_AGE) {
+  try {
+    const { stdout } = await execFileAsync(
+      'docker', ['ps', '-aq', '--filter', `name=${prefix}`], { timeout: 30000 },
+    );
+    const ids = stdout.trim().split('\n').filter(Boolean);
+    if (ids.length === 0) return;
+
+    // One call for all of them; .Created is RFC3339 and parses directly.
+    const { stdout: detail } = await execFileAsync(
+      'docker', ['inspect', '--format', '{{.Id}} {{.Created}}', ...ids], { timeout: 30000 },
+    );
+
+    const cutoff = Date.now() - maxAgeMs;
+    const stale = detail.trim().split('\n')
+      .map((line) => line.split(' '))
+      .filter(([, created]) => {
+        const at = Date.parse(created);
+        // A timestamp that will not parse is not grounds for deleting
+        // somebody's running container.
+        return Number.isFinite(at) && at < cutoff;
+      })
+      .map(([id]) => id);
+
+    if (stale.length > 0) {
+      await execFileAsync('docker', ['rm', '-f', ...stale], { timeout: 60000 });
+    }
+  } catch {
+    // Best effort. A failure here must not fail the tests, and whatever
+    // is wrong will be reported more usefully by the start that follows.
+  }
+}
diff --git a/test/helpers/minio.js b/test/helpers/minio.js
@@ -13,6 +13,7 @@ import { promisify } from 'node:util';
 import crypto from 'node:crypto';
 import net from 'node:net';
 import { signRequest, sha256Hex } from '../../src/lib/storage/sigv4.js';
+import { reapStale } from './containers.js';
 
 const execFileAsync = promisify(execFile);
 
@@ -62,18 +63,6 @@ async function freePort() {
   });
 }
 
-// A run that crashes never reaches its cleanup hook. Anything still
-// carrying the prefix is from a previous run and can go.
-async function reapStale() {
-  try {
-    const { stdout } = await execFileAsync('docker', ['ps', '-aq', '--filter', `name=${NAME_PREFIX}`], { timeout: 30000 });
-    const ids = stdout.trim().split('\n').filter(Boolean);
-    if (ids.length > 0) await execFileAsync('docker', ['rm', '-f', ...ids], { timeout: 60000 });
-  } catch {
-    // Best effort; a failure here must not fail the tests.
-  }
-}
-
 // Buckets are created with a signed request rather than the MinIO client,
 // which keeps the helper free of another dependency and exercises the
 // signer on the way in.
@@ -117,7 +106,7 @@ export async function startMinio() {
     };
   }
 
-  await reapStale();
+  await reapStale(NAME_PREFIX);
 
   const port = await freePort();
   const name = `${NAME_PREFIX}${crypto.randomBytes(4).toString('hex')}`;
diff --git a/test/helpers/oidc.js b/test/helpers/oidc.js
@@ -11,6 +11,7 @@
 import { execFile } from 'node:child_process';
 import { promisify } from 'node:util';
 import crypto from 'node:crypto';
+import { reapStale } from './containers.js';
 
 const execFileAsync = promisify(execFile);
 
@@ -105,24 +106,8 @@ const READY_TIMEOUT = 120 * 1000;
 
 const NAME_PREFIX = 'conductor-oidc-test-';
 
-// A test run that crashes hard, or is killed, never reaches its cleanup
-// hook and leaves the container behind. Since exactly one provider is
-// needed at a time, anything already carrying the prefix is stale and can
-// go. Cheaper than asking people to clean up after a failed run.
-async function reapStale() {
-  try {
-    const { stdout } = await execFileAsync('docker', [
-      'ps', '-aq', '--filter', `name=${NAME_PREFIX}`,
-    ], { timeout: 30000 });
-    const ids = stdout.trim().split('\n').filter(Boolean);
-    if (ids.length > 0) await execFileAsync('docker', ['rm', '-f', ...ids], { timeout: 60000 });
-  } catch {
-    // Best effort; a failure here must not fail the tests.
-  }
-}
-
 export async function startOidc() {
-  await reapStale();
+  await reapStale(NAME_PREFIX);
 
   const port = await freePort();
   const name = `${NAME_PREFIX}${crypto.randomBytes(4).toString('hex')}`;