Fix flaky test get cluster assertion

[GrantPSpencer]

Issues

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

#2749
testGetClusters fails due to equals assertion. The sets are the same but fail due to a difference in ordering

2024-02-02T14:06:09.1230631Z [ERROR] Tests run: 224, Failures: 1, Errors: 0, Skipped: 37, Time elapsed: 199.518 s <<< FAILURE! - in TestSuite
2024-02-02T14:06:09.1234422Z [ERROR] testGetClusters(org.apache.helix.rest.server.TestClusterAccessor)  Time elapsed: 0.128 s  <<< FAILURE!
2024-02-02T14:06:09.1239433Z java.lang.AssertionError: clusters from response: [TaskTestCluster, TestCluster_0, TestCluster_1, superCluster, TestCluster_2, TestCluster_3, TestCluster_4, StoppableTestCluster, StoppableTestCluster2] vs clusters actually: [TestCluster_0, TaskTestCluster, TestCluster_1, superCluster, TestCluster_2, TestCluster_3, TestCluster_4, StoppableTestCluster, StoppableTestCluster2]: Lists differ at element [0]: TestCluster_0 != TaskTestCluster expected:<TestCluster_0> but was:<TaskTestCluster>
2024-02-02T14:06:09.1244544Z 	at org.apache.helix.rest.server.TestClusterAccessor.testGetClusters(TestClusterAccessor.java:102)

Description

• Here are some details about my PR, including screenshots of any UI changes:

The old method of assertEquals(set, set, msg) calls a testNG method that doesn't invoke the Set object's .equals() method. The old method asserts same order

public static void assertEquals(Collection actual, Collection expected, String message) {
    if (actual != expected) {
      if (actual == null || expected == null) {
        if (message != null) {
          fail(message);
        } else {
          fail("Collections not equal: expected: " + expected + " and actual: " + actual);
        }
      }

      assertEquals(actual.size(), expected.size(), message + ": lists don't have the same size");
      Iterator actIt = actual.iterator();
      Iterator expIt = expected.iterator();
      int i = -1;

      while(actIt.hasNext() && expIt.hasNext()) {
        ++i;
        Object e = expIt.next();
        Object a = actIt.next();
        String explanation = "Lists differ at element [" + i + "]: " + e + " != " + a;
        String errorMessage = message == null ? explanation : message + ": " + explanation;
        assertEquals(a, e, errorMessage);
      }

    }
  }

simply changing to assertEquals(set, set) calls a method that invokes the set.equals() method which does not care about ordering

  public static void assertEquals(Set actual, Set expected) {
    if (actual != expected) {
      if (actual == null || expected == null) {
        fail("Sets not equal: expected: " + expected + " and actual: " + actual);
      }

      if (!actual.equals(expected)) {
        fail("Sets differ: expected " + expected + " but got " + actual);
      }

    }
  }

Tests

• The following tests are written for this issue:

Running CI on personal branch:
https://github.com/GrantPSpencer/helix/actions/workflows/Helix-PR-CI.yml

Commits

• My commits all reference appropriate Apache Helix GitHub issues in their subject lines. In addition, my commits follow the guidelines from "How to write a good git commit message":
  1. Subject is separated from body by a blank line
  2. Subject is limited to 50 characters (not including Jira issue reference)
  3. Subject does not end with a period
  4. Subject uses the imperative mood ("add", not "adding")
  5. Body wraps at 72 characters
  6. Body explains "what" and "why", not "how"

Code Quality

• My diff has been formatted using helix-style.xml
  (helix-style-intellij.xml if IntelliJ IDE is used)

[zpinto]

Thanks for the fix and the analysis 🧐

[desaikomal]

very cool !!

[GrantPSpencer]

Pull request approved by @xyuanlu, @zpinto, @desaikomal
Commit message: Fix flaky test testGetClusters