Skip to content

Commit 5f605d4

Browse files
test: poll for port release in TcpServerTest to fix flake (#166)
**Requirements** - [x] I have added test coverage for new or changed functionality - [x] I have followed the repository's [pull request submission guidelines](../blob/main/CONTRIBUTING.md#submitting-pull-requests) - [x] I have validated my changes against all supported platform versions **Related issues** Fixes a flaky `TcpServerTest.listensOnSpecificPort` test observed in CI. **Describe the solution you've provided** After `TcpServer.close()`, there can be a brief OS-level delay before the port is fully released. The test was immediately checking `doesPortHaveListener()` which could return true in that window. Replaced the immediate `assertFalse` calls with a polling helper (`assertPortReleased`) that retries for up to 1 second before failing. Applied to both `listensOnSpecificPort` and `listensOnAnyAvailablePort` for consistency. **Describe alternatives you've considered** - Adding a fixed `Thread.sleep()` before the assertion — less robust and adds unnecessary delay in the common case. - Modifying `TcpServer.close()` to join the accept thread — would be an application code change rather than a test fix. **Additional context** The flake was observed on `ubuntu-latest` with Java 19 during CI. Link to Devin session: https://app.devin.ai/sessions/89320790abd2401e90b44eb0fcfb66be Requested by: @kinyoklion <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Test-only timing change in shared test helpers; no production or security impact. > > **Overview** > **TcpServerTest** no longer asserts that a port is free the instant `TcpServer` closes. Both `listensOnAnyAvailablePort` and `listensOnSpecificPort` now call a new **`assertPortReleased`** helper that polls **`doesPortHaveListener`** for up to one second (50 ms between attempts) before failing with the same message as before. > > This addresses CI flakes where the OS can still report a listener briefly after close. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 9d42e36. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
1 parent 1c17de0 commit 5f605d4

1 file changed

Lines changed: 17 additions & 2 deletions

File tree

  • lib/shared/test-helpers/src/test/java/com/launchdarkly/testhelpers/tcptest

lib/shared/test-helpers/src/test/java/com/launchdarkly/testhelpers/tcptest/TcpServerTest.java

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ public void listensOnAnyAvailablePort() throws IOException {
2121
port = server.getPort();
2222
try (Socket s = new Socket("localhost", server.getPort())) {} // just verify that we can connect
2323
}
24-
assertFalse("expected listener to be closed, but it wasn't", doesPortHaveListener(port));
24+
assertPortReleased(port);
2525
}
2626

2727
@Test
@@ -37,6 +37,21 @@ public void listensOnSpecificPort() throws IOException {
3737
assertEquals(specificPort, server.getPort());
3838
try (Socket s = new Socket("localhost", specificPort)) {} // just verify that we can connect
3939
}
40-
assertFalse("expected listener to be closed, but it wasn't", doesPortHaveListener(specificPort));
40+
assertPortReleased(specificPort);
41+
}
42+
43+
private static void assertPortReleased(int port) {
44+
long deadline = System.currentTimeMillis() + 1000;
45+
while (System.currentTimeMillis() < deadline) {
46+
if (!doesPortHaveListener(port)) {
47+
return;
48+
}
49+
try {
50+
Thread.sleep(50);
51+
} catch (InterruptedException e) {
52+
throw new RuntimeException(e);
53+
}
54+
}
55+
assertFalse("expected listener to be closed, but it wasn't", doesPortHaveListener(port));
4156
}
4257
}

0 commit comments

Comments
 (0)