Copilot commented on code in PR #24423:
URL: https://github.com/apache/camel/pull/24423#discussion_r3524537457
##########
core/camel-core/src/test/java/org/apache/camel/processor/ThreadsRejectedExecutionTest.java:
##########
@@ -173,20 +180,25 @@ public void configure() {
NotifyBuilder notify = new
NotifyBuilder(context).whenDone(10).create();
- // there should be error handling for aborted tasks (eg no redeliveries
- // and no error handling)
- getMockEndpoint("mock:error").expectedMessageCount(0);
-
- getMockEndpoint("mock:result").expectedMinimumMessageCount(2);
for (int i = 0; i < 10; i++) {
template.sendBody("seda:start", "Message " + i);
}
- assertMockEndpointsSatisfied();
-
- assertTrue(notify.matchesWaitTime());
- int inflight = context.getInflightRepository().size();
- assertEquals(0, inflight);
+ // use Awaitility to wait for all exchanges to be done and inflight to
drain
+ Awaitility.await().atMost(30, TimeUnit.SECONDS)
+ .untilAsserted(() -> assertTrue(notify.matches()));
+ Awaitility.await().atMost(10, TimeUnit.SECONDS)
+ .untilAsserted(() -> assertEquals(0,
context.getInflightRepository().size()));
+
+ // at least 2 messages should have made it through the thread pool
+ assertTrue(getMockEndpoint("mock:result").getReceivedCounter() >= 2,
+ "Expected at least 2 messages at mock:result but got "
+ +
getMockEndpoint("mock:result")
+
.getReceivedCounter());
Review Comment:
Similar to the earlier assertion, this message calls `getReceivedCounter()`
multiple times and the wrapping/indentation is difficult to follow. Store the
counter in a local variable and reference it once in the assertion and its
message.
##########
core/camel-core/src/test/java/org/apache/camel/processor/ThreadsRejectedExecutionTest.java:
##########
@@ -119,16 +120,20 @@ public void configure() {
NotifyBuilder notify = new
NotifyBuilder(context).whenDone(10).create();
- getMockEndpoint("mock:result").expectedMinimumMessageCount(2);
for (int i = 0; i < 10; i++) {
template.sendBody("seda:start", "Message " + i);
}
- assertMockEndpointsSatisfied();
- assertTrue(notify.matchesWaitTime());
+ // use Awaitility to wait for all exchanges to be done and inflight to
drain
+ Awaitility.await().atMost(30, TimeUnit.SECONDS)
+ .untilAsserted(() -> assertTrue(notify.matches()));
+ Awaitility.await().atMost(10, TimeUnit.SECONDS)
+ .untilAsserted(() -> assertEquals(0,
context.getInflightRepository().size()));
- int inflight = context.getInflightRepository().size();
- assertEquals(0, inflight);
+ assertTrue(getMockEndpoint("mock:result").getReceivedCounter() >= 2,
+ "Expected at least 2 messages at mock:result but got "
+ +
getMockEndpoint("mock:result")
+
.getReceivedCounter());
Review Comment:
The failure message recomputes
`getMockEndpoint("mock:result").getReceivedCounter()` and the current
line-wrapping makes the concatenation hard to read. Capturing the counter once
improves readability and avoids duplicated calls in the assertion message.
##########
core/camel-core/src/test/java/org/apache/camel/processor/ThreadsRejectedExecutionTest.java:
##########
@@ -173,20 +180,25 @@ public void configure() {
NotifyBuilder notify = new
NotifyBuilder(context).whenDone(10).create();
- // there should be error handling for aborted tasks (eg no redeliveries
- // and no error handling)
- getMockEndpoint("mock:error").expectedMessageCount(0);
-
- getMockEndpoint("mock:result").expectedMinimumMessageCount(2);
for (int i = 0; i < 10; i++) {
template.sendBody("seda:start", "Message " + i);
}
- assertMockEndpointsSatisfied();
-
- assertTrue(notify.matchesWaitTime());
- int inflight = context.getInflightRepository().size();
- assertEquals(0, inflight);
+ // use Awaitility to wait for all exchanges to be done and inflight to
drain
+ Awaitility.await().atMost(30, TimeUnit.SECONDS)
+ .untilAsserted(() -> assertTrue(notify.matches()));
+ Awaitility.await().atMost(10, TimeUnit.SECONDS)
+ .untilAsserted(() -> assertEquals(0,
context.getInflightRepository().size()));
+
+ // at least 2 messages should have made it through the thread pool
+ assertTrue(getMockEndpoint("mock:result").getReceivedCounter() >= 2,
+ "Expected at least 2 messages at mock:result but got "
+ +
getMockEndpoint("mock:result")
+
.getReceivedCounter());
+
+ // there should be no error handling for aborted tasks (no
redeliveries and no error handling)
+ assertEquals(0, getMockEndpoint("mock:error").getReceivedCounter(),
+ "Expected 0 messages at mock:error but got " +
getMockEndpoint("mock:error").getReceivedCounter());
Review Comment:
This assertion evaluates
`getMockEndpoint("mock:error").getReceivedCounter()` twice (once for the actual
value and again for the message). Capture the counter once so the message is
consistent with the asserted value and avoids duplicated calls.
--
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]
