feat: update comparison logic to throw exceptions for KryoPlaceholder instances

Author: HeshamHM28

codeflash-java-runtime/src/main/java/com/codeflash/Comparator.java

@@ -284,10 +284,18 @@ private static boolean compareInternal(Object orig, Object newObj,
             return false;
         }
 
-        // KryoPlaceholder means the Serializer couldn't serialize this part.
-        // Skip comparison for placeholder fields — we can't compare what we couldn't serialize.
-        if (orig instanceof KryoPlaceholder || newObj instanceof KryoPlaceholder) {
-            return true;
+        // Detect and reject KryoPlaceholder
+        if (orig instanceof KryoPlaceholder) {
+            KryoPlaceholder p = (KryoPlaceholder) orig;
+            throw new KryoPlaceholderAccessException(
+                "Cannot compare: original contains placeholder for unserializable object",
+                p.getObjType(), p.getPath());
+        }
+        if (newObj instanceof KryoPlaceholder) {
+            KryoPlaceholder p = (KryoPlaceholder) newObj;
+            throw new KryoPlaceholderAccessException(
+                "Cannot compare: new object contains placeholder for unserializable object",
+                p.getObjType(), p.getPath());
         }
 
         // Handle exceptions specially

codeflash-java-runtime/src/test/java/com/codeflash/ComparatorCorrectnessTest.java

@@ -60,9 +60,9 @@ void testAllPlaceholderSkips() throws Exception {
         String json = Comparator.compareDatabases(originalDb.toString(), candidateDb.toString());
         Map<String, Object> result = parseJson(json);
 
-        // Placeholders are skipped during comparison (treated as matching) — counts as actual comparison
-        assertTrue((Boolean) result.get("equivalent"));
-        assertEquals(1, ((Number) result.get("actualComparisons")).intValue());
+        assertFalse((Boolean) result.get("equivalent"));
+        assertEquals(0, ((Number) result.get("actualComparisons")).intValue());
+        assertTrue(((Number) result.get("skippedPlaceholders")).intValue() > 0);
     }
 
     @Test
@@ -108,8 +108,8 @@ void testMixedRealAndPlaceholder() throws Exception {
         Map<String, Object> result = parseJson(json);
 
         assertTrue((Boolean) result.get("equivalent"));
-        // All 3 compared: 2 real values + 1 placeholder (placeholder skipped during deep compare)
-        assertEquals(3, ((Number) result.get("actualComparisons")).intValue());
+        assertEquals(2, ((Number) result.get("actualComparisons")).intValue());
+        assertEquals(1, ((Number) result.get("skippedPlaceholders")).intValue());
     }
 
     @Test

codeflash-java-runtime/src/test/java/com/codeflash/ComparatorTest.java

@@ -311,47 +311,49 @@ void testBothNullMessages() {
     class PlaceholderTests {
 
         @Test
-        @DisplayName("original contains placeholder: skipped, treated as matching")
+        @DisplayName("original contains placeholder: throws exception")
         void testOriginalPlaceholder() {
             KryoPlaceholder placeholder = new KryoPlaceholder(
                 "java.net.Socket", "<socket>", "error", "path"
             );
 
-            // Placeholders are skipped — comparison returns true (not comparable, so skip)
-            assertTrue(Comparator.compare(placeholder, "anything"));
+            assertThrows(KryoPlaceholderAccessException.class, () -> {
+                Comparator.compare(placeholder, "anything");
+            });
         }
 
         @Test
-        @DisplayName("new contains placeholder: skipped, treated as matching")
+        @DisplayName("new contains placeholder: throws exception")
         void testNewPlaceholder() {
             KryoPlaceholder placeholder = new KryoPlaceholder(
                 "java.net.Socket", "<socket>", "error", "path"
             );
 
-            assertTrue(Comparator.compare("anything", placeholder));
+            assertThrows(KryoPlaceholderAccessException.class, () -> {
+                Comparator.compare("anything", placeholder);
+            });
         }
 
         @Test
-        @DisplayName("placeholder in nested structure: skipped, rest compared normally")
+        @DisplayName("placeholder in nested structure: throws exception")
         void testNestedPlaceholder() {
             KryoPlaceholder placeholder = new KryoPlaceholder(
                 "java.net.Socket", "<socket>", "error", "data.socket"
             );
 
             Map<String, Object> map1 = new HashMap<>();
             map1.put("socket", placeholder);
-            map1.put("name", "same");
 
             Map<String, Object> map2 = new HashMap<>();
             map2.put("socket", "different");
-            map2.put("name", "same");
 
-            // Placeholder field skipped, "name" field compared normally → equivalent
-            assertTrue(Comparator.compare(map1, map2));
+            assertThrows(KryoPlaceholderAccessException.class, () -> {
+                Comparator.compare(map1, map2);
+            });
         }
 
         @Test
-        @DisplayName("compareWithDetails with placeholder returns equal (skipped)")
+        @DisplayName("compareWithDetails captures error message")
         void testCompareWithDetails() {
             KryoPlaceholder placeholder = new KryoPlaceholder(
                 "java.net.Socket", "<socket>", "error", "path"
@@ -360,8 +362,9 @@ void testCompareWithDetails() {
             Comparator.ComparisonResult result =
                 Comparator.compareWithDetails(placeholder, "anything");
 
-            assertTrue(result.isEqual());
-            assertFalse(result.hasError());
+            assertFalse(result.isEqual());
+            assertTrue(result.hasError());
+            assertNotNull(result.getErrorMessage());
         }
     }
 

codeflash-java-runtime/src/test/java/com/codeflash/SerializerTest.java

@@ -268,8 +268,8 @@ void testClassWithUnserializableAttribute() throws Exception {
     class PlaceholderAccessTests {
 
         @Test
-        @DisplayName("comparing objects with placeholder skips the placeholder field")
-        void testPlaceholderComparisonSkips() throws Exception {
+        @DisplayName("comparing objects with placeholder throws KryoPlaceholderAccessException")
+        void testPlaceholderComparisonThrowsException() throws Exception {
             try (Socket socket = new Socket()) {
                 Map<String, Object> data = new LinkedHashMap<>();
                 data.put("socket", socket);
@@ -279,8 +279,9 @@ void testPlaceholderComparisonSkips() throws Exception {
 
                 KryoPlaceholder placeholder = (KryoPlaceholder) reloaded.get("socket");
 
-                // Placeholders are skipped during comparison — treated as matching
-                assertTrue(Comparator.compare(placeholder, "anything"));
+                assertThrows(KryoPlaceholderAccessException.class, () -> {
+                    Comparator.compare(placeholder, "anything");
+                });
             }
         }
     }