Container stop timeouts should be in milliseconds (#962)

Author: digital88

docs/features/compose.md

@@ -151,11 +151,11 @@ const environment = await new DockerComposeEnvironment(composeFilePath, composeF
 await environment.down();
 ```
 
-If you need to wait for the environment to be downed, you can provide a timeout. The unit of timeout here is **second**:
+If you need to wait for the environment to be downed, you can provide a timeout:
 
 ```javascript
 const environment = await new DockerComposeEnvironment(composeFilePath, composeFile).up();
-await environment.down({ timeout: 10 }); // timeout after 10 seconds
+await environment.down({ timeout: 10_000 }); // 10 seconds
 ```
 
 Volumes created by the environment are removed when stopped. This is configurable:

docs/features/containers.md

@@ -334,11 +334,11 @@ const container = await new GenericContainer("alpine").start();
 await container.stop();
 ```
 
-If you need to wait for the container to be stopped, you can provide a timeout. The unit of timeout option here is **second**:
+If you need to wait for the container to be stopped, you can provide a timeout:
 
 ```javascript
 const container = await new GenericContainer("alpine").start();
-await container.stop({ timeout: 10 }); // 10 seconds
+await container.stop({ timeout: 10_000 }); // 10 seconds
 ```
 
 You can disable automatic removal of the container, which is useful for debugging, or if for example you want to copy content from the container once it has stopped:

docs/features/wait-strategies.md

@@ -1,12 +1,12 @@
 # Wait Strategies
 
-Note that the startup timeout of all wait strategies is configurable. The unit of timeout of wait strategies is **millisecond**:
+Note that the startup timeout of all wait strategies is configurable:
 
 ```javascript
 const { GenericContainer } = require("testcontainers");
 
 const container = await new GenericContainer("alpine")
-  .withStartupTimeout(120000) // wait 120 seconds
+  .withStartupTimeout(120_000) // 120 seconds
   .start();
 ```
 
@@ -73,18 +73,18 @@ const { GenericContainer, Wait } = require("testcontainers");
 const container = await new GenericContainer("alpine").withWaitStrategy(Wait.forHealthCheck()).start();
 ```
 
-Define your own health check. The unit of timeouts and intervals here is **millisecond**:
+Define your own health check:
 
 ```javascript
 const { GenericContainer, Wait } = require("testcontainers");
 
 const container = await new GenericContainer("alpine")
   .withHealthCheck({
     test: ["CMD-SHELL", "curl -f http://localhost || exit 1"],
-    interval: 1000,
-    timeout: 3000,
+    interval: 1000, // 1 second
+    timeout: 3000, // 3 seconds
     retries: 5,
-    startPeriod: 1000,
+    startPeriod: 1000, // 1 second
   })
   .withWaitStrategy(Wait.forHealthCheck())
   .start();
@@ -148,7 +148,7 @@ const container = await new GenericContainer("redis")
   .withMethod("POST")
   .withHeaders({ X_CUSTOM_VALUE: "custom" })
   .withBasicCredentials("username", "password")
-  .withReadTimeout(10000)) // timeout after 10 seconds
+  .withReadTimeout(10_000)) // 10 seconds
 ```
 
 ### Use TLS
@@ -186,7 +186,7 @@ This strategy is intended for use with containers that only run briefly and exit
 const { GenericContainer, Wait } = require("testcontainers");
 
 const container = await new GenericContainer("alpine")
-  .withWaitStrategy(Wait.forOneShotStartup()))
+  .withWaitStrategy(Wait.forOneShotStartup())
   .start();
 ```
 
@@ -202,11 +202,11 @@ const container = await new GenericContainer("alpine")
   .start();
 ```
 
-The composite wait strategy by default will respect each individual wait strategy's startup timeout. The unit of timeouts here is **millisecond**. For example:
+The composite wait strategy by default will respect each individual wait strategy's startup timeout. For example:
 
 ```javascript
-const w1 = Wait.forListeningPorts().withStartupTimeout(1000); // wait 1 second
-const w2 = Wait.forLogMessage("READY").withStartupTimeout(2000); // wait 2 seconds
+const w1 = Wait.forListeningPorts().withStartupTimeout(1000); // 1 second
+const w2 = Wait.forLogMessage("READY").withStartupTimeout(2000); // 2 seconds
 
 const composite = Wait.forAll([w1, w2]);
 
