apupier commented on code in PR #24858:
URL: https://github.com/apache/camel/pull/24858#discussion_r3612713158
##########
components/camel-box/camel-box-component/src/test/java/org/apache/camel/component/box/BoxFilesManagerIT.java:
##########
@@ -133,6 +133,7 @@ public void testCreateFileSharedLink() {
@Test
public void testDeleteFile() {
+ assertNotNull(testFile.getID(), "test file should have an ID before
deletion");
// using String message body for single parameter "fileId"
requestBody("direct://DELETEFILE", testFile.getID());
}
Review Comment:
also need an assertion checkign that the file is really deleted
##########
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:
Shouldn't we test that there is some specific content? In case it returns an
empty string this woudl be a problem too
##########
components/camel-azure/camel-azure-storage-blob/src/test/java/org/apache/camel/component/azure/storage/blob/BlobConsumerTest.java:
##########
@@ -79,8 +85,12 @@ void testDeleteAfterReadConfigured() throws Exception {
"azure-storage-blob://camelazure/container?blobName=blob&credentials=#creds&credentialType=SHARED_KEY_CREDENTIAL"
+ "&deleteAfterRead=true");
+ assertNotNull(endpoint, "endpoint should be created from URI");
+ assertTrue(endpoint.getConfiguration().isDeleteAfterRead(),
"deleteAfterRead should be enabled");
+
BlobConsumer consumer = (BlobConsumer)
endpoint.createConsumer(exchange -> {
});
+ assertNotNull(consumer, "consumer should be created successfully");
Review Comment:
these assertions are not really checking a lot more given thatit would have
thrown an NPE in thhe test on next steps.
the whole tests sounds like needs a whole refactor to really test more
things. i do not understand the catch of an exception to rethrown an exception,
mentioning tht ait is hould happen but it is not otherwise the test would fail.
i think it would be better to leave as it is and rethink to rewrite fully
the test. it will allow to keep the sonar issue in sight and avoid to forget
about it
##########
components/camel-ai/camel-a2a/src/test/java/org/apache/camel/component/a2a/state/InMemoryTaskStoreTest.java:
##########
@@ -180,6 +180,10 @@ void subscribeAndUnsubscribe() {
store.addSubscriber("t1", subscriber);
store.removeSubscriber("t1", subscriber);
+
+ // Task should still be intact after subscribe/unsubscribe cycle
+ assertThat(store.get("t1")).isNotNull();
+
assertThat(store.get("t1").status().state()).isEqualTo(TaskState.WORKING);
Review Comment:
I think we should test tha tthe subscriber is really removed. Maybe by
firing a store.notifySubscribers and ensuring that the subscriber is not called.
##########
components/camel-box/camel-box-component/src/test/java/org/apache/camel/component/box/BoxFoldersManagerIT.java:
##########
@@ -96,6 +96,7 @@ public void testCreateFolderByPath() {
@Test
public void testDeleteFolder() {
+ assertNotNull(testFolder.getID(), "test folder should have an ID
before deletion");
// using String message body for single parameter "folderId"
requestBody("direct://DELETEFOLDER", testFolder.getID());
Review Comment:
also need an assertion checkign that the file is really deleted
##########
components/camel-dhis2/camel-dhis2-api/src/test/java/org/apache/camel/component/dhis2/api/Dhis2DeleteTestCase.java:
##########
@@ -71,12 +72,14 @@ public String getUrl() {
@Test
public void testResourceGivenMapOfListsQueryParams() {
Dhis2Delete dhis2Delete = new Dhis2Delete(dhis2Client);
- dhis2Delete.resource(null, null, Map.of("foo", List.of("bar")));
+ InputStream result = dhis2Delete.resource(null, null, Map.of("foo",
List.of("bar")));
+ assertNotNull(result, "resource() should return a non-null
InputStream");
}
@Test
public void testResourceGivenMapOfStringsQueryParams() {
Dhis2Delete dhis2Delete = new Dhis2Delete(dhis2Client);
- dhis2Delete.resource(null, null, Map.of("foo", "bar"));
+ InputStream result = dhis2Delete.resource(null, null, Map.of("foo",
"bar"));
+ assertNotNull(result, "resource() should return a non-null
InputStream");
Review Comment:
should check the content of the inputstream
##########
components/camel-dhis2/camel-dhis2-api/src/test/java/org/apache/camel/component/dhis2/api/Dhis2DeleteTestCase.java:
##########
@@ -71,12 +72,14 @@ public String getUrl() {
@Test
public void testResourceGivenMapOfListsQueryParams() {
Dhis2Delete dhis2Delete = new Dhis2Delete(dhis2Client);
- dhis2Delete.resource(null, null, Map.of("foo", List.of("bar")));
+ InputStream result = dhis2Delete.resource(null, null, Map.of("foo",
List.of("bar")));
+ assertNotNull(result, "resource() should return a non-null
InputStream");
Review Comment:
should check the content of the inputstream
##########
components/camel-cxf/camel-cxf-soap/src/test/java/org/apache/camel/component/cxf/jaxws/CxfSchemaValidationTest.java:
##########
@@ -106,8 +106,13 @@ public void configure() {
@Test
public void schemaValidationDisabledServerTest() throws Exception {
- // invoke the service with a non-valid message
- invokeService(serviceAddressValidationDisabled,
RandomStringUtils.secure().next(40, true, true));
+ String longPersonId = RandomStringUtils.secure().next(40, true, true);
+ // invoke the service with a non-valid message; should succeed when
validation is disabled
+ invokeService(serviceAddressValidationDisabled, longPersonId);
+ // Verify the personId exceeded the schema maxLength constraint (30
chars),
+ // confirming that schema validation was indeed disabled for this
endpoint
+ assertTrue(longPersonId.length() > 30,
+ "personId should exceed schema maxLength to confirm validation
is disabled");
Review Comment:
this would be an Assume more than an assertion. i tis par tof the parameters
used for the test
##########
components/camel-cxf/camel-cxf-spring-soap/src/test/java/org/apache/camel/component/cxf/CxfWsdlFirstPayloadModeTest.java:
##########
@@ -43,8 +44,9 @@ protected ClassPathXmlApplicationContext
createApplicationContext() {
@Override
@Test
- public void testInvokingServiceWithCamelProducer() throws Exception {
- // this test does not apply to PAYLOAD mode
+ public void testInvokingServiceWithCamelProducer() {
+ // this test does not apply to PAYLOAD mode; override to verify
context is still valid
+ assertNotNull(context, "CamelContext should be available even though
this test is not applicable to PAYLOAD mode");
Review Comment:
Based on comment that this test does not apply to PayLoadMode, it must be
disabled
##########
components/camel-dhis2/camel-dhis2-api/src/test/java/org/apache/camel/component/dhis2/api/Dhis2PostTestCase.java:
##########
@@ -71,12 +72,14 @@ public String getUrl() {
@Test
public void testResourceGivenMapOfListsQueryParams() {
Dhis2Post dhis2Post = new Dhis2Post(dhis2Client);
- dhis2Post.resource(null, null, Map.of("foo", List.of("bar")));
+ InputStream result = dhis2Post.resource(null, null, Map.of("foo",
List.of("bar")));
+ assertNotNull(result, "resource() should return a non-null
InputStream");
}
@Test
public void testResourceGivenMapOfStringsQueryParams() {
Dhis2Post dhis2Post = new Dhis2Post(dhis2Client);
- dhis2Post.resource(null, null, Map.of("foo", "bar"));
+ InputStream result = dhis2Post.resource(null, null, Map.of("foo",
"bar"));
+ assertNotNull(result, "resource() should return a non-null
InputStream");
Review Comment:
should check the content of the inputstream
##########
components/camel-dhis2/camel-dhis2-api/src/test/java/org/apache/camel/component/dhis2/api/Dhis2PutTestCase.java:
##########
@@ -71,12 +72,14 @@ public String getUrl() {
@Test
public void testResourceGivenMapOfListsQueryParams() {
Dhis2Put dhis2Put = new Dhis2Put(dhis2Client);
- dhis2Put.resource(null, null, Map.of("foo", List.of("bar")));
+ InputStream result = dhis2Put.resource(null, null, Map.of("foo",
List.of("bar")));
+ assertNotNull(result, "resource() should return a non-null
InputStream");
}
@Test
public void testResourceGivenMapOfStringsQueryParams() {
Dhis2Put dhis2Put = new Dhis2Put(dhis2Client);
- dhis2Put.resource(null, null, Map.of("foo", "bar"));
+ InputStream result = dhis2Put.resource(null, null, Map.of("foo",
"bar"));
+ assertNotNull(result, "resource() should return a non-null
InputStream");
Review Comment:
should check the content of the inputstream
##########
components/camel-flink/src/test/java/org/apache/camel/component/flink/DataStreamProducerTest.java:
##########
@@ -41,14 +44,16 @@ public class DataStreamProducerTest extends
CamelTestSupport {
@Test
public void shouldExecuteDataStreamCallback() {
- template.sendBodyAndHeader(flinkDataStreamUri, null,
FlinkConstants.FLINK_DATASTREAM_CALLBACK_HEADER,
- new VoidDataStreamCallback() {
- @Override
- public void doOnDataStream(DataStream ds, Object...
payloads) throws Exception {
- // Just verify the callback is executed
- ds.print();
- }
- });
+ Exchange result = template.send(flinkDataStreamUri, exchange -> {
+
exchange.getIn().setHeader(FlinkConstants.FLINK_DATASTREAM_CALLBACK_HEADER,
+ new VoidDataStreamCallback() {
+ @Override
+ public void doOnDataStream(DataStream ds, Object...
payloads) throws Exception {
+ ds.print();
+ }
+ });
+ });
+ assertNull(result.getException(), "DataStream callback should execute
without error");
}
Review Comment:
the assertion is still not checking that the callback is executed
##########
components/camel-dhis2/camel-dhis2-api/src/test/java/org/apache/camel/component/dhis2/api/Dhis2PutTestCase.java:
##########
@@ -71,12 +72,14 @@ public String getUrl() {
@Test
public void testResourceGivenMapOfListsQueryParams() {
Dhis2Put dhis2Put = new Dhis2Put(dhis2Client);
- dhis2Put.resource(null, null, Map.of("foo", List.of("bar")));
+ InputStream result = dhis2Put.resource(null, null, Map.of("foo",
List.of("bar")));
+ assertNotNull(result, "resource() should return a non-null
InputStream");
Review Comment:
should check the content of the inputstream
##########
components/camel-dhis2/camel-dhis2-api/src/test/java/org/apache/camel/component/dhis2/api/Dhis2PostTestCase.java:
##########
@@ -71,12 +72,14 @@ public String getUrl() {
@Test
public void testResourceGivenMapOfListsQueryParams() {
Dhis2Post dhis2Post = new Dhis2Post(dhis2Client);
- dhis2Post.resource(null, null, Map.of("foo", List.of("bar")));
+ InputStream result = dhis2Post.resource(null, null, Map.of("foo",
List.of("bar")));
+ assertNotNull(result, "resource() should return a non-null
InputStream");
Review Comment:
should check the content of the inputstream
--
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]
