Skip to content

Commit 23f5cbd

Browse files
committed
Address additional comments from review
1 parent 0b9c342 commit 23f5cbd

File tree

3 files changed

+20
-40
lines changed

3 files changed

+20
-40
lines changed

packages/testcontainers/src/utils/bound-ports.ts

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,15 +7,17 @@ export class BoundPorts {
77
private readonly ports = new Map<string, number>();
88

99
public getBinding(port: number | string, protocol: string = "tcp"): number {
10-
const normalizedProtocol = protocol.toLowerCase();
11-
const key = typeof port === "string" ? port : `${port}/${normalizedProtocol}`;
12-
let binding = this.ports.get(key);
13-
if (!binding && typeof port === "string" && port.includes("/")) {
10+
let key: string;
11+
12+
if (typeof port === "string" && port.includes("/")) {
1413
const [portNumber, portProtocol] = port.split("/");
15-
const normalizedKey = `${portNumber}/${portProtocol.toLowerCase()}`;
16-
binding = this.ports.get(normalizedKey);
14+
key = `${portNumber}/${portProtocol.toLowerCase()}`;
15+
} else {
16+
key = `${port}/${protocol.toLowerCase()}`;
1717
}
1818

19+
const binding = this.ports.get(key);
20+
1921
if (!binding) {
2022
throw new Error(`No port binding found for :${key}`);
2123
}

packages/testcontainers/src/wait-strategies/host-port-wait-strategy.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ describe("HostPortWaitStrategy", { timeout: 180_000 }, () => {
2020
.withExposedPorts(8081)
2121
.withStartupTimeout(0)
2222
.start()
23-
).rejects.toThrowError(/Port (?:\d+|\d+\/\w+) not bound after 0ms/);
23+
).rejects.toThrowError(/Port \d+\/(tcp|udp) not bound after 0ms/);
2424

2525
expect(await getRunningContainerNames()).not.toContain(containerName);
2626
});

packages/testcontainers/src/wait-strategies/host-port-wait-strategy.ts

Lines changed: 11 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ export class HostPortWaitStrategy extends AbstractWaitStrategy {
4242
boundPorts: BoundPorts
4343
): Promise<void> {
4444
for (const [internalPort] of boundPorts.iterator()) {
45-
if (typeof internalPort === "string" && internalPort.toLowerCase().includes("/udp")) {
45+
if (internalPort.toLowerCase().includes("/udp")) {
4646
log.debug(`Skipping wait for internal UDP port ${internalPort}`, {
4747
containerId: container.id,
4848
});
@@ -60,37 +60,15 @@ export class HostPortWaitStrategy extends AbstractWaitStrategy {
6060
port: number | string,
6161
portCheck: PortCheck
6262
): Promise<void> {
63-
// Skip waiting for UDP ports
64-
if (typeof port === "string" && port.toLowerCase().includes("/udp")) {
65-
log.debug(`Skipping wait for UDP port ${port} (UDP port checks not supported)`, { containerId: container.id });
66-
return;
67-
}
68-
69-
log.debug(`Checking availability for port: ${port}`);
70-
71-
try {
72-
await new IntervalRetry<boolean, Error>(100).retryUntil(
73-
() => {
74-
console.log(`Checking if port ${port} is bound...`);
75-
return portCheck.isBound(port);
76-
},
77-
(isBound) => {
78-
if (isBound) {
79-
console.log(`Port ${port} is now bound!`);
80-
}
81-
return isBound;
82-
},
83-
() => {
84-
const message = `Port ${port} not bound after ${this.startupTimeoutMs}ms`;
85-
console.log(`TIMEOUT: ${message}`);
86-
log.error(message, { containerId: container.id });
87-
throw new Error(message);
88-
},
89-
this.startupTimeoutMs
90-
);
91-
} catch (err) {
92-
log.error(`Error checking port ${port} with error ${err}`);
93-
throw err;
94-
}
63+
await new IntervalRetry<boolean, Error>(100).retryUntil(
64+
() => portCheck.isBound(port),
65+
(isBound) => isBound,
66+
() => {
67+
const message = `Port ${port} not bound after ${this.startupTimeoutMs}ms`;
68+
log.error(message, { containerId: container.id });
69+
throw new Error(message);
70+
},
71+
this.startupTimeoutMs
72+
);
9573
}
9674
}

0 commit comments

Comments
 (0)