Copilot commented on code in PR #24422:
URL: https://github.com/apache/camel/pull/24422#discussion_r3524479916
##########
core/camel-core/src/test/java/org/apache/camel/processor/DelayerWhileShutdownTest.java:
##########
@@ -16,44 +16,45 @@
*/
package org.apache.camel.processor;
-import java.util.concurrent.Phaser;
+import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
import org.apache.camel.ContextTestSupport;
import org.apache.camel.builder.RouteBuilder;
import org.apache.camel.component.mock.MockEndpoint;
-import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
+import static org.awaitility.Awaitility.await;
+
/**
* Delayer while shutting down so its interrupted and will also stop.
*/
public class DelayerWhileShutdownTest extends ContextTestSupport {
- private final Phaser phaser = new Phaser(2);
-
- @BeforeEach
- void sendEarly() {
- template.sendBody("seda:a", "Long delay");
- template.sendBody("seda:b", "Short delay");
- }
+ private final CountDownLatch shortDelayStarted = new CountDownLatch(1);
@Test
public void testSendingMessageGetsDelayed() throws Exception {
MockEndpoint mock = getMockEndpoint("mock:result");
mock.expectedBodiesReceived("Short delay");
- phaser.awaitAdvanceInterruptibly(0, 5000, TimeUnit.SECONDS);
+ template.sendBody("seda:a", "Long delay");
+ template.sendBody("seda:b", "Short delay");
+
+ // Wait until the short-delay route has entered the delay phase,
+ // ensuring both routes are actively processing
+ shortDelayStarted.await(5, TimeUnit.SECONDS);
Review Comment:
`shortDelayStarted.await(5, TimeUnit.SECONDS)` ignores its boolean return
value, so if the latch is never counted down (route never starts) the test will
still proceed and can remain flaky/non-diagnostic. Also the comment says the
route has "entered the delay phase" and that this ensures both routes are
processing, but the latch is counted down before the delay and only covers the
short-delay route.
##########
core/camel-core/src/test/java/org/apache/camel/processor/DelayerWhileShutdownTest.java:
##########
@@ -16,44 +16,45 @@
*/
package org.apache.camel.processor;
-import java.util.concurrent.Phaser;
+import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
import org.apache.camel.ContextTestSupport;
import org.apache.camel.builder.RouteBuilder;
import org.apache.camel.component.mock.MockEndpoint;
-import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
+import static org.awaitility.Awaitility.await;
+
/**
* Delayer while shutting down so its interrupted and will also stop.
*/
public class DelayerWhileShutdownTest extends ContextTestSupport {
- private final Phaser phaser = new Phaser(2);
-
- @BeforeEach
- void sendEarly() {
- template.sendBody("seda:a", "Long delay");
- template.sendBody("seda:b", "Short delay");
- }
+ private final CountDownLatch shortDelayStarted = new CountDownLatch(1);
@Test
public void testSendingMessageGetsDelayed() throws Exception {
MockEndpoint mock = getMockEndpoint("mock:result");
mock.expectedBodiesReceived("Short delay");
- phaser.awaitAdvanceInterruptibly(0, 5000, TimeUnit.SECONDS);
+ template.sendBody("seda:a", "Long delay");
+ template.sendBody("seda:b", "Short delay");
+
+ // Wait until the short-delay route has entered the delay phase,
+ // ensuring both routes are actively processing
+ shortDelayStarted.await(5, TimeUnit.SECONDS);
- assertMockEndpointsSatisfied();
+ await().atMost(5, TimeUnit.SECONDS)
+ .untilAsserted(() -> assertMockEndpointsSatisfied());
}
@Override
protected RouteBuilder createRouteBuilder() {
return new RouteBuilder() {
public void configure() {
- from("seda:a").process(e ->
phaser.arriveAndAwaitAdvance()).delay(1000).to("mock:result");
- from("seda:b").process(e ->
phaser.arriveAndAwaitAdvance()).delay(1).to("mock:result");
+ from("seda:a").delay(5000).to("mock:result");
Review Comment:
The long delay is set to 5000ms, which is the same as the `await().atMost(5,
TimeUnit.SECONDS)` timeout. If the assertion takes close to the full timeout
under load, the "Long delay" exchange can reach `mock:result` and break the
expectation, reintroducing flakiness. Setting the long delay comfortably above
the assertion timeout avoids this race without slowing the test (shutdown
should interrupt the delay).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
