apupier commented on code in PR #24858:
URL: https://github.com/apache/camel/pull/24858#discussion_r3643720361
##########
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:
based on what is tested, I think we should rename the test to something like
testHtmlGenerationCalledWhenHtmlFormatAsked because it is not testing if the
html file is really generated neither checking the content of it
##########
components/camel-mllp/src/test/java/org/apache/camel/component/mllp/MllpTcpClientProducerRequiredEndOfDataWithoutValidationTest.java:
##########
Review Comment:
I do not understand the purpose of the changes removing the
assertDoNotThrows by a try catch throwing a RuntimeCamelException
##########
components/camel-stream/src/test/java/org/apache/camel/component/stream/StreamRouteBuilderTest.java:
##########
@@ -16,20 +16,30 @@
*/
package org.apache.camel.component.stream;
+import org.apache.camel.Exchange;
import org.apache.camel.builder.RouteBuilder;
import org.apache.camel.test.junit6.CamelTestSupport;
import org.junit.jupiter.api.Test;
-public class StreamRouteBuilderTest extends CamelTestSupport {
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+
+class StreamRouteBuilderTest extends CamelTestSupport {
@Test
- public void testStringContent() {
- template.sendBody("direct:start", "this is text\n");
+ void testStringContent() {
+ Exchange result = template.send("direct:start", exchange ->
exchange.getIn().setBody("this is text\n"));
+ assertNotNull(result);
+ assertFalse(result.isFailed(), "Sending string content should not
cause an exchange failure");
+ assertNotNull(result.getIn().getBody(), "Exchange body should be
preserved after sending");
Review Comment:
should check equality with body sent
##########
components/camel-stream/src/test/java/org/apache/camel/component/stream/StreamSystemErrTest.java:
##########
@@ -16,23 +16,33 @@
*/
package org.apache.camel.component.stream;
+import org.apache.camel.Exchange;
import org.apache.camel.builder.RouteBuilder;
import org.apache.camel.test.junit6.CamelTestSupport;
import org.junit.jupiter.api.Test;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+
/**
* Unit test for System.err
*/
-public class StreamSystemErrTest extends CamelTestSupport {
+class StreamSystemErrTest extends CamelTestSupport {
@Test
- public void testStringContent() {
- template.sendBody("direct:in", "Hello Text World\n");
+ void testStringContent() {
+ Exchange result = template.send("direct:in", exchange ->
exchange.getIn().setBody("Hello Text World\n"));
+ assertNotNull(result);
+ assertFalse(result.isFailed(), "Sending string content to stream:err
should not cause an exchange failure");
+ assertNotNull(result.getIn().getBody(), "Exchange body should be
preserved after sending");
Review Comment:
should check equality with body sent
##########
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:
I do not understand. there is mock Throwing an exception when calling .close
and here we check that close is not called
##########
components/camel-stream/src/test/java/org/apache/camel/component/stream/StreamRouteBuilderTest.java:
##########
@@ -16,20 +16,30 @@
*/
package org.apache.camel.component.stream;
+import org.apache.camel.Exchange;
import org.apache.camel.builder.RouteBuilder;
import org.apache.camel.test.junit6.CamelTestSupport;
import org.junit.jupiter.api.Test;
-public class StreamRouteBuilderTest extends CamelTestSupport {
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+
+class StreamRouteBuilderTest extends CamelTestSupport {
@Test
- public void testStringContent() {
- template.sendBody("direct:start", "this is text\n");
+ void testStringContent() {
+ Exchange result = template.send("direct:start", exchange ->
exchange.getIn().setBody("this is text\n"));
+ assertNotNull(result);
+ assertFalse(result.isFailed(), "Sending string content should not
cause an exchange failure");
+ assertNotNull(result.getIn().getBody(), "Exchange body should be
preserved after sending");
}
@Test
- public void testBinaryContent() {
- template.sendBody("direct:start", "This is bytes\n".getBytes());
+ void testBinaryContent() {
+ Exchange result = template.send("direct:start", exchange ->
exchange.getIn().setBody("This is bytes\n".getBytes()));
+ assertNotNull(result);
+ assertFalse(result.isFailed(), "Sending binary content should not
cause an exchange failure");
+ assertNotNull(result.getIn().getBody(), "Exchange body should be
preserved after sending");
Review Comment:
would be better to check equality with body sent
##########
components/camel-mllp/src/test/java/org/apache/camel/test/executor/PooledExecutorTest.java:
##########
@@ -54,6 +57,9 @@ public void testAddRunnable() throws Exception {
log.info("Starting second set of runnables");
startRunnables(runnableCount, runCount);
+
+ assertNotNull(instance.executor, "Executor should still be active
after submitting runnables");
+ assertFalse(instance.executor.isShutdown(), "Executor should not be
shut down after submitting runnables");
Review Comment:
I think a better assertion would be to verify what the runnablecount and
runCount are doing
##########
components/camel-netty/src/test/java/org/apache/camel/component/netty/MainNettyCustomCodecTest.java:
##########
@@ -56,7 +59,11 @@ public void configure() {
});
main.configure().withDurationMaxMessages(2);
main.configure().withDurationMaxSeconds(5);
+
main.run();
+ // run() blocks until duration/message limit is reached, then returns.
+ // The route processor inside validates data equality and throws if
mismatched.
+ assertNotNull(main.getCamelContext(), "CamelContext should have been
created during run");
Review Comment:
I think this should check that routes executed correctly
##########
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:
this cannot be null
##########
components/camel-stream/src/test/java/org/apache/camel/component/stream/StreamEncodingTest.java:
##########
@@ -16,21 +16,28 @@
*/
package org.apache.camel.component.stream;
+import org.apache.camel.Exchange;
import org.apache.camel.builder.RouteBuilder;
import org.apache.camel.test.junit6.CamelTestSupport;
import org.junit.jupiter.api.Test;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+
/**
* Unit test for encoding option
*/
-public class StreamEncodingTest extends CamelTestSupport {
+class StreamEncodingTest extends CamelTestSupport {
@Test
- public void testStringContent() {
+ void testStringContent() {
// include a UTF-8 char in the text \u0E08 is a Thai elephant
String body = "Hello Thai Elephant \u0E08";
- template.sendBody("direct:in", body);
+ Exchange result = template.send("direct:in", exchange ->
exchange.getIn().setBody(body));
+ assertNotNull(result);
+ assertFalse(result.isFailed(), "Sending UTF-8 content should not cause
an exchange failure");
+ assertNotNull(result.getIn().getBody(), "Exchange body should be
preserved after sending");
Review Comment:
would be better to check for the content equality with body
##########
components/camel-stream/src/test/java/org/apache/camel/component/stream/StreamSystemErrTest.java:
##########
@@ -16,23 +16,33 @@
*/
package org.apache.camel.component.stream;
+import org.apache.camel.Exchange;
import org.apache.camel.builder.RouteBuilder;
import org.apache.camel.test.junit6.CamelTestSupport;
import org.junit.jupiter.api.Test;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+
/**
* Unit test for System.err
*/
-public class StreamSystemErrTest extends CamelTestSupport {
+class StreamSystemErrTest extends CamelTestSupport {
@Test
- public void testStringContent() {
- template.sendBody("direct:in", "Hello Text World\n");
+ void testStringContent() {
+ Exchange result = template.send("direct:in", exchange ->
exchange.getIn().setBody("Hello Text World\n"));
+ assertNotNull(result);
+ assertFalse(result.isFailed(), "Sending string content to stream:err
should not cause an exchange failure");
+ assertNotNull(result.getIn().getBody(), "Exchange body should be
preserved after sending");
}
@Test
- public void testBinaryContent() {
- template.sendBody("direct:in", "Hello Bytes World\n".getBytes());
+ void testBinaryContent() {
+ Exchange result = template.send("direct:in", exchange ->
exchange.getIn().setBody("Hello Bytes World\n".getBytes()));
+ assertNotNull(result);
+ assertFalse(result.isFailed(), "Sending binary content to stream:err
should not cause an exchange failure");
+ assertNotNull(result.getIn().getBody(), "Exchange body should be
preserved after sending");
Review Comment:
should check equality with body sent
##########
components/camel-undertow/src/test/java/org/apache/camel/component/undertow/rest/RestUndertowProducerEncodingTest.java:
##########
@@ -16,24 +16,34 @@
*/
package org.apache.camel.component.undertow.rest;
+import org.apache.camel.Exchange;
import org.apache.camel.builder.RouteBuilder;
import org.apache.camel.component.undertow.BaseUndertowTest;
import org.junit.jupiter.api.Test;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
-public class RestUndertowProducerEncodingTest extends BaseUndertowTest {
+class RestUndertowProducerEncodingTest extends BaseUndertowTest {
@Test
- public void testSelect() {
-
template.sendBody("rest:get:bw-web-api/v1/objects/timesheets?companyId=RD&select=personId,personName",
"Hello World");
+ void testSelect() {
+ Exchange result = template.request(
+
"rest:get:bw-web-api/v1/objects/timesheets?companyId=RD&select=personId,personName",
+ exchange -> exchange.getIn().setBody("Hello World"));
+ assertNotNull(result);
+ assertFalse(result.isFailed(), "Request with select parameter should
complete without failure");
Review Comment:
should check the select content
##########
components/camel-xslt-saxon/src/test/java/org/apache/camel/component/xslt/PayloadWithDefaultNamespaceTest.java:
##########
@@ -35,7 +37,8 @@ public void configure() {
}
@Test
- public void testTransformWithDefaultNamespace() {
- template.sendBody("direct:start", PAYLOAD);
+ void testTransformWithDefaultNamespace() {
+ Object result = template.requestBody("direct:start", PAYLOAD);
+ assertNotNull(result, "XSLT transformation with default namespace
should produce output");
Review Comment:
check the content of the output
##########
components/camel-ai/camel-langchain4j-embeddingstore/src/test/java/org/apache/camel/component/langchain4j/embeddings/LangChain4jEmbeddingStoreComponentTest.java:
##########
@@ -51,16 +53,16 @@ protected CamelContext createCamelContext() throws
Exception {
}
@Test
- public void testSimpleEmbedding() {
-
+ public void testSimpleEmbedding() throws Exception {
EmbeddingModel embeddingModel = new AllMiniLmL6V2EmbeddingModel();
TextSegment segment1 = TextSegment.from("I like football.");
Embedding testEmbedding = embeddingModel.embed(segment1).content();
+ assertNotNull(testEmbedding, "embedding model should produce a
non-null embedding");
Message first = fluentTemplate.to("langchain4j-embeddingstore:first")
.withBody(testEmbedding)
.request(Message.class);
-
+ assertNotNull(first, "response message should not be null");
Review Comment:
there is a weaviate container available for tests
https://github.com/apache/camel/tree/main/test-infra/camel-test-infra-weaviate
##########
components/camel-undertow/src/test/java/org/apache/camel/component/undertow/rest/RestUndertowProducerEncodingTest.java:
##########
@@ -16,24 +16,34 @@
*/
package org.apache.camel.component.undertow.rest;
+import org.apache.camel.Exchange;
import org.apache.camel.builder.RouteBuilder;
import org.apache.camel.component.undertow.BaseUndertowTest;
import org.junit.jupiter.api.Test;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
-public class RestUndertowProducerEncodingTest extends BaseUndertowTest {
+class RestUndertowProducerEncodingTest extends BaseUndertowTest {
@Test
- public void testSelect() {
-
template.sendBody("rest:get:bw-web-api/v1/objects/timesheets?companyId=RD&select=personId,personName",
"Hello World");
+ void testSelect() {
+ Exchange result = template.request(
+
"rest:get:bw-web-api/v1/objects/timesheets?companyId=RD&select=personId,personName",
+ exchange -> exchange.getIn().setBody("Hello World"));
+ assertNotNull(result);
+ assertFalse(result.isFailed(), "Request with select parameter should
complete without failure");
}
@Test
- public void testFilter() {
-
template.sendBody("rest:get:bw-web-api/v1/objects/timesheets?companyId=RD&select=personId,personName"
- + "&filter=date(time/date) ge 2020-06-01 and
personId eq 'R10019'",
- "Bye World");
+ void testFilter() {
+ Exchange result = template.request(
+
"rest:get:bw-web-api/v1/objects/timesheets?companyId=RD&select=personId,personName"
+ + "&filter=date(time/date) ge
2020-06-01 and personId eq 'R10019'",
+ exchange -> exchange.getIn().setBody("Bye World"));
+ assertNotNull(result);
+ assertFalse(result.isFailed(), "Request with filter parameter should
complete without failure");
Review Comment:
should check the filter result
--
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]