Clean up healthcheck wait strategy coverage

Author: cristianrgreco

docs/features/compose.md

@@ -51,7 +51,7 @@ If you'd like to override the default wait strategy for all services, you can do
 
 ```js
 const environment = await new DockerComposeEnvironment(composeFilePath, composeFile)
-  .withDefaultWaitStrategy(Wait.forHealthCheck())
+  .withDefaultWaitStrategy(Wait.forListeningPorts())
   .up();
 ```
 

docs/features/wait-strategies.md

@@ -71,7 +71,7 @@ const container = await new GenericContainer("alpine")
 
 ## Health check
 
-Wait until the container's health check is successful:
+Explicitly wait until the container's health check is successful. This is optional when the image already defines a health check because Testcontainers uses that as the default wait strategy:
 
 ```js
 const { GenericContainer, Wait } = require("testcontainers");

packages/modules/kafka/src/kafka-container.ts

@@ -61,7 +61,7 @@ export class KafkaContainer extends GenericContainer {
   private zooKeeperHost?: string;
   private zooKeeperPort?: number;
   private saslSslConfig?: SaslSslListenerOptions;
-  private originalWaitinStrategy: WaitStrategy | undefined;
+  private originalWaitStrategy: WaitStrategy | undefined;
 
   constructor(image: string) {
     super(image);
@@ -81,7 +81,7 @@ export class KafkaContainer extends GenericContainer {
       KAFKA_GROUP_INITIAL_REBALANCE_DELAY_MS: "0",
       KAFKA_CONFLUENT_SUPPORT_METRICS_ENABLE: "false",
     });
-    this.originalWaitinStrategy = this.waitStrategy;
+    this.originalWaitStrategy = this.waitStrategy;
   }
 
   public withZooKeeper(host: string, port: number): this {
@@ -138,7 +138,7 @@ export class KafkaContainer extends GenericContainer {
 
     // Change the wait strategy to wait for a log message from a fake starter script
     // so that we can put a real starter script in place at that moment
-    this.originalWaitinStrategy = this.waitStrategy;
+    this.originalWaitStrategy = this.waitStrategy;
     this.waitStrategy = Wait.forLogMessage(WAIT_FOR_SCRIPT_MESSAGE);
     this.withEntrypoint(["sh"]);
     this.withCommand([
@@ -193,12 +193,7 @@ export class KafkaContainer extends GenericContainer {
     const boundPorts = BoundPorts.fromInspectResult(client.info.containerRuntime.hostIps, inspectResult).filter(
       this.exposedPorts
     );
-    await waitForContainer(
-      client,
-      dockerContainer,
-      this.originalWaitinStrategy ?? Wait.forListeningPorts(),
-      boundPorts
-    );
+    await waitForContainer(client, dockerContainer, this.originalWaitStrategy ?? Wait.forListeningPorts(), boundPorts);
 
     if (this.saslSslConfig && this.mode !== KafkaMode.KRAFT) {
       await this.createUser(container, this.saslSslConfig.sasl);

packages/modules/redpanda/src/redpanda-container.ts

@@ -21,7 +21,7 @@ const STARTER_SCRIPT = "/testcontainers_start.sh";
 const WAIT_FOR_SCRIPT_MESSAGE = "Waiting for script...";
 
 export class RedpandaContainer extends GenericContainer {
-  private originalWaitinStrategy: WaitStrategy | undefined;
+  private originalWaitStrategy: WaitStrategy | undefined;
 
   constructor(image: string) {
     super(image);
@@ -34,7 +34,7 @@ export class RedpandaContainer extends GenericContainer {
           target: "/etc/redpanda/.bootstrap.yaml",
         },
       ]);
-    this.originalWaitinStrategy = this.waitStrategy;
+    this.originalWaitStrategy = this.waitStrategy;
   }
 
   public override async start(): Promise<StartedRedpandaContainer> {
@@ -44,7 +44,7 @@ export class RedpandaContainer extends GenericContainer {
   protected override async beforeContainerCreated(): Promise<void> {
     // Change the wait strategy to wait for a log message from a fake starter script
     // so that we can put a real starter script in place at that moment
-    this.originalWaitinStrategy = this.waitStrategy;
+    this.originalWaitStrategy = this.waitStrategy;
     this.waitStrategy = Wait.forLogMessage(WAIT_FOR_SCRIPT_MESSAGE);
     this.withEntrypoint(["sh"]);
     this.withCommand([
@@ -71,12 +71,7 @@ export class RedpandaContainer extends GenericContainer {
     const boundPorts = BoundPorts.fromInspectResult(client.info.containerRuntime.hostIps, inspectResult).filter(
       this.exposedPorts
     );
-    await waitForContainer(
-      client,
-      dockerContainer,
-      this.originalWaitinStrategy ?? Wait.forListeningPorts(),
-      boundPorts
-    );
+    await waitForContainer(client, dockerContainer, this.originalWaitStrategy ?? Wait.forListeningPorts(), boundPorts);
   }
 
   private renderRedpandaFile(host: string, port: number): string {

packages/testcontainers/src/docker-compose-environment/docker-compose-environment.test.ts

@@ -90,9 +90,9 @@ describe("DockerComposeEnvironment", { timeout: 180_000 }, () => {
   });
 
   it("should support default wait strategy", async () => {
-    await using startedEnvironment = await new DockerComposeEnvironment(fixtures, "docker-compose-with-healthcheck.yml")
-      .withDefaultWaitStrategy(Wait.forHealthCheck())
-      .up();
+    await using startedEnvironment = await new DockerComposeEnvironment(fixtures, "docker-compose.yml")
+      .withDefaultWaitStrategy(Wait.forLogMessage("Listening on port 8080"))
+      .up(["container"]);
 
     await checkEnvironmentContainerIsHealthy(startedEnvironment, "container-1");
   });

packages/testcontainers/src/generic-container/generic-container-wait-strategy.test.ts

@@ -32,6 +32,24 @@ describe("GenericContainer default wait strategy", { timeout: 180_000 }, () => {
     await checkContainerIsHealthy(container);
   });
 
+  it("should prefer a healthcheck configured with withHealthCheck over an image healthcheck", async () => {
+    const context = path.resolve(fixtures, "docker-with-delayed-health-check");
+    const genericContainer = await GenericContainer.fromDockerfile(context).build();
+    await using container = await genericContainer
+      .withExposedPorts(8080)
+      .withCommand(["sh", "-c", "rm -f /tmp/ready /tmp/custom-ready; touch /tmp/custom-ready; node index.js"])
+      .withHealthCheck({
+        test: ["CMD-SHELL", "test -f /tmp/custom-ready"],
+        interval: 1_000,
+        timeout: 1_000,
+        retries: 10,
+      })
+      .start();
+
+    expect(await getHealthCheckStatus(container)).toBe("healthy");
+    await checkContainerIsHealthy(container);
+  });
+
   // Podman compat inspect does not consistently expose Config.Healthcheck for built images.
   if (!process.env.CI_PODMAN) {
     it("should wait for a healthcheck defined in the image", async () => {

packages/testcontainers/src/wait-strategies/health-check-wait-strategy.test.ts

@@ -28,7 +28,6 @@ describe("HealthCheckWaitStrategy", { timeout: 180_000 }, () => {
         retries: 5,
         startPeriod: 1000,
       })
-      .withWaitStrategy(Wait.forHealthCheck())
       .start();
 
     await checkContainerIsHealthy(container);
@@ -44,7 +43,6 @@ describe("HealthCheckWaitStrategy", { timeout: 180_000 }, () => {
         .withName(containerName)
         .withExposedPorts(8080)
         .withHealthCheck({ test: ["CMD-SHELL", "exit 1"], interval: 1, timeout: 3, retries: 3 })
-        .withWaitStrategy(Wait.forHealthCheck())
         .start()
     ).rejects.toThrowError("Health check failed");
 
@@ -60,7 +58,6 @@ describe("HealthCheckWaitStrategy", { timeout: 180_000 }, () => {
       customGenericContainer
         .withName(containerName)
         .withExposedPorts(8080)
-        .withWaitStrategy(Wait.forHealthCheck())
         .withHealthCheck({ test: ["CMD-SHELL", "sleep 10"], timeout: 10_000 })
         .withStartupTimeout(0)
         .start()
@@ -79,7 +76,6 @@ describe("HealthCheckWaitStrategy", { timeout: 180_000 }, () => {
         retries: 5,
         startPeriod: 1000,
       })
-      .withWaitStrategy(Wait.forHealthCheck())
       .start();
 
     await checkContainerIsHealthy(container);

packages/testcontainers/src/wait-strategies/utils/health-check.ts

@@ -3,7 +3,28 @@ import { HealthCheck } from "../../types";
 
 const DISABLED_HEALTH_CHECK_TEST = "NONE";
 
-export const hasHealthCheck = (healthCheck: HealthConfig | HealthCheck | undefined): boolean => {
-  const test = healthCheck === undefined ? undefined : "test" in healthCheck ? healthCheck.test : healthCheck.Test;
-  return test !== undefined && test.length > 0 && test[0].toUpperCase() !== DISABLED_HEALTH_CHECK_TEST;
+type HealthCheckConfig = HealthConfig | HealthCheck;
+
+const getHealthCheckTest = (healthCheck: HealthCheckConfig): string[] | undefined => {
+  if ("test" in healthCheck) {
+    return healthCheck.test;
+  }
+  return healthCheck.Test;
+};
+
+const isDisabledHealthCheck = (test: string[]): boolean => {
+  return test[0].toUpperCase() === DISABLED_HEALTH_CHECK_TEST;
+};
+
+export const hasHealthCheck = (healthCheck: HealthCheckConfig | undefined): boolean => {
+  if (healthCheck === undefined) {
+    return false;
+  }
+
+  const test = getHealthCheckTest(healthCheck);
+  if (test === undefined || test.length === 0) {
+    return false;
+  }
+
+  return !isDisabledHealthCheck(test);
 };