Skip to content

Commit 7352220

Browse files
digital88digital88
committed
fixes after review
1 parent 40fb443 commit 7352220

9 files changed

Lines changed: 81 additions & 85 deletions

File tree

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

Lines changed: 20 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -99,25 +99,30 @@ describe("DockerComposeEnvironment", { timeout: 180_000 }, () => {
9999
await checkEnvironmentContainerIsHealthy(startedEnvironment, await composeContainerName("container"));
100100
});
101101

102-
it("should use wait strategy Wait.forHealthCheck() if healthcheck is defined in service", async () => {
103-
await using startedEnvironment = await new DockerComposeEnvironment(
104-
fixtures,
105-
"docker-compose-with-healthcheck.yml"
106-
).up();
102+
if (!process.env.CI_PODMAN) {
103+
it("should use wait strategy Wait.forHealthCheck() if healthcheck is defined in service", async () => {
104+
await using startedEnvironment = await new DockerComposeEnvironment(
105+
fixtures,
106+
"docker-compose-with-healthcheck.yml"
107+
).up();
107108

108-
await checkEnvironmentContainerIsHealthy(startedEnvironment, await composeContainerName("container"));
109+
await checkEnvironmentContainerIsHealthy(startedEnvironment, await composeContainerName("container"));
109110

110-
const waitStrategy = startedEnvironment.getContainer("container-1").getWaitStrategy();
111-
expect(waitStrategy).toBeInstanceOf(HealthCheckWaitStrategy);
112-
});
113-
it("should use wait strategy Wait.forListeningPorts() if healthcheck is NOT defined in service", async () => {
114-
await using startedEnvironment = await new DockerComposeEnvironment(fixtures, "docker-compose-with-name.yml").up();
111+
const waitStrategy = startedEnvironment.getContainer("container-1")["getWaitStrategy"]();
112+
expect(waitStrategy).toBeInstanceOf(HealthCheckWaitStrategy);
113+
});
114+
it("should use wait strategy Wait.forListeningPorts() if healthcheck is NOT defined in service", async () => {
115+
await using startedEnvironment = await new DockerComposeEnvironment(
116+
fixtures,
117+
"docker-compose-with-name.yml"
118+
).up();
115119

116-
await checkEnvironmentContainerIsHealthy(startedEnvironment, "custom_container_name");
120+
await checkEnvironmentContainerIsHealthy(startedEnvironment, "custom_container_name");
117121

118-
const waitStrategy = startedEnvironment.getContainer("custom_container_name").getWaitStrategy();
119-
expect(waitStrategy).toBeInstanceOf(HostPortWaitStrategy);
120-
});
122+
const waitStrategy = startedEnvironment.getContainer("custom_container_name")["getWaitStrategy"]();
123+
expect(waitStrategy).toBeInstanceOf(HostPortWaitStrategy);
124+
});
125+
}
121126

