Merge pull request #870 from raindev/arrays-inside-collections

Compare arrays by value only when passed to assert methods directly

5 files changed, 93 insertions, 109 deletions

diff --git a/src/main/java/org/testng/Assert.java b/src/main/java/org/testng/Assert.java
index 5e6a99af..6ffcbf12 100644
--- a/src/main/java/org/testng/Assert.java
+++ b/src/main/java/org/testng/Assert.java
@@ -109,30 +109,42 @@ public class Assert {
    * @param message the assertion error message
    */
   static public void assertEquals(Object actual, Object expected, String message) {
-    if((expected == null) && (actual == null)) {
-      return;
-    }
-    if(expected == null ^ actual == null) {
-      failNotEquals(actual, expected, message);
-    }
-    if (expected.getClass().isArray()) {
+    if (expected != null && expected.getClass().isArray()) {
        assertArrayEquals(actual, expected, message);
        return;
     }
-    if (expected.equals(actual) && actual.equals(expected)) {
-       return;
-    }
-    failNotEquals(actual, expected, message);
+    assertEqualsImpl(actual, expected, message);
   }
 
   /**
-   * Asserts that two objects are equal. It they are not, an AssertionError,
-   * with given message, is thrown.
-   * @param actual the actual value
-   * @param expected the expected value (should be an non-null array value)
-   * @param message the assertion error message
+   * Differs from {@link #assertEquals(Object, Object, String)} by not taking arrays into
+   * special consideration hence comparing them by reference. Intended to be called directly
+   * to test equality of collections content.
    */
+  private static void assertEqualsImpl(Object actual, Object expected,
+          String message) {
+      if((expected == null) && (actual == null)) {
+        return;
+      }
+      if(expected == null ^ actual == null) {
+        failNotEquals(actual, expected, message);
+      }
+      if (expected.equals(actual) && actual.equals(expected)) {
+        return;
+      }
+      failNotEquals(actual, expected, message);
+    }
+
   private static void assertArrayEquals(Object actual, Object expected, String message) {
+    if (expected == actual) {
+      return;
+    }
+    if (null == expected) {
+      fail("expected a null array, but not null found. " + message);
+    }
+    if (null == actual) {
+      fail("expected not null array, but null found. " + message);
+    }
     //is called only when expected is an array
     if (actual.getClass().isArray()) {
       int expectedLength = Array.getLength(expected);
@@ -157,7 +169,7 @@ public class Assert {
     failNotEquals(actual, expected, message);
   }
 
-/**
+  /**
    * Asserts that two objects are equal. If they are not,
    * an AssertionError is thrown.
    * @param actual the actual value
@@ -548,7 +560,7 @@ public class Assert {
       String explanation = "Lists differ at element [" + i + "]: " + e + " != " + a;
       String errorMessage = message == null ? explanation : message + ": " + explanation;
 
-      assertEquals(a, e, errorMessage);
+      assertEqualsImpl(a, e, errorMessage);
     }
   }
   
@@ -591,7 +603,7 @@ public class Assert {
       String explanation = "Iterators differ at element [" + i + "]: " + e + " != " + a;
       String errorMessage = message == null ? explanation : message + ": " + explanation;
       
-      assertEquals(a, e, errorMessage);
+      assertEqualsImpl(a, e, errorMessage);
       
     }
     
@@ -738,48 +750,6 @@ public class Assert {
   }
 
   /**
-   * Asserts that two arrays contain the same elements in the same order. If they do not,
-   * an AssertionError is thrown.
-   *
-   * @param actual the actual value
-   * @param expected the expected value
-   */
-  static public void assertEquals(final byte[] actual, final byte[] expected) {
-    assertEquals(actual, expected, "");
-  }
-
-  /**
-   * Asserts that two arrays contain the same elements in the same order. If they do not,
-   * an AssertionError, with the given message, is thrown.
-   *
-   * @param actual the actual value
-   * @param expected the expected value
-   * @param message the assertion error message
-   */
-  static public void assertEquals(final byte[] actual, final byte[] expected, final String message) {
-    if(expected == actual) {
-      return;
-    }
-    if(null == expected) {
-      fail("expected a null array, but not null found. " + message);
-    }
-    if(null == actual) {
-      fail("expected not null array, but null found. " + message);
-    }
-
-    assertEquals(actual.length, expected.length, "arrays don't have the same size. " + message);
-
-    for(int i= 0; i < expected.length; i++) {
-      if(expected[i] != actual[i]) {
-        fail("arrays differ firstly at element [" + i +"]; "
-            + "expected value is <" + expected[i] +"> but was <"
-            + actual[i] + ">. "
-            + message);
-      }
-    }
-  }
-
-  /**
    * Asserts that two sets are equal.
    */
   static public void assertEquals(Set<?> actual, Set<?> expected) {
@@ -834,7 +804,7 @@ public class Assert {
       Object key = entry.getKey();
       Object value = entry.getValue();
       Object expectedValue = expected.get(key);
-      assertEquals(value, expectedValue, "Maps do not match for key:" + key + " actual:" + value
+      assertEqualsImpl(value, expectedValue, "Maps do not match for key:" + key + " actual:" + value
               + " expected:" + expectedValue);
     }
 
diff --git a/src/test/java/test/MapTest.java b/src/test/java/test/MapTest.java
deleted file mode 100644
index ae3aedc3..00000000
--- a/src/test/java/test/MapTest.java
+++ /dev/null
@@ -1,18 +0,0 @@
-package test;
-
-import org.testng.Assert;
-import org.testng.annotations.Test;
-
-import java.util.HashMap;
-import java.util.Map;
-
-public class MapTest {
-  @Test(description = "Make sure that arrays as map values work with assert")
-  public void testMap() {
-    Map<String, boolean[]> mapA = new HashMap<>();
-    Map<String, boolean[]> mapB = new HashMap<>();
-    mapA.put("A", new boolean[] { true });
-    mapB.put("A", new boolean[] { true });
-    Assert.assertEquals(mapA, mapB);
-  }
-}
\ No newline at end of file
diff --git a/src/test/java/test/asserttests/ArrayEqualityAssertTest.java b/src/test/java/test/asserttests/ArrayEqualityAssertTest.java
index d9e5c526..b98cff35 100644
--- a/src/test/java/test/asserttests/ArrayEqualityAssertTest.java
+++ b/src/test/java/test/asserttests/ArrayEqualityAssertTest.java
@@ -19,7 +19,48 @@ import static org.testng.Assert.assertNotEquals;
  * and arrays.

  */

 public class ArrayEqualityAssertTest {

+

+    @Test

+    public void arrayAssertEquals() {

+        assertEquals(new int[]{ 42 }, new int[] { 42 },

+                "arrays of primitives are compared by value in assertEquals");

+    }

+

+    @Test(expectedExceptions = AssertionError.class)

+    public void arrayAssertNotEquals() {

+        assertNotEquals(new int[]{ 42 }, new int[] { 42 },

+                "arrays of primitives are compared by value in assertNotEquals");

+    }

+

     @Test

+    public void boxedArrayAssertEquals() {

+        assertEquals(new Integer[]{ 42 }, new Integer[] { 42 },

+                "arrays of wrapped values are compared by value in assertEquals");

+    }

+

+    @Test(expectedExceptions = AssertionError.class)

+    public void boxedArrayAssertNotEquals() {

+        assertNotEquals(new Integer[]{ 42 }, new Integer[] { 42 },

+                "arrays of wrapped values are compared by value in assertNotEquals");

+    }

+

+    @Test

+    public void mixedArraysAssertEquals() {

+        assertEquals(new int[]{ 42 }, new Integer[] { 42 },

+                "arrays of wrapped values are compared by value in assertEquals");

+        assertEquals(new Integer[]{ 42 }, new int[] { 42 },

+                "arrays of wrapped values are compared by value in assertEquals");

+    }

+

+    @Test(expectedExceptions = AssertionError.class)

+    public void mixedArraysAssertNotEquals() {

+        assertNotEquals(new int[]{ 42 }, new Integer[] { 42 },

+                "arrays of wrapped values are compared by value in assertNotEquals");

+        assertNotEquals(new Integer[]{ 42 }, new int[] { 42 },

+                "arrays of wrapped values are compared by value in assertNotEquals");

+    }

+

+    @Test(expectedExceptions = AssertionError.class)

     public void arrayInsideListAssertEquals() {

         List<int[]> list = Arrays.asList(

             new int[]{ 42 }

@@ -28,7 +69,7 @@ public class ArrayEqualityAssertTest {
             new int[]{ 42 }

         );

         assertEquals(list, listCopy,

-                "arrays inside lists are compared by value in assertEquals");

+                "arrays inside lists are compared by reference in assertEquals");

     }

 

     @Test

@@ -40,17 +81,17 @@ public class ArrayEqualityAssertTest {
             new int[]{ 42 }

         );

         assertNotEquals(list, listCopy,

-                "arrays inside lists aren't compared by value in assertNotEquals");

+                "arrays inside lists are compared by reference in assertNotEquals");

     }

 

-    @Test

+    @Test(expectedExceptions = AssertionError.class)

     public void arrayInsideMapAssertEquals() {

         Map<String, int[]> map = new HashMap<>();

         map.put("array", new int[]{ 42 });

         Map<String, int[]> mapCopy = new HashMap<>();

         mapCopy.put("array", new int[]{ 42 });

 

-        // arrays inside maps are compared by value in assertEquals(Map,Map)

+        // arrays inside maps are compared by reference in assertEquals(Map,Map)

         assertEquals(map, mapCopy);

     }

 

@@ -62,7 +103,7 @@ public class ArrayEqualityAssertTest {
         mapCopy.put("array", new int[]{ 42 });

 

         assertEquals(map, mapCopy,

-                "arrays inside maps aren't compared by value in assertEquals(Map,Map,String)");

+                "arrays inside maps are compared by reference in assertEquals(Map,Map,String)");

     }

 

     @Test

@@ -73,7 +114,7 @@ public class ArrayEqualityAssertTest {
         mapCopy.put("array", new int[]{ 42 });

 

         assertNotEquals(map, mapCopy,

-                "arrays inside maps aren't compared by value in assertNotEquals");

+                "arrays inside maps are compared by reference in assertNotEquals");

     }

 

     @Test(expectedExceptions = AssertionError.class)

@@ -84,7 +125,7 @@ public class ArrayEqualityAssertTest {
         setCopy.add(new int[]{ 42 });

 

         assertEquals(set, setCopy,

-                "arrays inside sets aren't compared by value in assertNotEquals");

+                "arrays inside sets are compared by reference in assertNotEquals");

     }

 

     @Test

@@ -95,7 +136,7 @@ public class ArrayEqualityAssertTest {
         setCopy.add(new int[]{ 42 });

 

         assertNotEquals(set, setCopy,

-                "arrays inside sets aren't compared by value in assertNotEquals");

+                "arrays inside sets are compared by reference in assertNotEquals");

     }

 

     @Test(expectedExceptions = AssertionError.class)

@@ -104,7 +145,7 @@ public class ArrayEqualityAssertTest {
         List<List<int[]>> listCopy = Collections.singletonList(Arrays.asList(new int[]{ 42 }));

 

         assertEquals(list, listCopy,

-                "arrays inside lists which are inside lists themselves aren't compared by value in assertEquals");

+                "arrays inside lists which are inside lists themselves are compared by reference in assertEquals");

     }

 

     @Test(expectedExceptions = AssertionError.class)