@@ -217,21 +217,21 @@ expect(w2.getStartupTimeout()).toBe(2000);
 The startup timeout of inner wait strategies that have not defined their own startup timeout can be set by setting the startup timeout on the composite:
 
 ```javascript
-const w1 = Wait.forListeningPorts().withStartupTimeout(1000); // wait 1 second
+const w1 = Wait.forListeningPorts().withStartupTimeout(1000); // 1 second
 const w2 = Wait.forLogMessage("READY");
 
-const composite = Wait.forAll([w1, w2]).withStartupTimeout(2000); // wait 2 seconds
+const composite = Wait.forAll([w1, w2]).withStartupTimeout(2000); // 2 seconds
 
 expect(w1.getStartupTimeout()).toBe(1000);
 expect(w2.getStartupTimeout()).toBe(2000);
 ```
 
-The startup timeout of all wait strategies can be controlled by setting a deadline on the composite. In this case, the composite will throw unless all inner wait strategies have resolved before the deadline. The unit of deadline timeout is **millisecond**.
+The startup timeout of all wait strategies can be controlled by setting a deadline on the composite. In this case, the composite will throw unless all inner wait strategies have resolved before the deadline.
 
 ```javascript
 const w1 = Wait.forListeningPorts();
 const w2 = Wait.forLogMessage("READY");
-const composite = Wait.forAll([w1, w2]).withDeadline(2000); // wait 2 seconds
+const composite = Wait.forAll([w1, w2]).withDeadline(2000); // 2 seconds
 ```
 
 ## Other startup strategies

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

@@ -448,12 +448,12 @@ export class CouchbaseContainer extends GenericContainer {
             return jsonResponse.results[0].present;
           },
           () => {
-            const message = `URL /query/service not accessible after ${this.startupTimeout || 60_000}ms`;
+            const message = `URL /query/service not accessible after ${this.startupTimeoutMs || 60_000}ms`;
             log.error(message, { containerId: client.container.getById(startedTestContainer.getId()).id });
 
             throw new Error(message);
           },
-          this.startupTimeout || 60_000
+          this.startupTimeoutMs || 60_000
         );
       }
 
@@ -509,12 +509,12 @@ export class CouchbaseContainer extends GenericContainer {
               return jsonResponse.results[0].online;
             },
             () => {
-              const message = `URL /query/service not accessible after ${this.startupTimeout || 60_000}ms`;
+              const message = `URL /query/service not accessible after ${this.startupTimeoutMs || 60_000}ms`;
               log.error(message, { containerId: client.container.getById(startedTestContainer.getId()).id });
 
               throw new Error(message);
             },
