Ensure assertions is contents aware

Closes #3227

Author: krmahadevan

CHANGES.txt

@@ -1,4 +1,5 @@
 Current (7.12.0)
+Fixed: GITHUB-3227: assertEqualsNoOrder false positive when collection has same size and actual Collection is subset of expected collection (Krishnan Mahadevan)
 Fixed: GITHUB-3177: Method org.testng.xml.XmlSuite#toXml do not save new properties like "share-thread-pool-for-data-providers" (Krishnan Mahadevan)
 Fixed: GITHUB-3179: ClassCastException when use shouldUseGlobalThreadPool(true) property (Krishnan Mahadevan)
 Fixed: GITHUB-2765: Test timeouts using existing Executor now propagate the stack trace to the ThreadTimeoutException (Charlie Hayes)

testng-asserts/src/main/java/org/testng/Assert.java

@@ -1856,18 +1856,7 @@ public static void assertEqualsNoOrder(Object[] actual, Object[] expected, Strin
    */
   public static void assertEqualsNoOrder(
       Collection<?> actual, Collection<?> expected, String message) {
-    if (actual.size() != expected.size()) {
-      failAssertNoEqual(
-          "Collections do not have the same size: " + actual.size() + " != " + expected.size(),
-          message);
-    }
-
-    List<?> actualCollection = Lists.newArrayList(actual);
-    actualCollection.removeAll(expected);
-    if (!actualCollection.isEmpty()) {
-      failAssertNoEqual(
-          "Collections not equal: expected: " + expected + " and actual: " + actual, message);
-    }
+    validateEqualityNoOrder(actual, expected, "Collections", message);
   }
 
   /**
@@ -1879,26 +1868,25 @@ public static void assertEqualsNoOrder(
    * @param message the assertion error message
    */
   public static void assertEqualsNoOrder(Iterator<?> actual, Iterator<?> expected, String message) {
-    List<?> actualCollection = Lists.newArrayList(actual);
-    List<?> expectedCollection = Lists.newArrayList(expected);
+    validateEqualityNoOrder(
+        Lists.newArrayList(actual), Lists.newArrayList(expected), "Iterators", message);
+  }
 
-    if (actualCollection.size() != expectedCollection.size()) {
+  private static void validateEqualityNoOrder(
+      Collection<?> actual, Collection<?> expected, String prefix, String message) {
+    if (actual.size() != expected.size()) {
       failAssertNoEqual(
-          "Iterators do not have the same size: "
-              + actualCollection.size()
-              + " != "
-              + expectedCollection.size(),
+          prefix + " do not have the same size: " + actual.size() + " != " + expected.size(),
           message);
     }
+    List<?> actualCollection = Lists.newArrayList(actual);
+    for (Object o : expected) {
+      actualCollection.remove(o);
+    }
 
-    actualCollection.removeAll(expectedCollection);
     if (!actualCollection.isEmpty()) {
       failAssertNoEqual(
-          "Iterators not equal: expected: "
-              + toString(expected)
-              + " and actual: "
-              + toString(actual),
-          message);
+          prefix + " not equal: expected: " + expected + " and actual: " + actual, message);
     }
   }
 

testng-asserts/src/test/java/org/testng/AssertTest.java

@@ -2,6 +2,7 @@
 
 import com.google.common.collect.ImmutableMap;
 import java.util.*;
+import org.testng.annotations.DataProvider;
 import org.testng.annotations.Test;
 import org.testng.collections.Lists;
 import org.testng.collections.Maps;
@@ -602,6 +603,46 @@ public void testAssertEqualsDeepSetFail() {
     Assert.assertEqualsDeep(actualSet, expectedSet);
   }
 
+  @Test(
+      description = "GITHUB-3227",
+      dataProvider = "3227-dp",
+      expectedExceptions = AssertionError.class,
+      expectedExceptionsMessageRegExp = "list should not be equal")
+  public void testAssertEqualsNoOrderList(List<String> actual, List<String> expected) {
+    Assert.assertEqualsNoOrder(actual, expected, "list should not be equal");
+  }
+
+  @DataProvider(name = "3227-dp")
+  public Object[][] dataProvider() {
+    return new Object[][] {
+      {List.of("a", "b", "b"), List.of("a", "b", "c")},
+      {List.of("a", "b", "b"), List.of("a", "a", "b")},
+      {List.of("a", "a", "a", "a", "a", "a", "b"), List.of("a", "b", "b", "b", "b", "b", "b")}
+    };
+  }
+
+  @Test(
+      description = "GITHUB-3227",
+      dataProvider = "3227-dp",
+      expectedExceptions = AssertionError.class,
+      expectedExceptionsMessageRegExp = "queue should not be equal")
+  public void testAssertEqualsNoOrderQueue(List<String> actualIn, List<String> expectedIn) {
+    var actual = new ArrayDeque<>(actualIn);
+    var expected = new ArrayDeque<>(expectedIn);
+    Assert.assertEqualsNoOrder(actual, expected, "queue should not be equal");
+  }
+
+  @Test(
+      description = "GITHUB-3227",
+      dataProvider = "3227-dp",
+      expectedExceptions = AssertionError.class,
+      expectedExceptionsMessageRegExp = "iterator should not be equal")
+  public void testAssertEqualsNoOrderIterator(List<String> actualIn, List<String> expectedIn) {
+    var actual = actualIn.iterator();
+    var expected = expectedIn.iterator();
+    Assert.assertEqualsNoOrder(actual, expected, "iterator should not be equal");
+  }
+
   static class Contrived {
 
     int integer;