You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by "Mikep86 (via GitHub)" <gi...@apache.org> on 2024/09/13 15:51:13 UTC
[PR] Fix Flaky Test In TestBlockJoinBulkScorer [lucene]
Mikep86 opened a new pull request, #13785:
URL: https://github.com/apache/lucene/pull/13785
Fix the `testSetMinCompetitiveScoreWithScoreModeMax` test, which sometimes failed due to randomizations in how the docs were scored
--
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-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Fix Flaky Test In TestBlockJoinBulkScorer [lucene]
Posted by "jpountz (via GitHub)" <gi...@apache.org>.
jpountz commented on code in PR #13785:
URL: https://github.com/apache/lucene/pull/13785#discussion_r1761068214
##########
lucene/join/src/test/org/apache/lucene/search/join/TestBlockJoinBulkScorer.java:
##########
@@ -283,7 +292,33 @@ public void collect(int doc) throws IOException {
null,
0,
NO_MORE_DOCS);
- assertEquals(expectedScores, actualScores);
+
+ if (expectedScoresList.size() == 1) {
+ assertEquals(expectedScoresList.getFirst(), actualScores);
+ } else {
+ assertEqualsToOneOf(expectedScoresList, actualScores);
+ }
Review Comment:
OK
--
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-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Fix Flaky Test In TestBlockJoinBulkScorer [lucene]
Posted by "jpountz (via GitHub)" <gi...@apache.org>.
jpountz merged PR #13785:
URL: https://github.com/apache/lucene/pull/13785
--
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-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Fix Flaky Test In TestBlockJoinBulkScorer [lucene]
Posted by "jpountz (via GitHub)" <gi...@apache.org>.
jpountz commented on code in PR #13785:
URL: https://github.com/apache/lucene/pull/13785#discussion_r1760649790
##########
lucene/join/src/test/org/apache/lucene/search/join/TestBlockJoinBulkScorer.java:
##########
@@ -283,7 +292,33 @@ public void collect(int doc) throws IOException {
null,
0,
NO_MORE_DOCS);
- assertEquals(expectedScores, actualScores);
+
+ if (expectedScoresList.size() == 1) {
+ assertEquals(expectedScoresList.getFirst(), actualScores);
+ } else {
+ assertEqualsToOneOf(expectedScoresList, actualScores);
+ }
+ }
+
+ private static void assertEqualsToOneOf(List<?> expectedList, Object actual) {
+ boolean foundMatch = false;
+ for (Object expected : expectedList) {
+ if (expected == null) {
+ if (actual == null) {
+ foundMatch = true;
+ break;
+ }
+ } else {
+ foundMatch = expected.equals(actual);
+ if (foundMatch) {
+ break;
+ }
+ }
Review Comment:
nit: maybe merge these two conditions under a single `if (Objects.equals(expected, actual))` which would take care of the `null` case?
##########
lucene/join/src/test/org/apache/lucene/search/join/TestBlockJoinBulkScorer.java:
##########
@@ -283,7 +292,33 @@ public void collect(int doc) throws IOException {
null,
0,
NO_MORE_DOCS);
- assertEquals(expectedScores, actualScores);
+
+ if (expectedScoresList.size() == 1) {
+ assertEquals(expectedScoresList.getFirst(), actualScores);
+ } else {
+ assertEqualsToOneOf(expectedScoresList, actualScores);
+ }
Review Comment:
I'm curious why you don't call `assertEqualsToOneOf` all the time, shouldn't it also work in the size==1 case?
--
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-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Fix Flaky Test In TestBlockJoinBulkScorer [lucene]
Posted by "javanna (via GitHub)" <gi...@apache.org>.
javanna commented on PR #13785:
URL: https://github.com/apache/lucene/pull/13785#issuecomment-2353671468
Thanks for fixing this!
--
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-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Fix Flaky Test In TestBlockJoinBulkScorer [lucene]
Posted by "Mikep86 (via GitHub)" <gi...@apache.org>.
Mikep86 commented on code in PR #13785:
URL: https://github.com/apache/lucene/pull/13785#discussion_r1761059326
##########
lucene/join/src/test/org/apache/lucene/search/join/TestBlockJoinBulkScorer.java:
##########
@@ -283,7 +292,33 @@ public void collect(int doc) throws IOException {
null,
0,
NO_MORE_DOCS);
- assertEquals(expectedScores, actualScores);
+
+ if (expectedScoresList.size() == 1) {
+ assertEquals(expectedScoresList.getFirst(), actualScores);
+ } else {
+ assertEqualsToOneOf(expectedScoresList, actualScores);
+ }
Review Comment:
It should and I originally did that, but if you use `assertEquals` there are nicer IDE integrations for value comparison on failure. Given that this method is called with a single map of expected scores in nearly all cases, I tried to preserve that functionality. This could help in the case of a failure in `testScoreRandomIndices`, which could have a large random score map.
--
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-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org