vigyasharma commented on code in PR #633:
URL: https://github.com/apache/lucene/pull/633#discussion_r862515687
##########
lucene/test-framework/src/java/org/apache/lucene/tests/index/MockRandomMergePolicy.java:
##########
@@ -86,6 +86,20 @@ public MergeSpecification findMerges(
return mergeSpec;
}
+ @Override
+ public MergeSpecification findMerges(CodecReader... readers) throws
IOException {
+ if (random.nextBoolean() == true) {
+ return super.findMerges(readers);
+ } else {
+ // create a oneMerge for each reader to let them get concurrently
processed by addIndexes()
Review Comment:
Thanks for reviewing this @rmuir, @uschindler. I understand your concern.
Failures on concurrent code are already tough to reproduce; it gets much harder
to do this from a randomized test seed.
It is worth noting that the `findMerges(CodecReader[])` API modified in
`MockRandomMergePolicy`, is only invoked within the
`addIndexes(CodecReaders[])` API, which should contain its scope of impact. But
I definitely don't want to add noise with random failures on distantly related
tests. I'll remove this new policy from `MockRandomMergePolicy`, which is
frequently used for `IndexWriterConfig` in multiple tests.
For my awareness, what is the best practice in Lucene for testing
non-default, concurrency related code paths? Randomized tests help validate
infrequently triggered code, but concurrency poses has reproducibility
challenges. For such cases, do we try to rely entirely on change-specific
concurrency unit tests?
--
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: issues-unsubscr...@lucene.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
