fixed 3 flaky tests in StringJoinTest.java #319
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
t92549
reviewed
Dec 1, 2023
| @@ -48,7 +49,10 @@ public void shouldHandleNullInput() { | |||
| public void shouldHandleNullDelimiter() { | |||
| // Given | |||
| final StringJoin<String> function = new StringJoin<>(null); | |||
| final Set<String> input = Sets.newHashSet("a", "b", "c"); | |||
| final Set<String> input = new LinkedHashSet<>(); | |||
There was a problem hiding this comment.
Maybe an ArrayList could be used instead to keep this all on one line?
There was a problem hiding this comment.
Makes sense! Used an ArrayList to clean up and use a single line.
There was a problem hiding this comment.
You have made the input a Set with an ArrayList inside, which will work. However, for simplicity, I was thinking just make the input a List and don't use a Set at all.
There was a problem hiding this comment.
Oh. right. I assumed you wanted a set. but using a List makes sense as I believe it is deterministic and should solve the flakiness in the code!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Flaky Test Fixes
Overview
This document outlines the issues identified in flaky tests and the corresponding fixes applied to address them. The changes were made in commit 36c63c3.
Tests 1, 2, 3: StringJoinTest.shouldJoinIterableOfStrings, StringJoinTest.shouldHandleNullDelimiter, StringJoinTest.shouldJoinIterableOfStringsWithDelimiter
Issue
Use of HashSet caused the assertions to fail as it does not maintain initial order of the elements.
Fix
Replaced
newHashSetwith aLinkedHashSetas it maintains initial order of the list and improves readabilityReproduction
To reproduce the error, use the following command for each test:
Testing the Fixes
All fixes were tested by running the affected tests with the proposed changes to ensure reliability.
Conclusion
These changes aim to address flakiness in the identified tests and improve the overall stability of the test suite.
Additional Information
The fixes were identified and tested using the nondex tool.
Please do let me know if have any queries or these fixes look good to you!