gnodet-bot commented on code in PR #29741:
URL: https://github.com/apache/camel/pull/29741#discussion_r4554101103
##########
components/camel-ai/camel-mcp-server/src/test/java/org/apache/camel/component/mcp/server/vertx/VertxMcpSessionEvictionTest.java:
##########
@@ -49,60 +50,68 @@ class VertxMcpSessionEvictionTest {
@Test
void evictsIdleSessionWithoutDelete() throws Exception {
- int port = AvailablePortFinder.getNextAvailable();
- VertxMcpServerEngine engine = startEngine(port, new
McpServerInfo("eviction", "1.0", "/mcp", 0, 500));
- try {
- initializeSession(port);
- assertThat(engine.sessionCount()).isEqualTo(1);
-
- await().atMost(5, TimeUnit.SECONDS).untilAsserted(() ->
assertThat(engine.sessionCount()).isZero());
- } finally {
- engine.stop();
+ try (Port portHolder = AvailablePortFinder.find()) {
+ int port = portHolder.getPort();
+ VertxMcpServerEngine engine = startEngine(port, new
McpServerInfo("eviction", "1.0", "/mcp", 0, 500));
+ try {
+ initializeSession(port);
+ assertThat(engine.sessionCount()).isEqualTo(1);
+
+ await().atMost(5, TimeUnit.SECONDS).untilAsserted(() ->
assertThat(engine.sessionCount()).isZero());
+ } finally {
+ engine.stop();
+ }
}
}
@Test
void postOnlySessionSurvivesKeepAliveUntilIdle() throws Exception {
- int port = AvailablePortFinder.getNextAvailable();
- VertxMcpServerEngine engine = startEngine(port, new
McpServerInfo("eviction", "1.0", "/mcp", 300, 10_000));
- try {
- String sessionId = initializeSession(port);
-
- await().during(2, TimeUnit.SECONDS).atMost(3,
TimeUnit.SECONDS).untilAsserted(() -> {
- assertThat(postWithSession(port, sessionId,
INITIALIZED_NOTIFICATION)).isEqualTo(202);
- assertThat(engine.sessionCount()).isEqualTo(1);
- });
- } finally {
- engine.stop();
+ try (Port portHolder = AvailablePortFinder.find()) {
+ int port = portHolder.getPort();
+ VertxMcpServerEngine engine = startEngine(port, new
McpServerInfo("eviction", "1.0", "/mcp", 300, 10_000));
+ try {
+ String sessionId = initializeSession(port);
+
+ await().during(500, TimeUnit.MILLISECONDS).atMost(1500,
TimeUnit.MILLISECONDS).untilAsserted(() -> {
Review Comment:
⚠️ **Weakened assertion window** — `during(500ms)` barely covers ~1.7
keep-alive cycles (keep-alive interval = 300 ms), whereas the original
`during(2s)` covered ~6.7 cycles. On a loaded CI machine this may mask a broken
keep-alive: if the implementation stops sending keep-alives, the session could
still survive 500 ms purely on latency. Consider restoring `during(2, SECONDS)`
or at minimum `during(1, SECONDS)` to give the keep-alive machinery enough
cycles to prove it is working.
##########
components/camel-ai/camel-mcp-server/src/test/java/org/apache/camel/component/mcp/server/vertx/VertxMcpSessionEvictionTest.java:
##########
@@ -49,60 +50,68 @@ class VertxMcpSessionEvictionTest {
@Test
void evictsIdleSessionWithoutDelete() throws Exception {
- int port = AvailablePortFinder.getNextAvailable();
- VertxMcpServerEngine engine = startEngine(port, new
McpServerInfo("eviction", "1.0", "/mcp", 0, 500));
- try {
- initializeSession(port);
- assertThat(engine.sessionCount()).isEqualTo(1);
-
- await().atMost(5, TimeUnit.SECONDS).untilAsserted(() ->
assertThat(engine.sessionCount()).isZero());
- } finally {
- engine.stop();
+ try (Port portHolder = AvailablePortFinder.find()) {
+ int port = portHolder.getPort();
+ VertxMcpServerEngine engine = startEngine(port, new
McpServerInfo("eviction", "1.0", "/mcp", 0, 500));
+ try {
+ initializeSession(port);
+ assertThat(engine.sessionCount()).isEqualTo(1);
+
+ await().atMost(5, TimeUnit.SECONDS).untilAsserted(() ->
assertThat(engine.sessionCount()).isZero());
+ } finally {
+ engine.stop();
+ }
}
}
@Test
void postOnlySessionSurvivesKeepAliveUntilIdle() throws Exception {
- int port = AvailablePortFinder.getNextAvailable();
- VertxMcpServerEngine engine = startEngine(port, new
McpServerInfo("eviction", "1.0", "/mcp", 300, 10_000));
- try {
- String sessionId = initializeSession(port);
-
- await().during(2, TimeUnit.SECONDS).atMost(3,
TimeUnit.SECONDS).untilAsserted(() -> {
- assertThat(postWithSession(port, sessionId,
INITIALIZED_NOTIFICATION)).isEqualTo(202);
- assertThat(engine.sessionCount()).isEqualTo(1);
- });
- } finally {
- engine.stop();
+ try (Port portHolder = AvailablePortFinder.find()) {
+ int port = portHolder.getPort();
+ VertxMcpServerEngine engine = startEngine(port, new
McpServerInfo("eviction", "1.0", "/mcp", 300, 10_000));
+ try {
+ String sessionId = initializeSession(port);
+
+ await().during(500, TimeUnit.MILLISECONDS).atMost(1500,
TimeUnit.MILLISECONDS).untilAsserted(() -> {
+ assertThat(postWithSession(port, sessionId,
INITIALIZED_NOTIFICATION)).isEqualTo(202);
+ assertThat(engine.sessionCount()).isEqualTo(1);
+ });
+ } finally {
+ engine.stop();
+ }
}
}
@Test
void activeSessionSurvivesIdleEviction() throws Exception {
- int port = AvailablePortFinder.getNextAvailable();
- VertxMcpServerEngine engine = startEngine(port, new
McpServerInfo("eviction", "1.0", "/mcp", 0, 2_000));
- try {
- String sessionId = initializeSession(port);
-
- await().during(2, TimeUnit.SECONDS).atMost(3,
TimeUnit.SECONDS).untilAsserted(() -> {
- assertThat(postWithSession(port, sessionId,
INITIALIZED_NOTIFICATION)).isEqualTo(202);
- assertThat(engine.sessionCount()).isEqualTo(1);
- });
- } finally {
- engine.stop();
+ try (Port portHolder = AvailablePortFinder.find()) {
+ int port = portHolder.getPort();
+ VertxMcpServerEngine engine = startEngine(port, new
McpServerInfo("eviction", "1.0", "/mcp", 0, 2_000));
+ try {
+ String sessionId = initializeSession(port);
+
+ await().during(500, TimeUnit.MILLISECONDS).atMost(1500,
TimeUnit.MILLISECONDS).untilAsserted(() -> {
Review Comment:
⚠️ **Trivially-passing assertion** — the idle TTL is 2 000 ms, but
`during(500ms)` ends well before any eviction can occur. The session was never
at risk of being evicted during the assertion window, so the test passes
whether or not the active-request idle-reset logic works. The original
`during(2s)` matched the idle TTL and actually exercised the invariant.
Consider restoring the full 2 s window (or at least 1.5 s) so the test remains
a meaningful guard.
--
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]
