gnodet commented on code in PR #24858:
URL: https://github.com/apache/camel/pull/24858#discussion_r3655162090


##########
components/camel-plc4x/src/test/java/org/apache/camel/component/plc4x/Plc4XEndpointTest.java:
##########
@@ -56,11 +56,14 @@ public void isSingleton() {
     }
 
     @Test
-    public void doStopBadConnection() throws Exception {
+    void doStopBadConnection() throws Exception {
         PlcConnection plcConnectionMock = mock(PlcConnection.class);
         sut.connection = plcConnectionMock;
         doThrow(new RuntimeException("oh 
noes")).when(plcConnectionMock).close();
         sut.doStop();
+
+        // isConnected() returns false (mock default), so close is skipped
+        verify(plcConnectionMock, never()).close();

Review Comment:
   _Claude Code on behalf of gnodet_
   
   Good catch — the `doThrow` setup was misleading since `isConnected()` 
returns false on the mock, so `close()` is never called anyway. Removed the 
confusing `doThrow` and renamed the test to `doStopDisconnectedConnection` to 
clarify intent: when `isConnected()` is false, `doStop()` should skip closing.



##########
components/camel-salesforce/camel-salesforce-component/src/test/java/org/apache/camel/component/salesforce/api/dto/composite/SObjectNodeTest.java:
##########
@@ -130,8 +131,11 @@ public void shouldCreateNode() {
     }
 
     @Test
-    public void shouldCreateNodeWithoutChildRecords() {
-        new SObjectNode(new SObjectTree(), simpleAccount);
+    void shouldCreateNodeWithoutChildRecords() {
+        SObjectNode node = new SObjectNode(new SObjectTree(), simpleAccount);
+        assertNotNull(node);

Review Comment:
   _Claude Code on behalf of gnodet_
   
   You're right — `new SObjectNode()` can never return null. Removed the 
pointless `assertNotNull(node)` and kept the meaningful assertions 
(`assertSame` for the object and `assertEquals` for the size).



##########
catalog/camel-report-maven-plugin/src/test/java/org/apache/camel/maven/htmlxlsx/process/CoverageResultsProcessorTest.java:
##########
@@ -181,18 +190,65 @@ public void testGenerateEipStatistics() throws 
IllegalAccessException, IOExcepti
     }
 
     @Test
-    public void testGenerateChildEipStatistics() {
+    void testGenerateChildEipStatistics() {
 
-    }
+        ChildEip childEip = new ChildEip();
 
-    @Test
-    public void testGenerateExcel() {
+        // Add an EipAttribute entry (simulates a child EIP node with coverage 
data)
+        EipAttribute eipAttribute = new EipAttribute();
+        eipAttribute.setId("setBody");
+        eipAttribute.setIndex(5);
+        eipAttribute.setExchangesTotal(3);
+        eipAttribute.setTotalProcessingTime(42);
+        Properties props = new Properties();
+        props.put("uri", "direct:test");
+        eipAttribute.setProperties(props);
+        childEip.getEipAttributeMap().put("setBody", eipAttribute);
+
+        // Add a String entry (simulates a simple property like a constant 
expression)
+        childEip.getEipAttributeMap().put("constant", "Hello World");
+
+        ChildEipStatistic childEipStatistic = new ChildEipStatistic();
 
+        processor.generateChildEipStatistics(childEip, childEipStatistic);
+
+        Map<Integer, EipStatistic> resultMap = 
childEipStatistic.getEipStatisticMap();
+
+        assertAll(
+                () -> assertNotNull(resultMap),
+                () -> assertEquals(2, resultMap.size()),
+                // EipAttribute entry: keyed by index=5, tested because 
exchangesTotal > 0
+                () -> assertNotNull(resultMap.get(5)),
+                () -> assertEquals("setBody", resultMap.get(5).getId()),
+                () -> assertTrue(resultMap.get(5).isTested()),
+                () -> assertEquals(42, 
resultMap.get(5).getTotalProcessingTime()),
+                // String entry: keyed by 0, wraps value in Properties
+                () -> assertNotNull(resultMap.get(0)),
+                () -> assertEquals("constant", resultMap.get(0).getId()),
+                () -> assertEquals("Hello World", 
resultMap.get(0).getProperties().get("value")));
     }
 
     @Test
-    public void testGenerateHtml() {
+    void testGenerateHtml() throws IllegalAccessException, IOException {
+
+        Mockito
+                .doNothing()
+                .when(processor).writeDetailsAsHtml(any(RouteStatistic.class), 
any(File.class));
+
+        @SuppressWarnings("unchecked")
+        Map<String, RouteStatistic> routeStatisticMap
+                = (Map<String, RouteStatistic>) 
FieldUtils.readDeclaredField(processor, "routeStatisticMap", true);
+
+        RouteStatistic stat1 = new RouteStatistic();
+        stat1.setId("route1");
+        RouteStatistic stat2 = new RouteStatistic();
+        stat2.setId("route2");
+        routeStatisticMap.put("route1", stat1);
+        routeStatisticMap.put("route2", stat2);
 
+        processor.generateHtml(htmlPath());
+
+        Mockito.verify(processor, 
Mockito.times(2)).writeDetailsAsHtml(any(RouteStatistic.class), 
any(File.class));

Review Comment:
   _Claude Code on behalf of gnodet_
   
   Renamed to `testHtmlGenerationCalledWhenHtmlFormatAsked` as suggested.



-- 
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]

Reply via email to