oscerd commented on code in PR #2892:
URL: https://github.com/apache/camel-kamelets/pull/2892#discussion_r3497041074


##########
library/camel-kamelets-catalog/src/test/java/org/apache/camel/kamelets/catalog/KameletsCatalogTest.java:
##########
@@ -126,6 +126,18 @@ void testGetKameletsByNamespace() throws Exception {
         assertTrue(c.isEmpty());
     }
 
+    @Test
+    void testLookupMethodsNeverReturnNull() throws Exception {
+        // Hardened contract: the label/annotation lookups return a non-null
+        // (possibly empty) list and never NPE on a Kamelet that lacks the
+        // queried label/annotation.
+        assertNotNull(catalog.getKameletsByName("does-not-exist"));

Review Comment:
   Good point — added positive (non-empty) assertions for known values 
(`source` type, `AWS` namespace, `AWS S3` group, `Apache Software Foundation` 
provider), so the test now covers both the populated and the empty/no-match 
contracts.
   
   _Claude Code on behalf of Andrea Cosentino (@oscerd)_
   



##########
library/camel-kamelets-catalog/src/test/java/org/apache/camel/kamelets/catalog/KameletsCatalogTest.java:
##########
@@ -126,6 +126,18 @@ void testGetKameletsByNamespace() throws Exception {
         assertTrue(c.isEmpty());
     }
 
+    @Test
+    void testLookupMethodsNeverReturnNull() throws Exception {
+        // Hardened contract: the label/annotation lookups return a non-null
+        // (possibly empty) list and never NPE on a Kamelet that lacks the
+        // queried label/annotation.
+        assertNotNull(catalog.getKameletsByName("does-not-exist"));
+        assertTrue(catalog.getKameletsByType("does-not-exist").isEmpty());
+        assertTrue(catalog.getKameletsByNamespace("does-not-exist").isEmpty());
+        assertTrue(catalog.getKameletsByGroups("does-not-exist").isEmpty());
+        assertTrue(catalog.getKameletByProvider("does-not-exist").isEmpty());

Review Comment:
   Thanks — agreed AssertJ reads nicely. This module has no AssertJ dependency 
today and the whole `KameletsCatalogTest` uses JUnit Jupiter assertions, so to 
avoid adding a dependency and mixing two styles in a single test I kept JUnit 
but added descriptive failure messages that print the actual list (a failure 
now shows what was unexpectedly returned, e.g. `expected an empty list for an 
unknown type but got [...]`). Happy to standardise the module's tests on 
AssertJ as a separate change if the team would prefer that.
   
   _Claude Code on behalf of Andrea Cosentino (@oscerd)_
   



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