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

Fixed flakiness caused by HashSet ordering in PageImplTest #2894

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

mumbler6
Copy link

@mumbler6 mumbler6 commented Nov 11, 2024

Q                       A
Fixed Issues? Fixes #2893
Patch: Bug Fix? 👍
Minor: New Feature? No
Major: Breaking Change? No
Tests Added + Pass? Yes
Documentation Provided No
Any Dependency Changes? No
License Apache License, Version 2.0

Simple bug fix to remove indeterministic test behavior from a few tests. More about it in the linked issue.

@ky940819
Copy link
Contributor

I think that the best solution to this problem might be to update the AEM Mocks project to eliminate the source of the problem:
https://github.com/wcm-io/io.wcm.testing.aem-mock/blob/d9734113b672aa734f43ff2da83db4e14be0d95d/core/src/main/java/io/wcm/testing/mock/aem/MockTagManager.java#L380

Changing the order of the of Page#getKeywords() to enforce an alphabetic return order might not be desirable in all cases, and could be a breaking change to some users who expect the tags to be returned in the encounter order.

Additionally, the comparator isn't locale-specific.

Kyle

Copy link

sonarcloud bot commented Nov 14, 2024

@mumbler6
Copy link
Author

I understand. I'll try to make a PR in the AEM Mocks project where that HashSet is changed to a LinkedHashSet.

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

Successfully merging this pull request may close these issues.

[PageImpl] Non-deterministic behavior or HashSet might fail tests in PageImplTest
2 participants