Skip to content

Commit 4ac02b5

Browse files
committed
Remove unnecessary comments
1 parent abbf14e commit 4ac02b5

File tree

6 files changed

+8
-52
lines changed

6 files changed

+8
-52
lines changed

packages/testcontainers/src/generic-container/inspect-container-util-ports-exposed.test.ts

Lines changed: 2 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,6 @@ test("handles multiple ports with different protocols", async () => {
118118
});
119119

120120
test("fails when protocol doesn't match exposed port", async () => {
121-
// Container exposes TCP port but we're looking for UDP
122121
const data = mockInspectResult({ "8080/tcp": [{ HostIp: "0.0.0.0", HostPort: "45000" }] });
123122
const inspectFn = vi.fn().mockResolvedValue(data.inspectResult);
124123

@@ -135,7 +134,6 @@ test("ignores ports with wrong protocol", async () => {
135134
const data = mockInspectResult(ports);
136135
const inspectFn = vi.fn().mockResolvedValueOnce(data.inspectResult);
137136

138-
// Should only match the UDP port
139137
const result = await inspectContainerUntilPortsExposed(inspectFn, ["8080/udp"], "container-id");
140138

141139
expect(result).toEqual(data);
@@ -152,25 +150,16 @@ test("handles mixed protocol specifications in different formats", async () => {
152150

153151
const result = await inspectContainerUntilPortsExposed(
154152
inspectFn,
155-
[
156-
8080, // number (default tcp)
157-
"9090/udp", // string with protocol
158-
{ container: 7070, host: 47000 }, // object (default tcp)
159-
],
153+
[8080, "9090/udp", { container: 7070, host: 47000 }],
160154
"container-id"
161155
);
162156

163157
expect(result).toEqual(data);
164158
});
165159

166160
test("retry with gradually exposed ports of different protocols", async () => {
167-
// First call: No ports exposed
168161
const data1 = mockInspectResult({});
169-
170-
// Second call: Only TCP port exposed
171162
const data2 = mockInspectResult({ "8080/tcp": [{ HostIp: "0.0.0.0", HostPort: "45000" }] });
172-
173-
// Third call: Both TCP and UDP ports exposed
174163
const data3 = mockInspectResult({
175164
"8080/tcp": [{ HostIp: "0.0.0.0", HostPort: "45000" }],
176165
"8080/udp": [{ HostIp: "0.0.0.0", HostPort: "46000" }],
@@ -182,11 +171,7 @@ test("retry with gradually exposed ports of different protocols", async () => {
182171
.mockResolvedValueOnce(data2.inspectResult)
183172
.mockResolvedValueOnce(data3.inspectResult);
184173

185-
const result = await inspectContainerUntilPortsExposed(
186-
inspectFn,
187-
[8080, "8080/udp"], // Need both TCP and UDP ports
188-
"container-id"
189-
);
174+
const result = await inspectContainerUntilPortsExposed(inspectFn, [8080, "8080/udp"], "container-id");
190175

191176
expect(result).toEqual(data3);
192177
expect(inspectFn).toHaveBeenCalledTimes(3);

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

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,7 @@ describe("BoundPorts", () => {
1515
boundPorts.setBinding(1, 1000, "tcp");
1616
boundPorts.setBinding(1, 2000, "udp");
1717

18-
// Default protocol is tcp
1918
expect(boundPorts.getBinding(1)).toBe(1000);
20-
// Can explicitly specify protocol
2119
expect(boundPorts.getBinding(1, "tcp")).toBe(1000);
2220
expect(boundPorts.getBinding(1, "udp")).toBe(2000);
2321
});
@@ -29,7 +27,6 @@ describe("BoundPorts", () => {
2927

3028
expect(boundPorts.getBinding("8080/tcp")).toBe(1000);
3129
expect(boundPorts.getBinding("8080/udp")).toBe(2000);
32-
// Can also specify as number + protocol
3330
expect(boundPorts.getBinding(8080, "tcp")).toBe(1000);
3431
expect(boundPorts.getBinding(8080, "udp")).toBe(2000);
3532
});
@@ -79,10 +76,8 @@ describe("BoundPorts", () => {
7976

8077
const boundPorts = BoundPorts.fromInspectResult(hostIps, inspectResult as InspectResult);
8178

82-
// Default to TCP
8379
expect(boundPorts.getBinding(8080)).toBe(10000);
8480
expect(boundPorts.getBinding(8081)).toBe(10001);
85-
// Explicitly specify protocol
8681
expect(boundPorts.getBinding(8080, "tcp")).toBe(10000);
8782
expect(boundPorts.getBinding(8080, "udp")).toBe(10002);
8883
});
@@ -104,30 +99,21 @@ describe("BoundPorts", () => {
10499
boundPorts.setBinding(8080, 2000, "udp");
105100
boundPorts.setBinding(9090, 3000, "tcp");
106101

107-
// Create a simplified filter test
108-
let filtered = boundPorts.filter([8080]); // Just filter TCP port
109-
110-
// Should only include the TCP binding
102+
let filtered = boundPorts.filter([8080]);
111103
expect(filtered.getBinding(8080)).toBe(1000);
112104
expect(() => filtered.getBinding(8080, "udp")).toThrowError("No port binding found for :8080/udp");
113105
expect(() => filtered.getBinding(9090)).toThrowError("No port binding found for :9090/tcp");
114106

115-
// Now filter only the UDP port
116107
filtered = boundPorts.filter(["8080/udp"]);
117-
118-
// Should only include the UDP binding
119108
expect(filtered.getBinding(8080, "udp")).toBe(2000);
120109
expect(() => filtered.getBinding(8080, "tcp")).toThrowError("No port binding found for :8080/tcp");
121110
});
122111

123112
it("should handle case-insensitive protocols", () => {
124113
const boundPorts = new BoundPorts();
125114
boundPorts.setBinding(8080, 1000, "tcp");
126-
127-
// Should be able to retrieve with uppercase protocol
128115
expect(boundPorts.getBinding(8080, "TCP")).toBe(1000);
129116

130-
// Also works with uppercase in the key
131117
boundPorts.setBinding("9090/TCP", 2000);
132118
expect(boundPorts.getBinding(9090, "tcp")).toBe(2000);
133119
expect(boundPorts.getBinding("9090/tcp")).toBe(2000);

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

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,6 @@ export class BoundPorts {
1010
const normalizedProtocol = protocol.toLowerCase();
1111
const key = typeof port === "string" ? port : `${port}/${normalizedProtocol}`;
1212
let binding = this.ports.get(key);
13-
14-
// Try case-insensitive lookup if not found and port is a string that might contain a protocol
1513
if (!binding && typeof port === "string" && port.includes("/")) {
1614
const [portNumber, portProtocol] = port.split("/");
1715
const normalizedKey = `${portNumber}/${portProtocol.toLowerCase()}`;
@@ -36,16 +34,13 @@ export class BoundPorts {
3634
}
3735

3836
public setBinding(key: string | number, value: number, protocol: string = "tcp"): void {
39-
// Normalize protocol to lowercase for consistency
4037
const normalizedProtocol = protocol.toLowerCase();
4138

4239
if (typeof key === "string" && key.includes("/")) {
43-
// If key contains protocol, normalize it
4440
const [portNumber, portProtocol] = key.split("/");
4541
const normalizedKey = `${portNumber}/${portProtocol.toLowerCase()}`;
4642
this.ports.set(normalizedKey, value);
4743
} else {
48-
// Otherwise use the provided key/protocol
4944
const portKey = typeof key === "string" ? key : `${key}/${normalizedProtocol}`;
5045
this.ports.set(portKey, value);
5146
}
@@ -57,8 +52,6 @@ export class BoundPorts {
5752

5853
public filter(ports: PortWithOptionalBinding[]): BoundPorts {
5954
const boundPorts = new BoundPorts();
60-
61-
// Create map of port to protocol for lookup
6255
const containerPortsWithProtocol = new Map<number, string>();
6356
ports.forEach((port) => {
6457
const containerPort = getContainerPort(port);
@@ -69,8 +62,6 @@ export class BoundPorts {
6962
for (const [internalPortWithProtocol, hostPort] of this.iterator()) {
7063
const [internalPortStr, protocol] = internalPortWithProtocol.split("/");
7164
const internalPort = parseInt(internalPortStr, 10);
72-
73-
// Case-insensitive protocol comparison
7465
if (
7566
containerPortsWithProtocol.has(internalPort) &&
7667
containerPortsWithProtocol.get(internalPort)?.toLowerCase() === protocol?.toLowerCase()

packages/testcontainers/src/utils/port.test.ts

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -67,16 +67,14 @@ describe("port utilities", () => {
6767
});
6868

6969
test("maintains backward compatibility", () => {
70-
// Original usage patterns should still work
71-
expect(getProtocol(8080)).toBe("tcp"); // Number defaults to TCP
72-
expect(getProtocol("8080")).toBe("tcp"); // Simple string defaults to TCP
73-
expect(getProtocol({ container: 8080, host: 49000 })).toBe("tcp"); // Object without protocol defaults to TCP
70+
expect(getProtocol(8080)).toBe("tcp");
71+
expect(getProtocol("8080")).toBe("tcp");
72+
expect(getProtocol({ container: 8080, host: 49000 })).toBe("tcp");
7473
});
7574

7675
test("supports protocol specification in all formats", () => {
77-
// New usage patterns with protocols
78-
expect(getProtocol("8080/udp")).toBe("udp"); // String with protocol
79-
expect(getProtocol({ container: 8080, host: 49000, protocol: "udp" })).toBe("udp"); // Object with protocol
76+
expect(getProtocol("8080/udp")).toBe("udp");
77+
expect(getProtocol({ container: 8080, host: 49000, protocol: "udp" })).toBe("udp");
8078
});
8179

8280
test("note about UDP port checks", () => {

packages/testcontainers/src/wait-strategies/utils/port-check.test.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,6 @@ describe.sequential("PortCheck", () => {
3131
// @ts-ignore
3232
client = new ContainerRuntimeClient();
3333
portCheck = new InternalPortCheck(client, mockContainer);
34-
35-
// Make sure logging is enabled to capture all logs
3634
mockLogger.enabled.mockImplementation(() => true);
3735
});
3836

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ export class HostPortCheck implements PortCheck {
1111
constructor(private readonly client: ContainerRuntimeClient) {}
1212

1313
public isBound(port: number | string): Promise<boolean> {
14-
// Skip check for UDP ports as they can't be verified with a TCP socket connection
1514
if (typeof port === "string" && port.toLowerCase().includes("/udp")) {
1615
log.debug(`Skipping host port check for UDP port ${port} (UDP port checks not supported)`);
1716
return Promise.resolve(true);
@@ -48,7 +47,6 @@ export class InternalPortCheck implements PortCheck {
4847
) {}
4948

5049
public async isBound(port: number | string): Promise<boolean> {
51-
// Skip check for UDP ports as they require different verification methods
5250
if (typeof port === "string" && port.toLowerCase().includes("/udp")) {
5351
log.debug(`Skipping internal port check for UDP port ${port} (UDP port checks not supported)`, {
5452
containerId: this.container.id,

0 commit comments

Comments
 (0)