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

Fix flakiness due to indeterminate HashMap orderings in unit test #188

Merged
merged 2 commits into from
Mar 23, 2022

Conversation

kaiyaok2
Copy link
Contributor

@kaiyaok2 kaiyaok2 commented Mar 21, 2022

Description
Several (11 in total) flaky tests are found using Nondex when running commands
mvn edu.illinois:nondex-maven-plugin:1.1.2:nondex under scimono-client directory. All test flakinesses are due to the function ResourcePageQuery.apply(), which uses the default iterator of a HashMap.entrySet() when iterating through entries of the HashMap queryParams. However, an entry set for a normal HashMap is an unordered set, so it does not preserve element orders. As a result, the query parameters may not be retrieved in insertion order. This can lead to unexpected resultant URL, failure to get a PagedByIdentitySearchResult from a SCIMResponse, etc.
For example, consider testCreateCustomIdentityFilter() in ResourcePageQueryTest.java , the String resultRequestPath can be http://localhost:7070/idds/scim/v2/Users?startId=00000000-0000-1000-9000-000000000000&count=50 in one run and then http://localhost:7070/idds/scim/v2/Users?count=50&startId=00000000-0000-1000-9000-000000000000 in another.
For another example, testReadGroupsWithIndexPagedResponse() in GroupRequestTest.java may occasionally throw an javax.ws.rs.WebApplicationException after executing line 214 for the same reason.

Reasons for Fixing
These unit tests may fail nondeterministically when been run in a different OS or when using older / future versions of Java.

Fixes
Using LinkedHashMap instead of HashMap to ensure determinate iteration order.

@cla-assistant
Copy link

cla-assistant bot commented Mar 21, 2022

CLA assistant check
All committers have signed the CLA.

@karaulyanovsap karaulyanovsap merged commit d57a463 into SAP:master Mar 23, 2022
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.

None yet

2 participants