Copilot commented on code in PR #26704:
URL: https://github.com/apache/camel/pull/26704#discussion_r4063058610
##########
core/camel-management/src/test/java/org/apache/camel/management/BacklogDebuggerTest.java:
##########
@@ -325,13 +325,29 @@ public void testBacklogDebuggerRemoveBodyAndHeader()
throws Exception {
assertEquals(1, nodes.size());
assertEquals("foo", nodes.iterator().next());
- // update body and header
- mbeanServer.invoke(on, "removeMessageBodyOnBreakpoint", new Object[] {
"foo" }, new String[] { "java.lang.String" });
- mbeanServer.invoke(on, "removeMessageHeaderOnBreakpoint", new Object[]
{ "foo", "beer" },
+ // remove body, header, and exchange property
+ mbeanServer.invoke(on, "removeMessageBodyOnBreakpoint",
+ new Object[] { "foo" },
+ new String[] { "java.lang.String" });
+ mbeanServer.invoke(on, "removeMessageHeaderOnBreakpoint",
+ new Object[] { "foo", "beer" },
new String[] { "java.lang.String", "java.lang.String" });
- mbeanServer.invoke(on, "removeExchangePropertyOnBreakpoint", new
Object[] { "foo", "food" },
+ mbeanServer.invoke(on, "removeExchangePropertyOnBreakpoint",
+ new Object[] { "foo", "food" },
new String[] { "java.lang.String", "java.lang.String" });
+ // verify removals at breakpoint "foo" where they were performed
+ String xmlAtFoo = (String) mbeanServer.invoke(on,
"dumpTracedMessagesAsXml", new Object[] { "foo", true },
+ new String[] { "java.lang.String", "boolean" });
+ assertNotNull(xmlAtFoo);
+ log.info(xmlAtFoo);
Review Comment:
Logging full traced XML at INFO in a unit test can add a lot of noise to CI
logs. Prefer `debug` level, or only log the XML when an assertion fails (e.g.,
include it in failure messages) so normal successful runs stay quiet.
##########
core/camel-management/src/test/java/org/apache/camel/management/BacklogDebuggerTest.java:
##########
@@ -350,9 +366,7 @@ public void testBacklogDebuggerRemoveBodyAndHeader() throws
Exception {
log.info(xml);
assertTrue(xml.contains("<toNode>bar</toNode>"), "Should contain bar
node");
- assertFalse(xml.contains("<header"), "Should not contain any headers");
- assertFalse(xml.contains("<exchangeProperty key=\"food\""), "Should
not contain exchange property 'food'");
- assertTrue(xml.contains("<body></body>"), "Should not contain our
body");
+ // Note: removals at foo do not persist to bar - new SuspendedExchange
is created at bar
Review Comment:
This change removes the prior assertions about headers/body/properties at
`bar` and replaces them with a comment, which weakens what the test validates.
If the intended behavior is that removals at `foo` do not persist to `bar`,
consider asserting the opposite at `bar` (i.e., headers/property/body are
present as expected) to make that contract explicit and prevent regressions.
##########
core/camel-management/src/test/java/org/apache/camel/management/BacklogDebuggerTest.java:
##########
@@ -325,13 +325,29 @@ public void testBacklogDebuggerRemoveBodyAndHeader()
throws Exception {
assertEquals(1, nodes.size());
assertEquals("foo", nodes.iterator().next());
- // update body and header
- mbeanServer.invoke(on, "removeMessageBodyOnBreakpoint", new Object[] {
"foo" }, new String[] { "java.lang.String" });
- mbeanServer.invoke(on, "removeMessageHeaderOnBreakpoint", new Object[]
{ "foo", "beer" },
+ // remove body, header, and exchange property
+ mbeanServer.invoke(on, "removeMessageBodyOnBreakpoint",
+ new Object[] { "foo" },
+ new String[] { "java.lang.String" });
+ mbeanServer.invoke(on, "removeMessageHeaderOnBreakpoint",
+ new Object[] { "foo", "beer" },
new String[] { "java.lang.String", "java.lang.String" });
- mbeanServer.invoke(on, "removeExchangePropertyOnBreakpoint", new
Object[] { "foo", "food" },
+ mbeanServer.invoke(on, "removeExchangePropertyOnBreakpoint",
+ new Object[] { "foo", "food" },
new String[] { "java.lang.String", "java.lang.String" });
+ // verify removals at breakpoint "foo" where they were performed
+ String xmlAtFoo = (String) mbeanServer.invoke(on,
"dumpTracedMessagesAsXml", new Object[] { "foo", true },
+ new String[] { "java.lang.String", "boolean" });
+ assertNotNull(xmlAtFoo);
+ log.info(xmlAtFoo);
+
+ assertTrue(xmlAtFoo.contains("<toNode>foo</toNode>"), "Should contain
foo node");
+ assertFalse(xmlAtFoo.contains("<header"), "Should not contain any
headers at foo");
+ assertFalse(xmlAtFoo.contains("<exchangeProperty key=\"food\""),
+ "Should not contain exchange property 'food' at foo");
+ assertTrue(xmlAtFoo.contains("<body></body>"), "Body should be empty
at foo");
Review Comment:
These assertions are brittle because XML serialization details can vary
(whitespace, attribute ordering, or empty-element formatting like `<body/>` vs
`<body></body>`). To make the test more stable, parse `xmlAtFoo` as XML and
assert via XPath/DOM (e.g., confirm the `toNode` value, ensure there are zero
header elements, ensure no exchangeProperty with key `food`, and that the body
node is empty).
--
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]
