From 2fcd7110949776db7658d86fa5b031106efedc5e Mon Sep 17 00:00:00 2001 From: Daniel Bodart Date: Sat, 5 Apr 2025 10:25:22 +0100 Subject: [PATCH 01/10] Simplified log-wait-strategy, switch from callback to async, separated concerns, handled more timing edge cases. More details in the pull request --- .../src/wait-strategies/log-wait-strategy.ts | 64 ++++++++----------- 1 file changed, 26 insertions(+), 38 deletions(-) diff --git a/packages/testcontainers/src/wait-strategies/log-wait-strategy.ts b/packages/testcontainers/src/wait-strategies/log-wait-strategy.ts index 005ef113b..a95237fd6 100644 --- a/packages/testcontainers/src/wait-strategies/log-wait-strategy.ts +++ b/packages/testcontainers/src/wait-strategies/log-wait-strategy.ts @@ -1,5 +1,5 @@ -import byline from "byline"; import Dockerode from "dockerode"; +import { setTimeout } from "timers/promises"; import { log } from "../common"; import { getContainerRuntimeClient } from "../container-runtime"; import { BoundPorts } from "../utils/bound-ports"; @@ -16,46 +16,34 @@ export class LogWaitStrategy extends AbstractWaitStrategy { } public async waitUntilReady(container: Dockerode.Container, boundPorts: BoundPorts, startTime?: Date): Promise { + await Promise.race([this.handleTimeout(container.id), this.handleLogs(container, startTime)]); + } + + async handleTimeout(containerId: string): Promise { + await setTimeout(this.startupTimeout); + const message = `Log message "${this.message}" not received after ${this.startupTimeout}ms`; + log.error(message, { containerId }); + throw new Error(message); + } + + async handleLogs(container: Dockerode.Container, startTime?: Date): Promise { log.debug(`Waiting for log message "${this.message}"...`, { containerId: container.id }); const client = await getContainerRuntimeClient(); const stream = await client.container.logs(container, { since: startTime ? startTime.getTime() / 1000 : 0 }); - return new Promise((resolve, reject) => { - const timeout = setTimeout(() => { - const message = `Log message "${this.message}" not received after ${this.startupTimeout}ms`; - log.error(message, { containerId: container.id }); - reject(new Error(message)); - }, this.startupTimeout); - - const comparisonFn: (line: string) => boolean = (line: string) => { - if (this.message instanceof RegExp) { - return this.message.test(line); - } else { - return line.includes(this.message); - } - }; - - let count = 0; - const lineProcessor = (line: string) => { - if (comparisonFn(line)) { - if (++count === this.times) { - stream.destroy(); - clearTimeout(timeout); - log.debug(`Log wait strategy complete`, { containerId: container.id }); - resolve(); - } + + let matches = 0; + for await (const chunk of stream) { + if (this.matches(chunk)) { + if (++matches === this.times) { + return log.debug(`Log wait strategy complete`, { containerId: container.id }); } - }; - - byline(stream) - .on("data", lineProcessor) - .on("err", lineProcessor) - .on("end", () => { - stream.destroy(); - clearTimeout(timeout); - const message = `Log stream ended and message "${this.message}" was not received`; - log.error(message, { containerId: container.id }); - reject(new Error(message)); - }); - }); + } + } + + throw new Error("Log message not found"); + } + + matches(chunk: string): boolean { + return this.message instanceof RegExp ? this.message.test(chunk) : chunk.includes(this.message); } } From c02ea692eb7c55329984a665f80541f1e3829c56 Mon Sep 17 00:00:00 2001 From: Daniel Bodart Date: Sat, 5 Apr 2025 21:31:55 +0100 Subject: [PATCH 02/10] Restored original message as that was not meant to have changed. Added test to for this case to LogWaitrStrategy --- .../wait-strategies/log-wait-strategy.test.ts | 17 ++++++++++++++++- .../src/wait-strategies/log-wait-strategy.ts | 15 +++++++++------ 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/packages/testcontainers/src/wait-strategies/log-wait-strategy.test.ts b/packages/testcontainers/src/wait-strategies/log-wait-strategy.test.ts index 62e537f0c..e4d57c66e 100644 --- a/packages/testcontainers/src/wait-strategies/log-wait-strategy.test.ts +++ b/packages/testcontainers/src/wait-strategies/log-wait-strategy.test.ts @@ -3,7 +3,7 @@ import { GenericContainer } from "../generic-container/generic-container"; import { checkContainerIsHealthy, getRunningContainerNames } from "../utils/test-helper"; import { Wait } from "./wait"; -describe("LogWaitStrategy", { timeout: 180_000 }, () => { +describe("LogWaitStrategy", { timeout: 5_000 }, () => { it("should wait for log", async () => { const container = await new GenericContainer("cristianrgreco/testcontainer:1.1.14") .withExposedPorts(8080) @@ -54,4 +54,19 @@ describe("LogWaitStrategy", { timeout: 180_000 }, () => { expect(await getRunningContainerNames()).not.toContain(containerName); }); + + + it("should throw an error if the message is never received", async () => { + const containerName = `container-${new RandomUuid().nextUuid()}`; + + await expect( + new GenericContainer("cristianrgreco/testcontainer:1.1.14") + .withName(containerName) + .withCommand("/bin/sh", "-c", 'echo "Ready"') + .withWaitStrategy(Wait.forLogMessage("unexpected")) + .start() + ).rejects.toThrowError(`Log stream ended and message "unexpected" was not received`); + + expect(await getRunningContainerNames()).not.toContain(containerName); + }); }); diff --git a/packages/testcontainers/src/wait-strategies/log-wait-strategy.ts b/packages/testcontainers/src/wait-strategies/log-wait-strategy.ts index a95237fd6..6f9e07a49 100644 --- a/packages/testcontainers/src/wait-strategies/log-wait-strategy.ts +++ b/packages/testcontainers/src/wait-strategies/log-wait-strategy.ts @@ -10,7 +10,7 @@ export type Log = string; export class LogWaitStrategy extends AbstractWaitStrategy { constructor( private readonly message: Log | RegExp, - private readonly times: number + private readonly times: number, ) { super(); } @@ -21,9 +21,7 @@ export class LogWaitStrategy extends AbstractWaitStrategy { async handleTimeout(containerId: string): Promise { await setTimeout(this.startupTimeout); - const message = `Log message "${this.message}" not received after ${this.startupTimeout}ms`; - log.error(message, { containerId }); - throw new Error(message); + this.throwError(containerId, `Log message "${this.message}" not received after ${this.startupTimeout}ms`); } async handleLogs(container: Dockerode.Container, startTime?: Date): Promise { @@ -40,10 +38,15 @@ export class LogWaitStrategy extends AbstractWaitStrategy { } } - throw new Error("Log message not found"); + this.throwError(container.id, `Log stream ended and message "${this.message}" was not received`); } matches(chunk: string): boolean { return this.message instanceof RegExp ? this.message.test(chunk) : chunk.includes(this.message); } -} + + throwError(containerId: string, message: string): void { + log.error(message, { containerId }); + throw new Error(message); + } +} \ No newline at end of file From 843721e368db8f76a41cf24cd69092d6421a8d92 Mon Sep 17 00:00:00 2001 From: Daniel Bodart Date: Sat, 5 Apr 2025 21:35:08 +0100 Subject: [PATCH 03/10] =?UTF-8?q?Revert=20timeout=20to=20original=20value?= =?UTF-8?q?=C2=A3?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/wait-strategies/log-wait-strategy.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/testcontainers/src/wait-strategies/log-wait-strategy.test.ts b/packages/testcontainers/src/wait-strategies/log-wait-strategy.test.ts index e4d57c66e..70d2e6261 100644 --- a/packages/testcontainers/src/wait-strategies/log-wait-strategy.test.ts +++ b/packages/testcontainers/src/wait-strategies/log-wait-strategy.test.ts @@ -3,7 +3,7 @@ import { GenericContainer } from "../generic-container/generic-container"; import { checkContainerIsHealthy, getRunningContainerNames } from "../utils/test-helper"; import { Wait } from "./wait"; -describe("LogWaitStrategy", { timeout: 5_000 }, () => { +describe("LogWaitStrategy", { timeout: 180_000 }, () => { it("should wait for log", async () => { const container = await new GenericContainer("cristianrgreco/testcontainer:1.1.14") .withExposedPorts(8080) From 6ef481977360c980816c420bb31f5030c9bf14fa Mon Sep 17 00:00:00 2001 From: Daniel Bodart Date: Sat, 5 Apr 2025 21:53:54 +0100 Subject: [PATCH 04/10] Add test for logs that is not sent in a single line --- .../wait-strategies/log-wait-strategy.test.ts | 19 +++++++------------ .../src/wait-strategies/log-wait-strategy.ts | 1 + 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/packages/testcontainers/src/wait-strategies/log-wait-strategy.test.ts b/packages/testcontainers/src/wait-strategies/log-wait-strategy.test.ts index 70d2e6261..fefae106d 100644 --- a/packages/testcontainers/src/wait-strategies/log-wait-strategy.test.ts +++ b/packages/testcontainers/src/wait-strategies/log-wait-strategy.test.ts @@ -56,17 +56,12 @@ describe("LogWaitStrategy", { timeout: 180_000 }, () => { }); - it("should throw an error if the message is never received", async () => { - const containerName = `container-${new RandomUuid().nextUuid()}`; - - await expect( - new GenericContainer("cristianrgreco/testcontainer:1.1.14") - .withName(containerName) - .withCommand("/bin/sh", "-c", 'echo "Ready"') - .withWaitStrategy(Wait.forLogMessage("unexpected")) - .start() - ).rejects.toThrowError(`Log stream ended and message "unexpected" was not received`); + it("does not matter if container does not send all content in a single line", async () => { + const container = await new GenericContainer("cristianrgreco/testcontainer:1.1.14") + .withCommand(["node", "-e", "process.stdout.write('Hello '); setTimeout(() => process.stdout.write('World\\n'), 2000)"]) + .withWaitStrategy(Wait.forLogMessage("Hello World")) + .start(); - expect(await getRunningContainerNames()).not.toContain(containerName); + await container.stop(); }); -}); +}); \ No newline at end of file diff --git a/packages/testcontainers/src/wait-strategies/log-wait-strategy.ts b/packages/testcontainers/src/wait-strategies/log-wait-strategy.ts index 6f9e07a49..95d9ab0de 100644 --- a/packages/testcontainers/src/wait-strategies/log-wait-strategy.ts +++ b/packages/testcontainers/src/wait-strategies/log-wait-strategy.ts @@ -31,6 +31,7 @@ export class LogWaitStrategy extends AbstractWaitStrategy { let matches = 0; for await (const chunk of stream) { + console.log(chunk) if (this.matches(chunk)) { if (++matches === this.times) { return log.debug(`Log wait strategy complete`, { containerId: container.id }); From 1c126a91a861852f9c743152e3d6b8a306592927 Mon Sep 17 00:00:00 2001 From: Daniel Bodart Date: Sat, 5 Apr 2025 21:57:28 +0100 Subject: [PATCH 05/10] Restore second test --- .../wait-strategies/log-wait-strategy.test.ts | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/packages/testcontainers/src/wait-strategies/log-wait-strategy.test.ts b/packages/testcontainers/src/wait-strategies/log-wait-strategy.test.ts index fefae106d..b5002fd7b 100644 --- a/packages/testcontainers/src/wait-strategies/log-wait-strategy.test.ts +++ b/packages/testcontainers/src/wait-strategies/log-wait-strategy.test.ts @@ -56,6 +56,20 @@ describe("LogWaitStrategy", { timeout: 180_000 }, () => { }); + it("should throw an error if the message is never received", async () => { + const containerName = `container-${new RandomUuid().nextUuid()}`; + + await expect( + new GenericContainer("cristianrgreco/testcontainer:1.1.14") + .withName(containerName) + .withCommand("/bin/sh", "-c", 'echo "Ready"') + .withWaitStrategy(Wait.forLogMessage("unexpected")) + .start() + ).rejects.toThrowError(`Log stream ended and message "unexpected" was not received`); + + expect(await getRunningContainerNames()).not.toContain(containerName); + }); + it("does not matter if container does not send all content in a single line", async () => { const container = await new GenericContainer("cristianrgreco/testcontainer:1.1.14") .withCommand(["node", "-e", "process.stdout.write('Hello '); setTimeout(() => process.stdout.write('World\\n'), 2000)"]) @@ -64,4 +78,4 @@ describe("LogWaitStrategy", { timeout: 180_000 }, () => { await container.stop(); }); -}); \ No newline at end of file +}); From 621ff391cb0394fdecc68b144d9c59478b4a0087 Mon Sep 17 00:00:00 2001 From: Daniel Bodart Date: Sat, 5 Apr 2025 21:59:27 +0100 Subject: [PATCH 06/10] Remove test log --- packages/testcontainers/src/wait-strategies/log-wait-strategy.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/testcontainers/src/wait-strategies/log-wait-strategy.ts b/packages/testcontainers/src/wait-strategies/log-wait-strategy.ts index 95d9ab0de..6f9e07a49 100644 --- a/packages/testcontainers/src/wait-strategies/log-wait-strategy.ts +++ b/packages/testcontainers/src/wait-strategies/log-wait-strategy.ts @@ -31,7 +31,6 @@ export class LogWaitStrategy extends AbstractWaitStrategy { let matches = 0; for await (const chunk of stream) { - console.log(chunk) if (this.matches(chunk)) { if (++matches === this.times) { return log.debug(`Log wait strategy complete`, { containerId: container.id }); From d994641ab00407384c9743e824242b7935db3794 Mon Sep 17 00:00:00 2001 From: Daniel Bodart Date: Sat, 5 Apr 2025 22:29:50 +0100 Subject: [PATCH 07/10] Linted and formatted --- .../src/wait-strategies/log-wait-strategy.test.ts | 7 +++++-- .../src/wait-strategies/log-wait-strategy.ts | 4 ++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/packages/testcontainers/src/wait-strategies/log-wait-strategy.test.ts b/packages/testcontainers/src/wait-strategies/log-wait-strategy.test.ts index b5002fd7b..57961c1ea 100644 --- a/packages/testcontainers/src/wait-strategies/log-wait-strategy.test.ts +++ b/packages/testcontainers/src/wait-strategies/log-wait-strategy.test.ts @@ -55,7 +55,6 @@ describe("LogWaitStrategy", { timeout: 180_000 }, () => { expect(await getRunningContainerNames()).not.toContain(containerName); }); - it("should throw an error if the message is never received", async () => { const containerName = `container-${new RandomUuid().nextUuid()}`; @@ -72,7 +71,11 @@ describe("LogWaitStrategy", { timeout: 180_000 }, () => { it("does not matter if container does not send all content in a single line", async () => { const container = await new GenericContainer("cristianrgreco/testcontainer:1.1.14") - .withCommand(["node", "-e", "process.stdout.write('Hello '); setTimeout(() => process.stdout.write('World\\n'), 2000)"]) + .withCommand([ + "node", + "-e", + "process.stdout.write('Hello '); setTimeout(() => process.stdout.write('World\\n'), 2000)", + ]) .withWaitStrategy(Wait.forLogMessage("Hello World")) .start(); diff --git a/packages/testcontainers/src/wait-strategies/log-wait-strategy.ts b/packages/testcontainers/src/wait-strategies/log-wait-strategy.ts index 6f9e07a49..03a2966cc 100644 --- a/packages/testcontainers/src/wait-strategies/log-wait-strategy.ts +++ b/packages/testcontainers/src/wait-strategies/log-wait-strategy.ts @@ -10,7 +10,7 @@ export type Log = string; export class LogWaitStrategy extends AbstractWaitStrategy { constructor( private readonly message: Log | RegExp, - private readonly times: number, + private readonly times: number ) { super(); } @@ -49,4 +49,4 @@ export class LogWaitStrategy extends AbstractWaitStrategy { log.error(message, { containerId }); throw new Error(message); } -} \ No newline at end of file +} From 4bd72c9898c38135f2f89603c1e13cc4ef24bdda Mon Sep 17 00:00:00 2001 From: Daniel Bodart Date: Sat, 5 Apr 2025 23:13:23 +0100 Subject: [PATCH 08/10] Readded byline as you can just wrap the stream directly --- .../testcontainers/src/wait-strategies/log-wait-strategy.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/testcontainers/src/wait-strategies/log-wait-strategy.ts b/packages/testcontainers/src/wait-strategies/log-wait-strategy.ts index 03a2966cc..fafc70af9 100644 --- a/packages/testcontainers/src/wait-strategies/log-wait-strategy.ts +++ b/packages/testcontainers/src/wait-strategies/log-wait-strategy.ts @@ -4,6 +4,7 @@ import { log } from "../common"; import { getContainerRuntimeClient } from "../container-runtime"; import { BoundPorts } from "../utils/bound-ports"; import { AbstractWaitStrategy } from "./wait-strategy"; +import byline from "byline"; export type Log = string; @@ -30,8 +31,8 @@ export class LogWaitStrategy extends AbstractWaitStrategy { const stream = await client.container.logs(container, { since: startTime ? startTime.getTime() / 1000 : 0 }); let matches = 0; - for await (const chunk of stream) { - if (this.matches(chunk)) { + for await (const line of byline(stream)) { + if (this.matches(line)) { if (++matches === this.times) { return log.debug(`Log wait strategy complete`, { containerId: container.id }); } From 1b0bed95a225f284af039590c99cf0ac833daa79 Mon Sep 17 00:00:00 2001 From: Daniel Bodart Date: Sun, 6 Apr 2025 08:34:52 +0100 Subject: [PATCH 09/10] Minor clean up of param name --- .../testcontainers/src/wait-strategies/log-wait-strategy.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/testcontainers/src/wait-strategies/log-wait-strategy.ts b/packages/testcontainers/src/wait-strategies/log-wait-strategy.ts index fafc70af9..5c3551be2 100644 --- a/packages/testcontainers/src/wait-strategies/log-wait-strategy.ts +++ b/packages/testcontainers/src/wait-strategies/log-wait-strategy.ts @@ -42,8 +42,8 @@ export class LogWaitStrategy extends AbstractWaitStrategy { this.throwError(container.id, `Log stream ended and message "${this.message}" was not received`); } - matches(chunk: string): boolean { - return this.message instanceof RegExp ? this.message.test(chunk) : chunk.includes(this.message); + matches(line: string): boolean { + return this.message instanceof RegExp ? this.message.test(line) : line.includes(this.message); } throwError(containerId: string, message: string): void { From f5231e160ec1b60ea82d1c206e0c6fd614a98da7 Mon Sep 17 00:00:00 2001 From: Cristian Greco Date: Sun, 6 Apr 2025 17:35:57 +0100 Subject: [PATCH 10/10] Fix lint --- .../testcontainers/src/wait-strategies/log-wait-strategy.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/testcontainers/src/wait-strategies/log-wait-strategy.ts b/packages/testcontainers/src/wait-strategies/log-wait-strategy.ts index 5c3551be2..a54c3f928 100644 --- a/packages/testcontainers/src/wait-strategies/log-wait-strategy.ts +++ b/packages/testcontainers/src/wait-strategies/log-wait-strategy.ts @@ -1,10 +1,10 @@ +import byline from "byline"; import Dockerode from "dockerode"; import { setTimeout } from "timers/promises"; import { log } from "../common"; import { getContainerRuntimeClient } from "../container-runtime"; import { BoundPorts } from "../utils/bound-ports"; import { AbstractWaitStrategy } from "./wait-strategy"; -import byline from "byline"; export type Log = string;