epugh commented on code in PR #2631:
URL: https://github.com/apache/solr/pull/2631#discussion_r1711353839
##########
solr/core/src/test/org/apache/solr/update/processor/SkipExistingDocumentsProcessorFactoryTest.java:
##########
@@ -314,6 +314,22 @@ public void
testSkippableUpdateIsSkippedIfSkipUpdatesTrue() throws IOException {
verify(next, never()).processAdd(cmd);
}
+ @Test
Review Comment:
Thanks for adding tests. This is more of a personal comment, but I find
mocks to be harder to understand than more traditional tests (that in solr
world do end up being integration sytle). Mostly because I sometimes stugglre
to understand the data set up.... i know this class has other mocks as well...
##########
solr/core/src/test/org/apache/solr/update/processor/SkipExistingDocumentsProcessorFactoryTest.java:
##########
@@ -328,6 +344,22 @@ public void
testNonSkippableUpdateIsNotSkippedIfSkipUpdatesTrue() throws IOExcep
verify(next).processAdd(cmd);
}
+ @Test
+ public void testNonSkippableChildDocUpdateIsNotSkippedIfSkipUpdatesTrue()
throws IOException {
Review Comment:
Maybe `testNonSkippableChildDocUpdateIsNotSkippedIfSkipUpdatesIsTrue` ???
##########
solr/core/src/java/org/apache/solr/update/processor/SkipExistingDocumentsProcessorFactory.java:
##########
@@ -224,6 +224,17 @@ boolean doesDocumentExist(BytesRef indexedDocId) throws
IOException {
}
}
+ boolean doesChildDocumentExist(AddUpdateCommand cmd) throws IOException {
Review Comment:
I'd love to see some javadocs added to these methods. I know they didn't
have it before....
--
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]