@@ -119,7 +160,7 @@ public class ArrayEqualityAssertTest {
         mapCopy.put("map", innerMapCopy);

 

         assertEquals(map, mapCopy,

-                "arrays inside maps which are inside maps themselves aren't compared by value in assertEquals");

+                "arrays inside maps which are inside maps themselves are compared by reference in assertEquals");

     }

 

     @Test(expectedExceptions = AssertionError.class)

@@ -134,7 +175,7 @@ public class ArrayEqualityAssertTest {
         list.add(innerMapCopy);

 

         assertEquals(list, listCopy,

-                "arrays inside maps which are inside lists themselves aren't compared by value in assertEquals");

+                "arrays inside maps which are inside lists themselves are compared by reference in assertEquals");

     }

 

     @Test(expectedExceptions = AssertionError.class)

@@ -145,10 +186,10 @@ public class ArrayEqualityAssertTest {
         mapCopy.put("list", Arrays.asList(new int[]{ 42 }));

 

         assertEquals(map, mapCopy,

-                "arrays inside maps which are inside lists themselves aren't compared by value in assertEquals");

+                "arrays inside maps which are inside lists themselves are compared by reference in assertEquals");

     }

 

-    @Test

+    @Test(expectedExceptions = AssertionError.class)

     public void arrayInsideIterableAssertEquals() {

         Iterable<int[]> iterable = Arrays.asList(

             new int[]{ 42 }

@@ -157,7 +198,7 @@ public class ArrayEqualityAssertTest {
             new int[]{ 42 }

         );

         assertEquals(iterable, iterableCopy,

-                "arrays inside Iterables are compared by value in assertEquals");

+                "arrays inside Iterables are compared by reference in assertEquals");

     }

 

     @Test(expectedExceptions = AssertionError.class)

