Address PR comments

Author: technocidal

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

@@ -10,6 +10,7 @@ import { CommitOptions, ContentToCopy, DirectoryToCopy, ExecOptions, ExecResult,
 import { BoundPorts } from "../utils/bound-ports";
 import { LABEL_TESTCONTAINERS_SESSION_ID } from "../utils/labels";
 import { mapInspectResult } from "../utils/map-inspect-result";
+import { PortWithOptionalBinding } from "../utils/port";
 import { waitForContainer } from "../wait-strategies/wait-for-container";
 import { WaitStrategy } from "../wait-strategies/wait-strategy";
 import { inspectContainerUntilPortsExposed } from "./inspect-container-util-ports-exposed";
@@ -95,7 +96,10 @@ export class StartedGenericContainer implements StartedTestContainer {
     }
 
     this.boundPorts = BoundPorts.fromInspectResult(client.info.containerRuntime.hostIps, mappedInspectResult).filter(
-      Array.from(this.boundPorts.iterator()).map((port) => port[0])
+      Array.from(this.boundPorts.iterator()).map((port) => {
+        const [portNumber, protocol] = port[0].split("/");
+        return `${portNumber}/${protocol}` as PortWithOptionalBinding;
+      })
     );
 
     await waitForContainer(client, this.container, this.waitStrategy, this.boundPorts, startTime);

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

@@ -3,73 +3,50 @@ import { getContainerPort, getProtocol, hasHostBinding } from "./port";
 
 describe("port utilities", () => {
   describe("getContainerPort", () => {
-    it("returns number as is", () => {
+    it("should return the container port when defined as a number", () => {
       expect(getContainerPort(8080)).toBe(8080);
     });
 
-    it("parses port from string", () => {
-      expect(getContainerPort("8080")).toBe(8080);
-    });
-
-    it("parses port from string with protocol", () => {
+    it("should return the port when defined as a string with format `port/protocol`", () => {
       expect(getContainerPort("8080/tcp")).toBe(8080);
       expect(getContainerPort("8080/udp")).toBe(8080);
     });
 
-    it("returns container port from object", () => {
+    it("should return the container port from when defined as `PortWithBinding`", () => {
       expect(getContainerPort({ container: 8080, host: 49000 })).toBe(8080);
     });
-
-    it("throws error for invalid port format", () => {
-      expect(() => getContainerPort("invalid")).toThrow("Invalid port format: invalid");
-    });
   });
 
   describe("hasHostBinding", () => {
-    it("returns true for object with host binding", () => {
+    it("should return true for `PortWithBinding` with defined `host` parameter", () => {
       expect(hasHostBinding({ container: 8080, host: 49000 })).toBe(true);
     });
 
-    it("returns false for number", () => {
+    it("should return false when querying for a number", () => {
       expect(hasHostBinding(8080)).toBe(false);
     });
 
-    it("returns false for string", () => {
-      expect(hasHostBinding("8080")).toBe(false);
+    it("should return false when querying for a string with format `port/protocol`", () => {
       expect(hasHostBinding("8080/tcp")).toBe(false);
     });
   });
 
   describe("getProtocol", () => {
-    it("returns tcp for number", () => {
+    it("should return the default `tcp` for a number", () => {
       expect(getProtocol(8080)).toBe("tcp");
     });
 
-    it("returns tcp for string without protocol", () => {
-      expect(getProtocol("8080")).toBe("tcp");
-    });
-
-    it("returns protocol from string", () => {
+    it("should return the protocol part of a string with format `port/protocol`", () => {
       expect(getProtocol("8080/tcp")).toBe("tcp");
       expect(getProtocol("8080/udp")).toBe("udp");
     });
 
-    it("returns protocol from object", () => {
+    it("should maintain backwards compatibility when defined as `PortWithBinding` without protocol", () => {
       expect(getProtocol({ container: 8080, host: 49000 })).toBe("tcp");
-      expect(getProtocol({ container: 8080, host: 49000, protocol: "udp" })).toBe("udp");
-    });
-
-    it("handles protocol case-insensitively", () => {
-      expect(getProtocol({ container: 8080, host: 49000, protocol: "TCP" })).toBe("tcp");
-      expect(getProtocol({ container: 8080, host: 49000, protocol: "UDP" })).toBe("udp");
-      expect(getProtocol("8080/TCP")).toBe("tcp");
-      expect(getProtocol("8080/UDP")).toBe("udp");
     });
 
-    it("maintains backward compatibility", () => {
-      expect(getProtocol(8080)).toBe("tcp");
-      expect(getProtocol("8080")).toBe("tcp");
-      expect(getProtocol({ container: 8080, host: 49000 })).toBe("tcp");
+    it("should return the protocol parameter when defined as `PortWithBinding`", () => {
+      expect(getProtocol({ container: 8080, host: 49000, protocol: "udp" })).toBe("udp");
     });
   });
 });

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

@@ -23,7 +23,7 @@ export class HostPortWaitStrategy extends AbstractWaitStrategy {
     boundPorts: BoundPorts
   ): Promise<void> {
     for (const [portKey, hostPort] of boundPorts.iterator()) {
-      if (portKey.toLowerCase().includes("/udp")) {
+      if (portKey.toLowerCase().endsWith("/udp")) {
         log.debug(`Skipping wait for host port ${hostPort} (mapped from UDP port ${portKey})`, {
           containerId: container.id,
         });
@@ -42,7 +42,7 @@ export class HostPortWaitStrategy extends AbstractWaitStrategy {
     boundPorts: BoundPorts
   ): Promise<void> {
     for (const [internalPort] of boundPorts.iterator()) {
-      if (internalPort.toLowerCase().includes("/udp")) {
+      if (internalPort.toLowerCase().endsWith("/udp")) {
         log.debug(`Skipping wait for internal UDP port ${internalPort}`, {
           containerId: container.id,
         });

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

@@ -11,7 +11,7 @@ export class HostPortCheck implements PortCheck {
   constructor(private readonly client: ContainerRuntimeClient) {}
 
   public isBound(port: number | string): Promise<boolean> {
-    if (typeof port === "string" && port.toLowerCase().includes("/udp")) {
+    if (typeof port === "string" && port.toLowerCase().endsWith("/udp")) {
       log.debug(`Skipping host port check for UDP port ${port} (UDP port checks not supported)`);
       return Promise.resolve(true);
     }