weiqingy commented on code in PR #771:
URL: https://github.com/apache/flink-agents/pull/771#discussion_r3366805718
##########
integrations/mcp/src/test/java/org/apache/flink/agents/integrations/mcp/MCPServerTest.java:
##########
@@ -71,6 +74,28 @@ void testSimpleConstructor() {
assertThat(server.getAuth()).isNull();
}
+ @Test
+ @DisabledOnJre(JRE.JAVA_11)
+ @DisplayName("Read timeout from ResourceDescriptor")
+ void testTimeoutFromResourceDescriptor() {
+ ResourceDescriptor descriptor =
+ ResourceDescriptor.Builder.newBuilder(ResourceName.MCP_SERVER)
+ .addInitialArgument("endpoint", DEFAULT_ENDPOINT)
+ .addInitialArgument("timeout", 60)
+ .build();
+
+ MCPServer server =
+ new MCPServer(
+ descriptor,
+ ResourceContext.fromGetResource(
+ (name, type) -> {
+ throw new UnsupportedOperationException(
+ "No dependencies expected");
+ }));
+
+ assertThat(server.getTimeoutSeconds()).isEqualTo(60);
Review Comment:
The bug here was a cross-language wire-key mismatch — Java emitted and read
`"timeoutSeconds"` while Python and the descriptor use `"timeout"`. This test
covers the descriptor→field path, and the existing `testJsonSerialization`
round-trips through Jackson. But since that round-trip's write and read sides
both go through `FIELD_TIMEOUT`, it stays green even if the constant were
renamed back to `"timeoutSeconds"` — the one thing none of the tests pin is the
literal wire key that Python depends on.
Would it be worth asserting on the serialized string so a future rename
can't silently reintroduce the same Java/Python mismatch? Something like, in
`testJsonSerialization`:
```java
assertThat(json).contains("\"timeout\"");
```
--
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]