@@ -166,10 +207,10 @@ public class ArrayEqualityAssertTest {
         List<List<int[]>> iterableCopy = Collections.singletonList(Arrays.asList(new int[]{ 42 }));

 

         assertEquals(iterable, iterableCopy,

-                "arrays inside Iterables which are inside Iterables themselves aren't compared by value in assertEquals");

+                "arrays inside Iterables which are inside Iterables themselves are compared by reference in assertEquals");

     }

 

-    @Test

+    @Test(expectedExceptions = AssertionError.class)

     public void arrayInsideArrayAssertEquals() {

         int[][] array = new int[][] {

             new int[]{ 42 }

@@ -178,10 +219,10 @@ public class ArrayEqualityAssertTest {
             new int[]{ 42 }

         };

         assertEquals(array, arrayCopy,

-                "arrays inside arrays are compared by value in assertEquals");

+                "arrays inside arrays are compared by reference in assertEquals");

     }

 

-    @Test

+    @Test(expectedExceptions = AssertionError.class)

     public void arrayDeepInArraysAssertEquals() {

         int[][][] array = new int[][][] {

             new int[][] { new int[]{ 42 } }

@@ -191,7 +232,7 @@ public class ArrayEqualityAssertTest {
         };

 

         assertEquals(array, arrayCopy,

-                "arrays inside arrays which are inside arrays themselves are compared by value in assertEquals");

+                "arrays inside arrays which are inside arrays themselves are compared by reference in assertEquals");

     }

 

     @SuppressWarnings("unchecked")

