gnodet-bot commented on code in PR #29935:
URL: https://github.com/apache/camel/pull/29935#discussion_r4573491884
##########
core/camel-console/src/main/java/org/apache/camel/impl/console/SendDevConsole.java:
##########
@@ -84,7 +90,7 @@ public record Response(
javaType = "java.lang.Boolean")
public static final String POLL = "poll";
- @Metadata(label = "query", description = "Timeout when using poll mode",
+ @Metadata(label = "query", description = "Timeout of the send, or of the
poll in poll mode, in millis",
javaType = "java.lang.Integer", defaultValue = "20000")
public static final String POLL_TIMEOUT = "pollTimeout";
Review Comment:
🔤 **Naming nit:** `POLL_TIMEOUT` / `pollTimeout` now governs both send and
poll timeouts (see the `CompletableFuture.get(timeout, …)` call below), but the
name implies poll-only. The description was updated, but the identifier wasn't
— could cause confusion when readers see it used in the non-poll path.
##########
core/camel-console/src/test/java/org/apache/camel/impl/console/SendDevConsoleTest.java:
##########
@@ -72,6 +75,62 @@ public void testSendConsoleBasic() {
assertNotNull(jsonOut.getString("endpoint"));
}
+ @Test
+ public void testSendIsBoundedByTheTimeout() {
+ // the timeout was for --poll only: the send waited as long as the
route took (CAMEL-26929)
+ DevConsole console = startedSendConsole();
+
+ Map<String, Object> options = createOptions("direct:slow", "Hello");
+ options.put(SendDevConsole.POLL_TIMEOUT, "200");
+ JsonObject out = callJson(console, options);
+
+ assertEquals("timeout", out.getString("status"), out.toJson());
+ assertTrue(out.getLong("elapsed") < 1500, "answered at the timeout: "
+ out.toJson());
Review Comment:
⚠️ **Potential CI flakiness:** 200 ms timeout with a `< 1500` upper-bound is
a 7.5× margin. On a slow/overloaded CI agent the measurement overhead alone can
consume hundreds of milliseconds. Consider a larger bound (e.g. `< 5000`) or
using `Assertions.assertTimeout` with a soft deadline.
##########
dsl/camel-jbang/camel-jbang-core/src/test/java/org/apache/camel/dsl/jbang/core/commands/action/CamelSendActionResultTest.java:
##########
@@ -58,14 +58,26 @@ void sendsTheMessage() throws Exception {
assertThat(action.getString("endpoint")).isEqualTo("direct:start");
assertThat(action.getString("body")).isEqualTo("hello");
assertThat(action.getString("exchangePattern")).isEqualTo("InOnly");
- // the timeout is at least 5 seconds
- assertThat(action.getLong("pollTimeout")).isEqualTo(5000L);
+ // the timeout as given: it was raised to 5 seconds without a word,
and the integration bounds the send by it
+ // (CAMEL-26929)
+ assertThat(action.getLong("pollTimeout")).isEqualTo(100L);
JsonArray headers = action.getCollection("headers");
JsonObject header = headers.getMap(0);
assertThat(header.getString("key")).isEqualTo("id");
assertThat(header.getString("value")).isEqualTo("1");
}
+ @Test
+ void aTimeoutOfZeroIsAUsageError() throws Exception {
+ writeStatusFile(TEST_PID, "orders");
+
+ assertThatThrownBy(() -> callWithResponse(command("orders",
"--endpoint=direct:start", "--timeout=0"),
+ answer("success")))
+ .isInstanceOfSatisfying(CliException.class,
+ e ->
assertThat(e.getExitCode()).isEqualTo(CliExitCode.USAGE))
+ .hasMessage("--timeout must be more than 0, was: 0");
Review Comment:
🧪 **Missing coverage:** The guard is `timeout <= 0` but only `--timeout=0`
is tested. Negative values hit the same branch. A second assertion (e.g.
`--timeout=-1`) in this test method would give complete coverage.
--
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]
