From 0ce54659e78abadd5410af3a9b08c7c40714b977 Mon Sep 17 00:00:00 2001 From: Golo Roden Date: Mon, 19 Oct 2026 17:10:48 +0200 Subject: [PATCH] chore: Wait for the background removal in the test of a container that Docker creates late, instead of polling for a fixed time. The test held the creations back until the start had failed, and then polled for 5 seconds for the containers to disappear, while their removal only runs once testcontainers gives up on them, which may take longer under load. Now the test lets Docker create the containers only after the start has failed, and awaits the removals in the background through a test-only hook of startContainer. The watcher of created containers no longer assumes that Docker's events have arrived by the time the start returns: it creates a marker container and waits for its event, after which all earlier events have arrived. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01QykegdHe7vhHMBN3kWxgfH --- src/container/startContainer.test.ts | 126 ++++++++++++++++----------- src/container/startContainer.ts | 7 +- 2 files changed, 83 insertions(+), 50 deletions(-) diff --git a/src/container/startContainer.test.ts b/src/container/startContainer.test.ts index 1df6d72..f1558c4 100644 --- a/src/container/startContainer.test.ts +++ b/src/container/startContainer.test.ts @@ -5,7 +5,7 @@ import { Readable } from 'node:stream'; import { suite, test } from 'node:test'; import { setTimeout as sleep } from 'node:timers/promises'; import type { StartedTestContainer } from 'testcontainers'; -import { getContainerRuntimeClient } from 'testcontainers'; +import { getContainerRuntimeClient, ImageName } from 'testcontainers'; import { getImageTag } from '../getImageTag.js'; import { failPortLookups } from '../testing/failPortLookups.js'; import { integrationTimeout } from '../testing/integrationTimeout.js'; @@ -14,6 +14,7 @@ import { startContainer } from './startContainer.js'; const startLabel = 'io.eventsourcingdb.container-start'; const testLabel = 'io.eventsourcingdb.client-javascript-test'; +const markerLabel = 'io.eventsourcingdb.client-javascript-test-marker'; const exitedPrefix = 'Failed to start the container, it exited before the database became reachable: '; @@ -42,9 +43,16 @@ const listContainers = async (testId: string): Promise => { // Watches Docker for the containers that it creates for a test, and returns a // function that stops watching and returns the start label of each of them. // Docker keeps only the latest events, so the test watches while it runs, -// instead of asking for its events afterwards. -const watchCreatedContainers = async (testId: string): Promise<() => string[]> => { +// instead of asking for its events afterwards. Docker passes on its events in +// order, so to stop, the test creates a container of its own and waits for its +// event, after which the events of all containers created before have arrived. +const watchCreatedContainers = async ( + testId: string, + image: string, +): Promise<() => Promise> => { const client = await getContainerRuntimeClient(); + await client.image.pull(ImageName.fromString(image)); + const events = await client.container.dockerode.getEvents({ filters: { type: ['container'], event: ['create'], label: [`${testLabel}=${testId}`] }, }); @@ -55,19 +63,37 @@ const watchCreatedContainers = async (testId: string): Promise<() => string[]> = // Lets the test end even if it fails before it stops watching. events.socket.unref(); - const chunks: Buffer[] = []; + const startIds: string[] = []; + const markerArrived = Promise.withResolvers(); + let pending = ''; events.on('data', (chunk: Buffer): void => { - chunks.push(chunk); + const lines = `${pending}${chunk.toString('utf8')}`.split('\n'); + pending = lines.pop() ?? ''; + + for (const line of lines.filter(candidate => candidate !== '')) { + const { Attributes: attributes } = JSON.parse(line).Actor; + if (attributes[markerLabel] === testId) { + markerArrived.resolve(); + } else { + startIds.push(attributes[startLabel]); + } + } }); - return (): string[] => { - events.destroy(); + return async (): Promise => { + const marker = await client.container.create({ + Image: ImageName.fromString(image).string, + Labels: { [testLabel]: testId, [markerLabel]: testId }, + }); + + try { + await markerArrived.promise; + } finally { + events.destroy(); + await client.container.remove(marker, { removeVolumes: true }); + } - return Buffer.concat(chunks) - .toString('utf8') - .split('\n') - .filter(line => line !== '') - .map(line => JSON.parse(line).Actor.Attributes[startLabel]); + return startIds; }; }; @@ -79,7 +105,7 @@ suite('startContainer', () => { integrationTimeout, async (): Promise => { const testId = randomUUID(); - const stopWatching = await watchCreatedContainers(testId); + const stopWatching = await watchCreatedContainers(testId, image); // The database listens on another port than the one that is mapped. await assert.rejects( @@ -97,7 +123,7 @@ suite('startContainer', () => { }, ); - const startIds = stopWatching(); + const startIds = await stopWatching(); assert.equal(startIds.length, 3); assert.equal(new Set(startIds).size, 3); assert.deepEqual(await listContainers(testId), []); @@ -109,7 +135,7 @@ suite('startContainer', () => { integrationTimeout, async (): Promise => { const testId = randomUUID(); - const stopWatching = await watchCreatedContainers(testId); + const stopWatching = await watchCreatedContainers(testId, image); await assert.rejects( startContainer({ @@ -126,7 +152,7 @@ suite('startContainer', () => { }, ); - assert.equal(stopWatching().length, 3); + assert.equal((await stopWatching()).length, 3); assert.deepEqual(await listContainers(testId), []); }, ); @@ -136,7 +162,7 @@ suite('startContainer', () => { integrationTimeout, async (): Promise => { const testId = randomUUID(); - const stopWatching = await watchCreatedContainers(testId); + const stopWatching = await watchCreatedContainers(testId, image); await assert.rejects( startContainer({ @@ -162,7 +188,7 @@ suite('startContainer', () => { }, ); - assert.equal(stopWatching().length, 1); + assert.equal((await stopWatching()).length, 1); assert.deepEqual(await listContainers(testId), []); }, ); @@ -172,7 +198,7 @@ suite('startContainer', () => { integrationTimeout, async (): Promise => { const testId = randomUUID(); - const stopWatching = await watchCreatedContainers(testId); + const stopWatching = await watchCreatedContainers(testId, image); await assert.rejects( startContainer({ @@ -191,7 +217,7 @@ suite('startContainer', () => { }, ); - assert.equal(stopWatching().length, 0); + assert.equal((await stopWatching()).length, 0); assert.deepEqual(await listContainers(testId), []); }, ); @@ -201,7 +227,7 @@ suite('startContainer', () => { integrationTimeout, async (): Promise => { const testId = randomUUID(); - const stopWatching = await watchCreatedContainers(testId); + const stopWatching = await watchCreatedContainers(testId, image); // Docker cannot copy a file into another file. await assert.rejects( @@ -221,7 +247,7 @@ suite('startContainer', () => { }, ); - assert.equal(stopWatching().length, 1); + assert.equal((await stopWatching()).length, 1); assert.deepEqual(await listContainers(testId), []); }, ); @@ -232,7 +258,7 @@ suite('startContainer', () => { async (): Promise => { const testId = randomUUID(); const endFailing = await failPortLookups(1); - const stopWatching = await watchCreatedContainers(testId); + const stopWatching = await watchCreatedContainers(testId, image); let container: StartedTestContainer; try { @@ -249,7 +275,7 @@ suite('startContainer', () => { } try { - const startIds = stopWatching(); + const startIds = await stopWatching(); assert.equal(startIds.length, 2); assert.equal(new Set(startIds).size, 2); assert.equal((await listContainers(testId)).length, 1); @@ -271,7 +297,7 @@ suite('startContainer', () => { // lookup of the port fails, so testcontainers gives up on the container // without removing it. const endFailing = await failPortLookups(Number.POSITIVE_INFINITY); - const stopWatching = await watchCreatedContainers(testId); + const stopWatching = await watchCreatedContainers(testId, image); try { await assert.rejects( @@ -292,7 +318,7 @@ suite('startContainer', () => { endFailing(); } - assert.equal(stopWatching().length, 3); + assert.equal((await stopWatching()).length, 3); assert.deepEqual(await listContainers(testId), []); }, ); @@ -317,7 +343,7 @@ suite('startContainer', () => { await pull.apply(imageClient, parameters); }; - const stopWatching = await watchCreatedContainers(testId); + const stopWatching = await watchCreatedContainers(testId, image); let container: StartedTestContainer; try { @@ -338,7 +364,7 @@ suite('startContainer', () => { // Had the pull counted, the first attempt would have created its // container late, and another attempt one more. - assert.equal(stopWatching().length, 1); + assert.equal((await stopWatching()).length, 1); } finally { await container.stop(); } @@ -356,14 +382,16 @@ suite('startContainer', () => { const { container: containerClient } = client; const { create } = containerClient; const creations: Promise[] = []; + const backgroundRemovals: Promise[] = []; - // Docker takes longer to create a container than the attempt waits for - // it, and testcontainers gives up on it without removing it, since the + // Docker creates no container until the test lets it, and + // testcontainers gives up on a container without removing it, since the // lookup of its port fails. + const letDockerCreate = Promise.withResolvers(); containerClient.create = async ( ...parameters: Parameters ): ReturnType => { - const creation = sleep(4000).then( + const creation = letDockerCreate.promise.then( async (): ReturnType => await create.apply(containerClient, parameters), ); creations.push(creation); @@ -372,7 +400,8 @@ suite('startContainer', () => { const endFailing = await failPortLookups(Number.POSITIVE_INFINITY); try { - const startedAt = Date.now(); + // The attempts give up on the creations, which are still pending, + // once the settle timeout has run out. await assert.rejects( startContainer({ image, @@ -382,27 +411,24 @@ suite('startContainer', () => { startupTimeout: 100, labels: { [testLabel]: testId }, settleTimeout: 300, + onBackgroundRemoval: (removal: Promise): void => { + backgroundRemovals.push(removal); + }, }), { message: 'Failed to start the container, the database did not become reachable within 0.1s in any of 3 containers', }, ); - - // The attempts do not wait for Docker beyond the settle timeout. - assert.ok(Date.now() - startedAt < 4000); - - await Promise.allSettled(creations); assert.equal(creations.length, 3); + assert.equal(backgroundRemovals.length, 3); - let containers = await listContainers(testId); - for (let poll = 0; containers.length > 0 && poll < 50; poll += 1) { - // biome-ignore lint/performance/noAwaitInLoops: Each poll waits for the one before. - await sleep(100); - containers = await listContainers(testId); - } - assert.deepEqual(containers, []); + letDockerCreate.resolve(); + await Promise.all([...creations, ...backgroundRemovals]); + + assert.deepEqual(await listContainers(testId), []); } finally { + letDockerCreate.resolve(); endFailing(); containerClient.create = create; } @@ -428,7 +454,7 @@ suite('startContainer', () => { }, }); const endFailing = await failPortLookups(Number.POSITIVE_INFINITY); - const stopWatching = await watchCreatedContainers(testId); + const stopWatching = await watchCreatedContainers(testId, image); try { await assert.rejects( @@ -455,12 +481,14 @@ suite('startContainer', () => { return true; }, ); - - assert.equal(stopWatching().length, 3); } finally { endFailing(); dockerode.getContainer = getContainer; + } + try { + assert.equal((await stopWatching()).length, 3); + } finally { for (const { Id: id } of await listContainers(testId)) { // biome-ignore lint/performance/noAwaitInLoops: The test removes its containers one after another. await dockerode.getContainer(id).remove({ force: true, v: true }); @@ -513,7 +541,7 @@ suite('startContainer', () => { integrationTimeout, async (): Promise => { const testId = randomUUID(); - const stopWatching = await watchCreatedContainers(testId); + const stopWatching = await watchCreatedContainers(testId, image); const container = await startContainer({ image, @@ -531,7 +559,7 @@ suite('startContainer', () => { assert.equal(response.status, 200); await response.body?.cancel(); - assert.equal(stopWatching().length, 1); + assert.equal((await stopWatching()).length, 1); } finally { await container.stop(); } diff --git a/src/container/startContainer.ts b/src/container/startContainer.ts index 1b72a8b..7e93c52 100644 --- a/src/container/startContainer.ts +++ b/src/container/startContainer.ts @@ -26,6 +26,10 @@ interface StartContainerOptions { startupTimeout: number; labels?: Record; settleTimeout?: number; + + // Receives the removal that runs in the background once the wait for a + // pending start has run out, so that a test can wait for it. + onBackgroundRemoval?: (removal: Promise) => void; } const maximumAttempts = 3; @@ -151,7 +155,8 @@ const attemptToStart = async ( if (!hasSettled) { // The caller does not wait for a start that Docker does not finish, and // the removal collects its own errors. - Promise.allSettled([starting]).then(removeContainers); + const removal = Promise.allSettled([starting]).then(removeContainers); + options.onBackgroundRemoval?.(removal); } if ((deadline.signal.aborted && error === deadline.signal.reason) || isPortError(error)) {