-
Notifications
You must be signed in to change notification settings - Fork 5.1k
CAMEL-23902: Fix flaky UndertowWsConsumerRouteTest #24421
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -46,6 +46,7 @@ | |
| import org.slf4j.Logger; | ||
| import org.slf4j.LoggerFactory; | ||
|
|
||
| import static org.awaitility.Awaitility.await; | ||
| import static org.junit.jupiter.api.Assertions.assertArrayEquals; | ||
| import static org.junit.jupiter.api.Assertions.assertEquals; | ||
| import static org.junit.jupiter.api.Assertions.assertNotNull; | ||
|
|
@@ -168,11 +169,15 @@ public void echo() throws Exception { | |
| wsclient1.connect(); | ||
|
|
||
| wsclient1.sendTextMessage("Test1"); | ||
| wsclient1.sendTextMessage("Test2"); | ||
|
|
||
| assertTrue(wsclient1.await(10)); | ||
| // Wait for the first echo before sending the next message to avoid | ||
| // IllegalStateException from JDK WebSocket when a send is still pending | ||
| await().atMost(10, TimeUnit.SECONDS) | ||
| .untilAsserted(() -> assertTrue(wsclient1.getReceived(String.class).contains("Test1"))); | ||
|
|
||
| assertEquals(Arrays.asList("Test1", "Test2"), wsclient1.getReceived(String.class)); | ||
| wsclient1.sendTextMessage("Test2"); | ||
| await().atMost(10, TimeUnit.SECONDS) | ||
| .untilAsserted( | ||
| () -> assertEquals(Arrays.asList("Test1", "Test2"), wsclient1.getReceived(String.class))); | ||
|
|
||
| wsclient1.close(); | ||
| } | ||
|
|
@@ -187,11 +192,11 @@ public void echoMulti() throws Exception { | |
| wsclient1.sendTextMessage("Gambas"); | ||
| wsclient2.sendTextMessage("Calamares"); | ||
|
|
||
| assertTrue(wsclient1.await(10)); | ||
| assertTrue(wsclient2.await(10)); | ||
|
|
||
| assertEquals(List.of("Gambas"), wsclient1.getReceived(String.class)); | ||
| assertEquals(List.of("Calamares"), wsclient2.getReceived(String.class)); | ||
| await().atMost(10, TimeUnit.SECONDS) | ||
| .untilAsserted(() -> { | ||
| assertEquals(List.of("Gambas"), wsclient1.getReceived(String.class)); | ||
| assertEquals(List.of("Calamares"), wsclient2.getReceived(String.class)); | ||
| }); | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Addressed in commit 01d5f72. Replaced |
||
|
|
||
| wsclient1.close(); | ||
| wsclient2.close(); | ||
|
|
@@ -207,19 +212,18 @@ public void sendToAll() throws Exception { | |
| wsclient1.sendTextMessage("Gambas"); | ||
| wsclient2.sendTextMessage("Calamares"); | ||
|
|
||
| assertTrue(wsclient1.await(10)); | ||
| assertTrue(wsclient2.await(10)); | ||
| await().atMost(10, TimeUnit.SECONDS) | ||
| .untilAsserted(() -> { | ||
| List<String> received1 = wsclient1.getReceived(String.class); | ||
| assertEquals(2, received1.size()); | ||
| assertTrue(received1.contains("Gambas")); | ||
| assertTrue(received1.contains("Calamares")); | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Addressed in commit 01d5f72. The Awaitility polling now uses |
||
|
|
||
| List<String> received1 = wsclient1.getReceived(String.class); | ||
| assertEquals(2, received1.size()); | ||
|
|
||
| assertTrue(received1.contains("Gambas")); | ||
| assertTrue(received1.contains("Calamares")); | ||
|
|
||
| List<String> received2 = wsclient2.getReceived(String.class); | ||
| assertEquals(2, received2.size()); | ||
| assertTrue(received2.contains("Gambas")); | ||
| assertTrue(received2.contains("Calamares")); | ||
| List<String> received2 = wsclient2.getReceived(String.class); | ||
| assertEquals(2, received2.size()); | ||
| assertTrue(received2.contains("Gambas")); | ||
| assertTrue(received2.contains("Calamares")); | ||
| }); | ||
|
|
||
| wsclient1.close(); | ||
| wsclient2.close(); | ||
|
|
@@ -292,27 +296,34 @@ public void connectionKeyList() throws Exception { | |
| wsclient2.connect(); | ||
| wsclient3.connect(); | ||
|
|
||
| wsclient1.await(10); | ||
| await().atMost(10, TimeUnit.SECONDS) | ||
| .untilAsserted(() -> assertConnected(wsclient1)); | ||
| final String connectionKey1 = assertConnected(wsclient1); | ||
| assertNotNull(connectionKey1); | ||
| wsclient2.await(10); | ||
| await().atMost(10, TimeUnit.SECONDS) | ||
|
Comment on lines
+304
to
+308
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Addressed in commit 01d5f72. The Awaitility polling now uses |
||
| .untilAsserted(() -> assertConnected(wsclient2)); | ||
| final String connectionKey2 = assertConnected(wsclient2); | ||
| wsclient3.await(10); | ||
| await().atMost(10, TimeUnit.SECONDS) | ||
| .untilAsserted(() -> assertConnected(wsclient3)); | ||
| final String connectionKey3 = assertConnected(wsclient3); | ||
|
|
||
| wsclient1.reset(1); | ||
| wsclient2.reset(1); | ||
| wsclient3.reset(1); | ||
| final String broadcastMsg = BROADCAST_MESSAGE_PREFIX + connectionKey2 + " " + connectionKey3; | ||
| wsclient1.sendTextMessage(broadcastMsg); // this one should go to wsclient2 and wsclient3 | ||
| wsclient1.sendTextMessage("private"); // this one should go to wsclient1 only | ||
| // Wait for broadcast delivery before sending the next message to avoid | ||
| // IllegalStateException from JDK WebSocket when a send is still pending | ||
| await().atMost(10, TimeUnit.SECONDS) | ||
| .untilAsserted(() -> { | ||
| assertEquals(broadcastMsg, wsclient2.getReceived(String.class).get(0)); | ||
| assertEquals(broadcastMsg, wsclient3.getReceived(String.class).get(0)); | ||
| }); | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Addressed in commit 01d5f72. Both the broadcast and private message waits now use |
||
|
|
||
| wsclient2.await(10); | ||
| assertEquals(broadcastMsg, wsclient2.getReceived(String.class).get(0)); | ||
| wsclient3.await(10); | ||
| assertEquals(broadcastMsg, wsclient3.getReceived(String.class).get(0)); | ||
| wsclient1.await(10); | ||
| assertEquals("private", wsclient1.getReceived(String.class).get(0)); | ||
| wsclient1.sendTextMessage("private"); // this one should go to wsclient1 only | ||
| await().atMost(10, TimeUnit.SECONDS) | ||
| .untilAsserted( | ||
| () -> assertEquals("private", wsclient1.getReceived(String.class).get(0))); | ||
|
|
||
| wsclient1.close(); | ||
| wsclient2.close(); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed in commit 01d5f72. Changed the Awaitility polling from
untilAsserted()withgetReceived(String.class).contains()(which iterates via for-each/iterator) tountil(() -> getReceived().size() >= 1)which uses index-basedsize()— no iterator, safe under concurrent modification. ThegetReceived(String.class)call is now only made after the size gate confirms all expected messages have arrived and the listener thread is done mutating the list.