rahulgoswami commented on code in PR #4279:
URL: https://github.com/apache/solr/pull/4279#discussion_r3070136016


##########
solr/core/src/test/org/apache/solr/handler/admin/UpgradeCoreIndexActionTest.java:
##########
@@ -365,10 +365,192 @@ public void 
testUpgradeCoreIndexFailsWithNestedDocuments() throws Exception {
                           coreName),
                       resp));
 
-      // Verify the exception message indicates nested documents are not 
supported
+      // Verify the exception message indicates child documents are not 
supported
       assertThat(
           thrown.getMessage(),
-          containsString("does not support indexes containing nested 
documents"));
+          containsString("does not support indexes containing child 
documents"));
+    } finally {
+      admin.shutdown();
+      admin.close();
+    }
+  }
+
+  // --- Child docs detection tests ---
+  //
+  // These tests verify that the child document detection in the upgrade path
+  // correctly distinguishes between genuine child docs and non-child docs,
+  // even in the presence of updates and deletes that leave deleted documents
+  // in segments (since NoMergePolicy prevents segment merges from purging 
them).
+
+  @Test
+  public void testChildDocsDetection_noChildDocsJustAdd() throws Exception {

Review Comment:
   I understand the intent that we are trying to be explicit about the edge 
cases of the child docs check. In certain cases however, I'd argue that certain 
tests like this one are effectively testing the happy path which is already 
getting covered inherently in existing tests like 
testNoUpgradeNeededWhenAllSegmentsCurrent(). 
   
   I appreciate the thorough tests, but I think we have an opportunity to keep 
the coverage tight here by relying on certain existing tests like above. 
Another reasoning is that every test is "just another test" by itself, but can 
gradually add to the build times.



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


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to