fix: fix flaky tests in org.apache.helix.rest.metadatastore.TestZkMetadataStoreDirectory#testComputePartitionAssignment

[hofi1]

Issues

• My PR addresses the following Helix issues and references them in the PR description:

Fixes apache#2645

Description

Sets return the elements in a non-deterministic order, which means that this assertion is not correct, because it checks whether the collections contain the same elements in the same order. This leads to a flack test. To fix this problem, the assertion has been rewritten to check if the collections contain the same amount of elements as well as booth collections contain all values of the other collection.

Tests that cause problems:

helix/helix-rest/src/test/java/org/apache/helix/rest/metadatastore/TestZkMetadataStoreDirectory.java

Lines 133 to 141 in 6241bcb

	public void testGetAllMetadataStoreRealms() {
	Set<String> realms = new HashSet<>();
	realms.add(TEST_REALM_1);
	realms.add(TEST_REALM_2);
	for (String namespace : _routingZkAddrMap.keySet()) {
	Assert.assertEquals(_metadataStoreDirectory.getAllMetadataStoreRealms(namespace), realms);
	}
	}

and

helix/helix-rest/src/test/java/org/apache/helix/rest/metadatastore/TestZkMetadataStoreDirectory.java

Lines 144 to 152 in 6241bcb

	public void testGetAllShardingKeys() {
	Set<String> allShardingKeys = new HashSet<>();
	allShardingKeys.addAll(TEST_SHARDING_KEYS_1);
	allShardingKeys.addAll(TEST_SHARDING_KEYS_2);
	for (String namespace : _routingZkAddrMap.keySet()) {
	Assert.assertEquals(_metadataStoreDirectory.getAllShardingKeys(namespace), allShardingKeys);
	}
	}

This problem was found by the NonDex Engine – to reproduce run

mvn -pl helix-rest edu.illinois:nondex-maven-plugin:2.1.1:nondex -Dtest=org.apache.helix.rest.metadatastore.TestZkMetadataStoreDirectory

Solution

Replace the assertions with a custom matcher, which checks if the sets contain the same elements without taking care of the order of the elements returned by the set.

Custom Matcher:

helix/helix-rest/src/test/java/org/apache/helix/rest/metadatastore/TestZkMetadataStoreDirectory.java

Lines 378 to 383 in 9cd4181

	private void assertCollectionsContainSameElementsIgnoringOrder(Collection<String> collection1,
	Collection<String> collection2) {
	Assert.assertEquals(collection2.size(), collection1.size());
	Assert.assertTrue(collection2.containsAll(collection1));
	Assert.assertTrue(collection1.containsAll(collection2));
	}

Tests

No test have been written – one existing test has been updated.

• The following is the result of the "mvn test" command on the appropriate module:

[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 02:51 min
[INFO] Finished at: 2023-10-04T23:16:04-05:00
[INFO] ------------------------------------------------------------------------

[CaseyPan]

The fix LGTM.
One suggestion is to provide an explanation of the original data structure that was leading to non-deterministic returns.

[KiruthikaJanakiraman]

The fix looks good! In the description, you could add a link to the NonDex tool. You could also add links pointing to the line of code that causes the flakiness.

[zzjas]

You can consider if it makes sense to combine some of your PRs for helix into a larger one.

You can proceed to open a real PR. Once you open a real PR, please mark this tentative PR as Opened in your tentative_pr.csv file and also raise a PR to IDoFT marking this as Opened. Thanks!