-            this.startupTimeout || 60_000
+            this.startupTimeoutMs || 60_000
           );
         } else {
           log.info(

packages/testcontainers/src/common/index.ts

@@ -3,5 +3,6 @@ export { hash } from "./hash";
 export { Logger, buildLog, composeLog, containerLog, execLog, log, pullLog } from "./logger";
 export { IntervalRetry, Retry } from "./retry";
 export { streamToString } from "./streams";
+export * from "./time";
 export * from "./type-guards";
 export { RandomUuid, Uuid } from "./uuid";

packages/testcontainers/src/common/retry.ts

@@ -5,7 +5,7 @@ export interface Retry<T, U> {
     fn: () => Promise<T>,
     predicate: (result: T) => boolean | Promise<boolean>,
     onTimeout: () => U,
-    timeout: number
+    timeoutMs: number
   ): Promise<T | U>;
 }
 
@@ -16,11 +16,11 @@ abstract class AbstractRetry<T, U> implements Retry<T, U> {
     fn: () => Promise<T>,
     predicate: (result: T) => boolean | Promise<boolean>,
     onTimeout: () => U,
-    timeout: number
+    timeoutMs: number
   ): Promise<T | U>;
 
-  protected hasTimedOut(timeout: number, startTime: Time): boolean {
-    return this.clock.getTime() - startTime > timeout;
+  protected hasTimedOut(timeoutMs: number, startTime: Time): boolean {
+    return this.clock.getTime() - startTime > timeoutMs;
   }
 
   protected wait(duration: number): Promise<void> {
@@ -37,15 +37,15 @@ export class IntervalRetry<T, U> extends AbstractRetry<T, U> {
     fn: (attempt: number) => Promise<T>,
     predicate: (result: T) => boolean | Promise<boolean>,
     onTimeout: () => U,
-    timeout: number
+    timeoutMs: number
   ): Promise<T | U> {
     const startTime = this.clock.getTime();
 
     let attemptNumber = 0;
     let result = await fn(attemptNumber++);
 
     while (!(await predicate(result))) {
-      if (this.hasTimedOut(timeout, startTime)) {
+      if (this.hasTimedOut(timeoutMs, startTime)) {
         return onTimeout();
       }
       await this.wait(this.interval);

packages/testcontainers/src/common/time.test.ts

@@ -0,0 +1,25 @@
+import { toNanos, toSeconds } from "./time";
+
+test.for([
+  [0, 0],
+  [10, 0],
+  [999, 0],
+  [1010, 1],
+  [1999, 1],
+  [10_000, 10],
+  [-10, -0],
+  [-999, -0],
+  [-1010, -1],
+  [-1999, -1],
+  [-10_000, -10],
+])("should convert %i ms to %i seconds", ([ms, s]) => {
+  expect(toSeconds(ms)).toEqual(s);
+});
+
+test.for([
+  [0, 0],
+  [1, 1_000_000],
+  [-1, -1_000_000],
+])("should convert %i ms to %i ns", ([ms, ns]) => {
+  expect(toNanos(ms)).toEqual(ns);
+});

packages/testcontainers/src/common/time.ts

@@ -0,0 +1,3 @@
+export const toSeconds = (ms: number) => Math.trunc(ms * 1e-3);
+
+export const toNanos = (ms: number) => ms * 1e6;

packages/testcontainers/src/container-runtime/clients/compose/compose-client.ts

@@ -1,5 +1,5 @@
 import compose from "docker-compose";
-import { log, pullLog } from "../../../common";
+import { log, pullLog, toSeconds } from "../../../common";
 import { defaultComposeOptions } from "./default-compose-options";
 import { ComposeDownOptions, ComposeOptions } from "./types";
 
@@ -126,7 +126,7 @@ function composeDownCommandOptions(options: ComposeDownOptions): string[] {
     result.push("-v");
   }
   if (options.timeout) {
-    result.push("-t", `${options.timeout / 1000}`);
+    result.push("-t", `${toSeconds(options.timeout)}`);
   }
   return result;
 }

packages/testcontainers/src/container-runtime/clients/container/docker-container-client.ts

@@ -9,7 +9,7 @@ import Dockerode, {
 } from "dockerode";
 import { IncomingMessage } from "http";
 import { PassThrough, Readable } from "stream";
-import { execLog, log, streamToString } from "../../../common";
+import { execLog, log, streamToString, toSeconds } from "../../../common";
 import { ContainerClient } from "./container-client";
 import { ContainerCommitOptions, ContainerStatus, ExecOptions, ExecResult } from "./types";
 
@@ -120,8 +120,7 @@ export class DockerContainerClient implements ContainerClient {
 
   async inspect(container: Dockerode.Container): Promise<ContainerInspectInfo> {
     try {
-      const inspectInfo = await container.inspect();
-      return inspectInfo;
+      return await container.inspect();
     } catch (err) {
       log.error(`Failed to inspect container: ${err}`, { containerId: container.id });
       throw err;
@@ -131,7 +130,7 @@ export class DockerContainerClient implements ContainerClient {
   async stop(container: Container, opts?: { timeout: number }): Promise<void> {
     try {
       log.debug(`Stopping container...`, { containerId: container.id });
-      await container.stop({ t: opts?.timeout });
+      await container.stop({ t: toSeconds(opts?.timeout ?? 0) });
       log.debug(`Stopped container`, { containerId: container.id });
       // eslint-disable-next-line @typescript-eslint/no-explicit-any
     } catch (err: any) {
@@ -267,7 +266,7 @@ export class DockerContainerClient implements ContainerClient {
   async restart(container: Container, opts?: { timeout: number }): Promise<void> {
     try {
       log.debug(`Restarting container...`, { containerId: container.id });
-      await container.restart({ t: opts?.timeout });
+      await container.restart({ t: toSeconds(opts?.timeout ?? 0) });
       log.debug(`Restarted container`, { containerId: container.id });
     } catch (err) {
       log.error(`Failed to restart container: ${err}`, { containerId: container.id });