Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
gsmiller opened this issue Sep 6, 2024 · 3 comments

Comments

@gsmiller
Copy link
Contributor

gsmiller commented Sep 6, 2024

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 0ec453d, 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
Copy link
Contributor

javanna commented Sep 9, 2024

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
Copy link
Member

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
Copy link
Contributor Author

gsmiller commented Sep 9, 2024

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants