nick-boss-tech opened a new pull request, #5203: URL: https://github.com/apache/solr/pull/5203
🤖 *AI text below* 🤖 *(posted on behalf of Nick Shanin)* https://issues.apache.org/jira/browse/SOLR-17844 ## What happens today **TestFieldCacheSortRandom computes the expected sort order and then does not check it: two workaround commits removed the ordering assertions, so the test passes while verifying only the runtime types of its results.** The test computes expected values ([TestFieldCacheSortRandom.java at the base](https://github.com/nick-boss-tech/solr/blob/b73f03976fa17b857bc0a81bc8af2c6b1c31e0bb/solr/core/src/test/org/apache/solr/uninverting/TestFieldCacheSortRandom.java#L202-L231)), but the result check ([same file](https://github.com/nick-boss-tech/solr/blob/b73f03976fa17b857bc0a81bc8af2c6b1c31e0bb/solr/core/src/test/org/apache/solr/uninverting/TestFieldCacheSortRandom.java#L268-L289)) verifies each result's runtime type and unconditionally skips the exact ordering comparisons; the hit count is not asserted there either. The ticket says the earlier fix is "likely incorrect" and needs to be looked into, which is a question about whether the workaround commits are sound, not only whether the test exits green. ## What this change does **The test asserts the hit count, the document order, and the sort values again, deduplicating repeated observations of the same document.** [TestFieldCacheSortRandom.java](https://github.com/nick-boss-tech/solr/blob/b46e78cb50fea8cf8059bbbdb49815662a197a8e/solr/core/src/test/org/apache/solr/uninverting/TestFieldCacheSortRandom.java) is the only file changed. Lucene 10.4.0 partition dispatch can invoke the full-leaf supplier more than once, so the same global docID can be observed repeatedly; the restored assertions deduplicate the expected observations by docID and assert that repeated observations agree. The expected order is computed by an explicit comparator keyed on the test’s own missing-value setting, not by the sort under test. No expected value is taken from actual search results, and no production code changes. ## Proof **The restored assertions catch both a reversed sort and a flipped missing-value placement, and the class passes at the historical seed, verified 2026-10-22.** - Direction: with the actual SortField direction temporarily inverted and the expected direction unchanged, both tests fail with "Unexpected document at position" (seed 1BB02D3762B56611, the failure seed recorded in the message of workaround commit 3a58c85b9a12): 2 run, 2 failures, 0 skipped. - Missing-value placement: an independent review found that an earlier version of this repair took its expected order from the sort's own comparator, so flipping the missing-value placement passed undetected. With the explicit comparator restored, the same flip fails both tests. - With the comparator restored, the full class at the same seed: 2 run, 0 skipped, 0 failures, and 20 executions on random seeds pass. Tidy made no changes at this head. A confirmation run of the Error Prone compile and the module check at this head is in progress; this description will be updated when it lands. ## Limits **Test code only: production sorting is untouched, and an earlier version of this repair failed its own count assertion before the deduplication was added.** - The first version restored the assertions without deduplication and failed the count check (42 expected against 14 returned on the original run; a rerun at the same seed on another host gave 30 against 14, because the count depends on how the test searcher splits segments across threads), because repeated partition observations were counted as distinct hits. The deduplication is the correction for that; the failure is recorded, not hidden. - No changelog entry: test-only changes are exempt from the changelog check. *AI-generated description. Nick Shanin directed the work and takes responsibility for this contribution.* -- 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]
