Open
Conversation
Author
|
Hi, @barrybecker4 could you please review this Pull Request? I found the tests mentioned in this PR are flaky and I found a way to fix them. Please let me know if you need additional details. Thanks in advance! |
This file contains hidden or 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
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.
Description
Fixed the flaky test
testConstructionOfNodeWithNoChildreninside theUctNodeTestclass.Fixed the flaky test
testCrazyP1CombotestFinalPlayer1MoveInHomeBintestFinalPlayer1MoveOnPlayersSidetestFinalPlayer2MoveInHomeBintestFinalPlayer2MoveOnPlayersSidetestPlayer1CapturingMovetestPlayer2CapturingMoveinside theMoveMakerTestclass.Root Cause
UctNodeTest
The test
testConstructionOfNodeWithNoChildrenhas been reported as flaky when run with the NonDex tool. The test failed because it tries to compare two strings, but the methoductNode.getAttributes()returns a NodeAttribute Object.bb4-games/test/com/barrybecker4/game/twoplayer/common/search/strategy/UctNodeTest.java
Line 48 in 5dfcc6b
bb4-games/source/com/barrybecker4/game/twoplayer/common/search/strategy/UctNode.java
Lines 190 to 196 in 5dfcc6b
However, the
NodeAttributeclass extends from Java Hashmap.bb4-games/source/com/barrybecker4/game/twoplayer/common/search/tree/NodeAttributes.java
Line 21 in 5dfcc6b
Java Hashmap does not guarantee the order of elements in it. Therefore, it fails when applying
toString()and is compared with a hardcoded string.MoveMakerTest
All tests I mentioned at the beginning has been reported as flaky when run with the NonDex tool. These tests failed because they tried to compare two strings. They all have the same root cause, so I will use
testFinalPlayer1MoveInHomeBinfor instance.bb4-games/test/com/barrybecker4/game/twoplayer/mancala/move/MoveMakerTest.java
Lines 170 to 173 in 5dfcc6b
bb4-games/source/com/barrybecker4/game/twoplayer/mancala/move/MancalaMove.java
Lines 72 to 74 in 5dfcc6b
bb4-games/source/com/barrybecker4/game/twoplayer/mancala/move/MancalaMove.java
Line 33 in 5dfcc6b
Method
move.getCaptures()will return a Capture Object as inferred by the codes above. However, Capture Object is a Class that extends from Java HashMap.bb4-games/source/com/barrybecker4/game/twoplayer/mancala/move/Captures.java
Line 11 in 5dfcc6b
Java Hashmap does not guarantee the order of elements in it. Therefore, the test fails when applying
toString()to a Capture object and compares it with a hardcoded string.Fix
UctNodeTest
Test in this class is fixed by adding a temporary NodeAttribute
tempAttrand putting information in the hard-coded string into thistempAttr. Then comparetempAttrwithuctNode.getAttributes(). Since these two are all NodeAttribute types,assertEqualscan apply to them.MoveMakerTest
Tests in this class are fixed by adding a temporary Capture
tempCapand putting information in the hard-coded string into thistempCap. Then comparetempCapwithmove.getCaptures(). Since these two are all Capture types,assertEqualscan apply to them.How has this been tested?
Command used -
for UctNodetTest class
Command used -
for MoveMakerTest class
Note: replace Specific_Test_Name_Here with the test names mentioned in the beginning.
Command used -
for UctNodetTest class
Command used -
for UctNodetTest class
Note: replace Specific_Test_Name_Here with the test names mentioned in the beginning.
Replace X with an int number.
The NonDex test passed after the fix.