[Rette66]

Description

Fixed the flaky test testConstructionOfNodeWithNoChildren inside the UctNodeTest class.
Fixed the flaky test testCrazyP1Combo testFinalPlayer1MoveInHomeBin testFinalPlayer1MoveOnPlayersSide testFinalPlayer2MoveInHomeBin testFinalPlayer2MoveOnPlayersSide testPlayer1CapturingMove testPlayer2CapturingMove inside the MoveMakerTest class.

Root Cause

UctNodeTest
The test testConstructionOfNodeWithNoChildren has been reported as flaky when run with the NonDex tool. The test failed because it tries to compare two strings, but the method uctNode.getAttributes() returns a NodeAttribute Object.

bb4-games/test/com/barrybecker4/game/twoplayer/common/search/strategy/UctNodeTest.java

Line 48 in 5dfcc6b

assertEquals("Unexpected attrs", "{wins=0.0, visits=0, winRate=0.50122}", uctNode.getAttributes().toString());

bb4-games/source/com/barrybecker4/game/twoplayer/common/search/strategy/UctNode.java

Lines 190 to 196 in 5dfcc6b

	public NodeAttributes getAttributes() {
	NodeAttributes attributes = new NodeAttributes();
	attributes.put("visits", Integer.toString(numVisits));
	attributes.put("wins", Float.toString(numWins));
	attributes.put("winRate", FormatUtil.formatNumber(getWinRate()));
	return attributes;
	}

However, the NodeAttribute class extends from Java Hashmap.

bb4-games/source/com/barrybecker4/game/twoplayer/common/search/tree/NodeAttributes.java

Line 21 in 5dfcc6b

public class NodeAttributes extends HashMap<String, String> {

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 testFinalPlayer1MoveInHomeBin for instance.

bb4-games/test/com/barrybecker4/game/twoplayer/mancala/move/MoveMakerTest.java

Lines 170 to 173 in 5dfcc6b

	assertEquals("Unexpected captures.",
	"{(row=2, column=2)=3, (row=2, column=3)=3, (row=2, column=4)=3, (row=2, column=5)=3, " +
	"(row=2, column=6)=3, (row=2, column=7)=3}",
	move.getCaptures().toString());

bb4-games/source/com/barrybecker4/game/twoplayer/mancala/move/MancalaMove.java

Lines 72 to 74 in 5dfcc6b

	public Captures getCaptures() {
	return captures;
	}

bb4-games/source/com/barrybecker4/game/twoplayer/mancala/move/MancalaMove.java

Line 33 in 5dfcc6b

private Captures captures = new Captures();

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

public class Captures extends HashMap<Location, Byte> {

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 tempAttr and putting information in the hard-coded string into this tempAttr. Then compare tempAttr with uctNode.getAttributes(). Since these two are all NodeAttribute types, assertEquals can apply to them.

MoveMakerTest
Tests in this class are fixed by adding a temporary Capture tempCap and putting information in the hard-coded string into this tempCap. Then compare tempCap with move.getCaptures(). Since these two are all Capture types, assertEquals can apply to them.

How has this been tested?

1. Regular test - Successful
   Command used -

./gradlew --info test --tests com.barrybecker4.game.twoplayer.common.search.strategy.UctNodeTest.testConstructionOfNodeWithNoChildren

for UctNodetTest class

Command used -

./gradlew --info test --tests com.barrybecker4.game.twoplayer.mancala.move.MoveMakerTest.Specific_Test_Name_Here

for MoveMakerTest class
Note: replace Specific_Test_Name_Here with the test names mentioned in the beginning.

2. NonDex test - Failed
   Command used -

./gradlew --info nondexTest --tests=com.barrybecker4.game.twoplayer.common.search.strategy.UctNodeTest.testConstructionOfNodeWithNoChildren --nondexRuns=X

for UctNodetTest class

Command used -

./gradlew --info nondexTest --tests=com.barrybecker4.game.twoplayer.mancala.move.MoveMakerTest.Specific_Test_Name_Here --nondexRuns=X

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.

[Rette66]

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!

[barrybecker4]

Merged current master and reapplied the fix on the Scala tests. The Java files this branch edited were removed in the Java-to-Scala port, which was the conflict.

Captures is still a HashMap, so comparing toString() to a fixed string is order-dependent. The tests now compare map contents. UctNodeTest uses the same comparison for node attributes.

UctNodeTest and MoveMakerTest pass locally, and CI on this pull request is green.

[barrybecker4]

@Rette66 - Thank you very much for your contribution. I'm sorry that it took me 3 years to notice it! I recently ported this repo from java to scala and have been making some other fixers.