122127
it("should support log message wait strategy", async () => {
123128
await using startedEnvironment = await new DockerComposeEnvironment(fixtures, "docker-compose.yml")

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

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import { Environment } from "../types";
77
import { BoundPorts } from "../utils/bound-ports";
88
import { mapInspectResult } from "../utils/map-inspect-result";
99
import { ImagePullPolicy, PullPolicy } from "../utils/pull-policy";
10-
import { NullWaitStrategy } from "../wait-strategies/null-wait-strategy";
1110
import { Wait } from "../wait-strategies/wait";
1211
import { waitForContainer } from "../wait-strategies/wait-for-container";
1312
import { WaitStrategy } from "../wait-strategies/wait-strategy";
@@ -24,7 +23,7 @@ export class DockerComposeEnvironment {
2423
private profiles: string[] = [];
2524
private environment: Environment = {};
2625
private pullPolicy: ImagePullPolicy = PullPolicy.defaultPolicy();
27-
private defaultWaitStrategy: WaitStrategy = new NullWaitStrategy();
26+
private defaultWaitStrategy: WaitStrategy | undefined;
2827
private waitStrategy: { [containerName: string]: WaitStrategy } = {};
2928
private startupTimeoutMs?: number;
3029
private clientOptions: Partial<ComposeOptions> = {};
@@ -207,11 +206,11 @@ export class DockerComposeEnvironment {
207206
});
208207
}
209208

210-
private selectWaitStrategy(containerName: string, inspectResult: ContainerInspectInfo) {
209+
private selectWaitStrategy(containerName: string, inspectResult: ContainerInspectInfo): WaitStrategy {
211210
const containerWaitStrategy = this.waitStrategy[containerName]
212211
? this.waitStrategy[containerName]
213212
: this.defaultWaitStrategy;
214-
if (!(containerWaitStrategy instanceof NullWaitStrategy)) return containerWaitStrategy;
213+
if (containerWaitStrategy) return containerWaitStrategy;
215214
const healthcheck = (
216215
inspectResult as ContainerInspectInfo & {
217216
Config: ContainerInspectInfo["Config"] & {

‎packages/testcontainers/src/generic-container/abstract-started-container.ts‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -76,10 +76,6 @@ export class AbstractStartedContainer implements StartedTestContainer {
7676
return this.startedTestContainer.getIpAddress(networkName);
7777
}
7878

79-
public getWaitStrategy() {
80-
return this.startedTestContainer.getWaitStrategy();
81-
}
82-
8379
public async copyFilesToContainer(filesToCopy: FileToCopy[]): Promise<void> {
8480
return this.startedTestContainer.copyFilesToContainer(filesToCopy);
8581
}

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

Lines changed: 46 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -3,48 +3,53 @@ import { HealthCheckWaitStrategy } from "../wait-strategies/health-check-wait-st
33
import { HostPortWaitStrategy } from "../wait-strategies/host-port-wait-strategy";
44
import { Wait } from "../wait-strategies/wait";
55
import { GenericContainer } from "./generic-container";
6+
import { StartedGenericContainer } from "./started-generic-container";
67

78
const fixtures = path.resolve(__dirname, "..", "..", "fixtures", "docker");
89

9-
describe("GenericContainer wait strategy", { timeout: 180_000 }, () => {
10-
it("should use Wait.forListeningPorts if healthcheck is not defined in DOCKERFILE", async () => {
11-
await using container = await new GenericContainer("cristianrgreco/testcontainer:1.1.14")
12-
.withExposedPorts(8080)
13-
.start();
14-
expect(container.getWaitStrategy()).toBeInstanceOf(HostPortWaitStrategy);
10+
if (!process.env.CI_PODMAN) {
11+
describe("GenericContainer wait strategy", { timeout: 180_000 }, () => {
12+
it("should use Wait.forListeningPorts if healthcheck is not defined in DOCKERFILE", async () => {
13+
await using container = await new GenericContainer("cristianrgreco/testcontainer:1.1.14")
14+
.withExposedPorts(8080)
15+
.start();
16+
expect((container as StartedGenericContainer)["getWaitStrategy"]()).toBeInstanceOf(HostPortWaitStrategy);
17+
});
18+
it("should use Wait.forHealthCheck if withHealthCheck() explicitly called", async () => {
19+
await using container = await new GenericContainer("cristianrgreco/testcontainer:1.1.14")
20+
.withExposedPorts(8080)
21+
.withHealthCheck({
22+
test: ["CMD-SHELL", "echo 'started' && exit 0"],
23+
})
24+
.start();
25+
expect((container as StartedGenericContainer)["getWaitStrategy"]()).toBeInstanceOf(HealthCheckWaitStrategy);
26+
});
27+
it("should use Wait.forHealthCheck if healthcheck is defined in DOCKERFILE", async () => {
28+
const context = path.resolve(fixtures, "docker-with-health-check");
29+
const genericContainer = await GenericContainer.fromDockerfile(context).build();
30+
await using startedContainer = await genericContainer.start();
31+
expect((startedContainer as StartedGenericContainer)["getWaitStrategy"]()).toBeInstanceOf(
32+
HealthCheckWaitStrategy
33+
);
34+
});
35+
it("should use same WaitStrategy if it's explicitly defined in withWaitStrategy() even if image defines healthcheck", async () => {
36+
const context = path.resolve(fixtures, "docker-with-health-check");
37+
const genericContainer = await GenericContainer.fromDockerfile(context).build();
38+
await using container = await genericContainer
39+
.withExposedPorts(8080)
40+
.withWaitStrategy(Wait.forListeningPorts())
41+
.start();
42+
expect((container as StartedGenericContainer)["getWaitStrategy"]()).toBeInstanceOf(HostPortWaitStrategy);
43+
});
44+
it("should use same WaitStrategy if it's explicitly defined in withWaitStrategy() even if withHealthCheck() is called", async () => {
45+
await using container = await new GenericContainer("cristianrgreco/testcontainer:1.1.14")
46+
.withExposedPorts(8080)
47+
.withHealthCheck({
48+
test: ["CMD-SHELL", "echo 'started' && exit 0"],
49+
})
50+
.withWaitStrategy(Wait.forListeningPorts())
51+
.start();
52+
expect((container as StartedGenericContainer)["getWaitStrategy"]()).toBeInstanceOf(HostPortWaitStrategy);
53+
});
1554
});
16-
it("should use Wait.forHealthCheck if withHealthCheck() explicitly called", async () => {
17-
await using container = await new GenericContainer("cristianrgreco/testcontainer:1.1.14")
18-
.withExposedPorts(8080)
19-
.withHealthCheck({
20-
test: ["CMD-SHELL", "echo 'started' && exit 0"],
21-
})
22-
.start();
23-
expect(container.getWaitStrategy()).toBeInstanceOf(HealthCheckWaitStrategy);
24-
});
25-
it("should use Wait.forHealthCheck if healthcheck is defined in DOCKERFILE", async () => {
26-
const context = path.resolve(fixtures, "docker-with-health-check");
27-
const genericContainer = await GenericContainer.fromDockerfile(context).build();
28-
await using startedContainer = await genericContainer.start();
29-
expect(startedContainer.getWaitStrategy()).toBeInstanceOf(HealthCheckWaitStrategy);
30-
});
31-
it("should use same WaitStrategy if it's explicitly defined in withWaitStrategy() even if image defines healthcheck", async () => {
32-
const context = path.resolve(fixtures, "docker-with-health-check");
33-
const genericContainer = await GenericContainer.fromDockerfile(context).build();
34-
await using container = await genericContainer
35-
.withExposedPorts(8080)
36-
.withWaitStrategy(Wait.forListeningPorts())
37-
.start();
38-
expect(container.getWaitStrategy()).toBeInstanceOf(HostPortWaitStrategy);
39-
});
40-
it("should use same WaitStrategy if it's explicitly defined in withWaitStrategy() even if withHealthCheck() is called", async () => {
41-
await using container = await new GenericContainer("cristianrgreco/testcontainer:1.1.14")
42-
.withExposedPorts(8080)
43-
.withHealthCheck({
44-
test: ["CMD-SHELL", "echo 'started' && exit 0"],
45-
})
46-
.withWaitStrategy(Wait.forListeningPorts())
47-
.start();
48-
expect(container.getWaitStrategy()).toBeInstanceOf(HostPortWaitStrategy);
49-
});
50-
});
55+
}

‎packages/testcontainers/src/generic-container/generic-container.ts‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ import { createLabels, LABEL_TESTCONTAINERS_CONTAINER_HASH, LABEL_TESTCONTAINERS
2929
import { mapInspectResult } from "../utils/map-inspect-result";
3030
import { getContainerPort, getProtocol, hasHostBinding, PortWithOptionalBinding } from "../utils/port";
3131
import { ImagePullPolicy, PullPolicy } from "../utils/pull-policy";
32-
import { NullWaitStrategy } from "../wait-strategies/null-wait-strategy";
3332
import { Wait } from "../wait-strategies/wait";
3433
import { waitForContainer } from "../wait-strategies/wait-for-container";
3534
import { WaitStrategy } from "../wait-strategies/wait-strategy";
@@ -49,7 +48,7 @@ export class GenericContainer implements TestContainer {
4948

5049
protected imageName: ImageName;
5150
protected startupTimeoutMs?: number;
52-
protected waitStrategy: WaitStrategy = new NullWaitStrategy();
51+
protected waitStrategy: WaitStrategy | undefined;
5352
protected environment: Record<string, string> = {};
5453
protected exposedPorts: PortWithOptionalBinding[] = [];
5554
protected reuse = false;
@@ -121,7 +120,7 @@ export class GenericContainer implements TestContainer {
121120
}
122121

123122
private async selectWaitStrategy(client: ContainerRuntimeClient): Promise<WaitStrategy> {
124-
if (!(this.waitStrategy instanceof NullWaitStrategy)) return this.waitStrategy;
123+
if (this.waitStrategy) return this.waitStrategy;
125124
if (this.healthCheck) {
126125
return Wait.forHealthCheck();
127126
}
@@ -169,7 +168,7 @@ export class GenericContainer implements TestContainer {
169168
this.exposedPorts
170169
);
171170
if (this.startupTimeoutMs !== undefined) {
172-
this.waitStrategy.withStartupTimeout(this.startupTimeoutMs);
171+
this.waitStrategy?.withStartupTimeout(this.startupTimeoutMs);
173172
}
174173

175174
await waitForContainer(client, container, this.waitStrategy, boundPorts);
@@ -180,7 +179,7 @@ export class GenericContainer implements TestContainer {
180179
inspectResult,
181180
boundPorts,
182181
inspectResult.Name,
183-
this.waitStrategy,
182+
this.waitStrategy ?? Wait.forListeningPorts(),
184183
this.autoRemove
185184
);
186185
}
@@ -225,7 +224,7 @@ export class GenericContainer implements TestContainer {
225224
);
226225

227226
if (this.startupTimeoutMs !== undefined) {
228-
this.waitStrategy.withStartupTimeout(this.startupTimeoutMs);
227+
this.waitStrategy?.withStartupTimeout(this.startupTimeoutMs);
229228
}
230229

231230
if (containerLog.enabled() || this.logConsumer !== undefined) {
@@ -252,7 +251,7 @@ export class GenericContainer implements TestContainer {
252251
inspectResult,
253252
boundPorts,
254253
inspectResult.Name,
255-
this.waitStrategy,
254+
this.waitStrategy ?? Wait.forListeningPorts(),
256255
this.autoRemove
257256
);
258257

‎packages/testcontainers/src/generic-container/started-generic-container.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -168,10 +168,6 @@ export class StartedGenericContainer implements StartedTestContainer {
168168
return this.getNetworkSettings()[networkName].ipAddress;
169169
}
170170

171-
public getWaitStrategy() {
172-
return this.waitStrategy;
173-
}
174-
175171
private getNetworkSettings() {
176172
return Object.entries(this.inspectResult.NetworkSettings.Networks)
177173
.map(([networkName, network]) => ({
@@ -248,4 +244,8 @@ export class StartedGenericContainer implements StartedTestContainer {
248244
async [Symbol.asyncDispose]() {
249245
await this.stop();
250246
}
247+
248+
private getWaitStrategy() {
249+
return this.waitStrategy;
250+
}
251251
}

‎packages/testcontainers/src/test-container.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,6 @@ export interface StartedTestContainer extends AsyncDisposable {
7979
getNetworkNames(): string[];
8080
getNetworkId(networkName: string): string;
8181
getIpAddress(networkName: string): string;
82-
getWaitStrategy(): WaitStrategy;
8382
copyArchiveFromContainer(path: string): Promise<NodeJS.ReadableStream>;
8483
copyArchiveToContainer(tar: Readable, target?: string): Promise<void>;
8584
copyDirectoriesToContainer(directoriesToCopy: DirectoryToCopy[]): Promise<void>;

‎packages/testcontainers/src/wait-strategies/null-wait-strategy.ts‎

Lines changed: 0 additions & 7 deletions
This file was deleted.

‎packages/testcontainers/src/wait-strategies/wait-for-container.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,14 @@ import { WaitStrategy } from "./wait-strategy";
77
export const waitForContainer = async (
88
client: ContainerRuntimeClient,
99
container: Container,
10-
waitStrategy: WaitStrategy,
10+
waitStrategy: WaitStrategy | undefined,
1111
boundPorts: BoundPorts,
1212
startTime?: Date
1313
): Promise<void> => {
1414
log.debug(`Waiting for container to be ready...`, { containerId: container.id });
1515

1616
try {
17-
await waitStrategy.waitUntilReady(container, boundPorts, startTime);
17+
await waitStrategy?.waitUntilReady(container, boundPorts, startTime);
1818
log.info(`Container is ready`, { containerId: container.id });
1919
} catch (err) {
2020
log.error(`Container failed to be ready: ${err}`, { containerId: container.id });

0 commit comments

Comments
 (0)