@@ -205,7 +246,7 @@ public class ArrayEqualityAssertTest {
         };

 

         assertEquals(array, arrayCopy,

-                "arrays inside arrays which are inside arrays themselves aren't compared by value in assertEquals");

+                "arrays inside arrays which are inside arrays themselves are compared by reference in assertEquals");

     }

 

 }

diff --git a/src/test/java/test/asserttests/AssertTest.java b/src/test/java/test/asserttests/AssertTest.java
index f935493a..7a4abfdc 100644
--- a/src/test/java/test/asserttests/AssertTest.java
+++ b/src/test/java/test/asserttests/AssertTest.java
@@ -36,14 +36,6 @@ public class AssertTest {
     Assert.assertEquals(intArr00, intArr01);
   }
 
-  @Test
-  public void int_Integer_Arrays() {
-    int[] intArr = {1, 2};
-    Integer[] integerArr = {1, 2};
-    Assert.assertEquals(intArr, integerArr);
-    Assert.assertEquals(integerArr, intArr);
-  }
-
   @Test(expectedExceptions = AssertionError.class)
   public void arraysFailures_1() {
     int[] intArr = {1, 2};
diff --git a/src/test/resources/testng.xml b/src/test/resources/testng.xml
index 41a2098f..351ee62b 100644
--- a/src/test/resources/testng.xml
+++ b/src/test/resources/testng.xml
@@ -96,7 +96,6 @@
       <class name="test.preserveorder.PreserveOrderTest" />
       <class name="test.SerializationTest" />
       <class name="test.CountTest" />
-      <class name="test.MapTest" />
       <class name="test.remote.RemoteTest" />
       <class name="test.EclipseTest" />
       <class name="test.ReporterApiTest" />