search

apache / lucene

Apache Lucene open-source search software
https://lucene.apache.org/
Apache License 2.0
2.69k stars 1.04k forks source link

Improve TestTaxonomyFacetAssociations#validateFloats to not rely on summation ordering #13738

Open gsmiller opened 2 months ago

gsmiller commented 2 months ago

Description

After merging #13726 we saw test failures because TestTaxonomyFacetAssociations#validateFloats is written to (intentionally) sum floats in a consistent order and then use exact equality, but the test update brought in search concurrency which breaks that consistency. This got fixed with 0ec453d485df5411d21edd24b47f4880befd30d0, which just disables the concurrency in these tests for now, but maybe we should make this test less fragile and bring the concurrency back? Could be a nice opportunity to leverage #13723 when it gets merged.

javanna commented 2 months ago

Sounds good to me, I disabled concurrency in that test because I could not come up with a quick way to keep it enabled for this test. It looked to me as if it requires sequential execution, but I am not an expert of facets.

mikemccand commented 2 months ago

We could also accept that operations are sometimes out of order and use the new ULP-based epsilon float equality assertion method to check.

gsmiller commented 2 months ago

@javanna yeah, +1 to the quick fix you put in place (after all, the test itself documents that it expects sequential, consistent ordering in summation). @mikemccand, agreed.

I don't see any urgency to picking this up but wanted to capture the idea. Could be a good starter task for someone potentially.