From b152bae65c67c4654efa79b1da5de4e29068e12d Mon Sep 17 00:00:00 2001 From: Tom Anderson Date: Sun, 15 May 2016 23:35:58 +0100 Subject: [PATCH 1/3] Issue #1127; when arrays differ in length, say so, but go ahead and find the first difference as usual to ease diagnosis --- .../junit/internal/ComparisonCriteria.java | 56 ++++++++++++------- .../junit/tests/assertion/AssertionTest.java | 43 ++++++++++++-- 2 files changed, 75 insertions(+), 24 deletions(-) diff --git a/src/main/java/org/junit/internal/ComparisonCriteria.java b/src/main/java/org/junit/internal/ComparisonCriteria.java index 3a0ca1f2e62b..8cbcfc9c7d48 100644 --- a/src/main/java/org/junit/internal/ComparisonCriteria.java +++ b/src/main/java/org/junit/internal/ComparisonCriteria.java @@ -41,9 +41,23 @@ private void arrayEquals(String message, Object expecteds, Object actuals, boole // Only include the user-provided message in the outer exception. String exceptionMessage = outer ? header : ""; - int expectedsLength = assertArraysAreSameLength(expecteds, actuals, exceptionMessage); - for (int i = 0; i < expectedsLength; i++) { + if (expecteds == null) { + Assert.fail(exceptionMessage + "expected array was null"); + } + if (actuals == null) { + Assert.fail(exceptionMessage + "actual array was null"); + } + + int actualsLength = Array.getLength(actuals); + int expectedsLength = Array.getLength(expecteds); + if (actualsLength != expectedsLength) { + header = header + "array lengths differed, expected.length=" + + expectedsLength + " actual.length=" + actualsLength + "; "; + } + int prefixLength = Math.min(actualsLength, expectedsLength); + + for (int i = 0; i < prefixLength; i++) { Object expected = Array.get(expecteds, i); Object actual = Array.get(actuals, i); @@ -65,27 +79,31 @@ private void arrayEquals(String message, Object expecteds, Object actuals, boole } } } - } - private boolean isArray(Object expected) { - return expected != null && expected.getClass().isArray(); + if (actualsLength != expectedsLength) { + Object expected = getArrayElementOrSentinel(expecteds, expectedsLength, prefixLength); + Object actual = getArrayElementOrSentinel(actuals, actualsLength, prefixLength); + try { + Assert.assertEquals(expected, actual); + } catch (AssertionError e) { + throw new ArrayComparisonFailure(header, e, prefixLength); + } + } } - private int assertArraysAreSameLength(Object expecteds, - Object actuals, String header) { - if (expecteds == null) { - Assert.fail(header + "expected array was null"); - } - if (actuals == null) { - Assert.fail(header + "actual array was null"); - } - int actualsLength = Array.getLength(actuals); - int expectedsLength = Array.getLength(expecteds); - if (actualsLength != expectedsLength) { - Assert.fail(header + "array lengths differed, expected.length=" - + expectedsLength + " actual.length=" + actualsLength); + private static final Object END_OF_ARRAY_SENTINEL = new Object() { + @Override + public String toString() { + return "end of array"; } - return expectedsLength; + }; + + private Object getArrayElementOrSentinel(Object array, int length, int index) { + return index < length ? Array.get(array, index) : END_OF_ARRAY_SENTINEL; + } + + private boolean isArray(Object expected) { + return expected != null && expected.getClass().isArray(); } protected abstract void assertElementsEqual(Object expected, Object actual); diff --git a/src/test/java/org/junit/tests/assertion/AssertionTest.java b/src/test/java/org/junit/tests/assertion/AssertionTest.java index 760ad7ec8a55..d89997851db9 100755 --- a/src/test/java/org/junit/tests/assertion/AssertionTest.java +++ b/src/test/java/org/junit/tests/assertion/AssertionTest.java @@ -67,6 +67,21 @@ public void arraysNotEqualWithMessage() { assertArrayEquals("not equal", (new Object[]{new Object()}), (new Object[]{new Object()})); } + @Test(expected = AssertionError.class) + public void arraysDifferentLengthDifferingAtStartNotEqual() { + assertArrayEquals("not equal", (new Object[]{true}), (new Object[]{false, true})); + } + + @Test(expected = AssertionError.class) + public void arraysDifferentLengthDifferingAtEndNotEqual() { + assertArrayEquals("not equal", (new Object[]{true}), (new Object[]{true, false})); + } + + @Test(expected = AssertionError.class) + public void arraysDifferentLengthDifferingAtEndAndExpectedArrayLongerNotEqual() { + assertArrayEquals("not equal", (new Object[]{true, false}), (new Object[]{true})); + } + @Test public void arraysExpectedNullMessage() { try { @@ -86,11 +101,29 @@ public void arraysActualNullMessage() { } @Test - public void arraysDifferentLengthMessage() { + public void arraysDifferentLengthDifferingAtStartMessage() { + try { + assertArrayEquals("not equal", (new Object[]{true}), (new Object[]{false, true})); + } catch (AssertionError exception) { + assertEquals("not equal: array lengths differed, expected.length=1 actual.length=2; arrays first differed at element [0]; expected: but was:", exception.getMessage()); + } + } + + @Test + public void arraysDifferentLengthDifferingAtEndMessage() { + try { + assertArrayEquals("not equal", (new Object[]{true}), (new Object[]{true, false})); + } catch (AssertionError exception) { + assertEquals("not equal: array lengths differed, expected.length=1 actual.length=2; arrays first differed at element [1]; expected: but was:", exception.getMessage()); + } + } + + @Test + public void arraysDifferentLengthDifferingAtEndAndExpectedArrayLongerMessage() { try { - assertArrayEquals("not equal", (new Object[0]), (new Object[1])); + assertArrayEquals("not equal", (new Object[]{true, false}), (new Object[]{true})); } catch (AssertionError exception) { - assertEquals("not equal: array lengths differed, expected.length=0 actual.length=1", exception.getMessage()); + assertEquals("not equal: array lengths differed, expected.length=2 actual.length=1; arrays first differed at element [1]; expected: but was:", exception.getMessage()); } } @@ -219,7 +252,7 @@ public void multiDimensionalArraysDifferentLengthMessage() { try { assertArrayEquals("message", new Object[][]{{true, true}, {false, false}}, new Object[][]{{true, true}, {false}}); } catch (AssertionError exception) { - assertEquals("message: arrays first differed at element [1]; array lengths differed, expected.length=2 actual.length=1", exception.getMessage()); + assertEquals("message: array lengths differed, expected.length=2 actual.length=1; arrays first differed at element [1][1]; expected: but was:", exception.getMessage()); return; } @@ -231,7 +264,7 @@ public void multiDimensionalArraysDifferentLengthNoMessage() { try { assertArrayEquals(new Object[][]{{true, true}, {false, false}}, new Object[][]{{true, true}, {false}}); } catch (AssertionError exception) { - assertEquals("arrays first differed at element [1]; array lengths differed, expected.length=2 actual.length=1", exception.getMessage()); + assertEquals("array lengths differed, expected.length=2 actual.length=1; arrays first differed at element [1][1]; expected: but was:", exception.getMessage()); return; } From 3701cd97e39744314a1574acfd48a9ebd4e03f24 Mon Sep 17 00:00:00 2001 From: Tom Anderson Date: Sun, 15 May 2016 23:42:19 +0100 Subject: [PATCH 2/3] Make testing of array equality more thorough and consistent --- .../junit/tests/assertion/AssertionTest.java | 209 ++++++++++++------ 1 file changed, 136 insertions(+), 73 deletions(-) diff --git a/src/test/java/org/junit/tests/assertion/AssertionTest.java b/src/test/java/org/junit/tests/assertion/AssertionTest.java index d89997851db9..7116b3789e65 100755 --- a/src/test/java/org/junit/tests/assertion/AssertionTest.java +++ b/src/test/java/org/junit/tests/assertion/AssertionTest.java @@ -57,120 +57,117 @@ public void failWithMessageToString() { } } - @Test(expected = AssertionError.class) + @Test public void arraysNotEqual() { - assertArrayEquals((new Object[]{new Object()}), (new Object[]{new Object()})); + assertArrayEqualsFailure( + new Object[]{"right"}, + new Object[]{"wrong"}, + "arrays first differed at element [0]; expected:<[right]> but was:<[wrong]>"); } - @Test(expected = AssertionError.class) + @Test public void arraysNotEqualWithMessage() { - assertArrayEquals("not equal", (new Object[]{new Object()}), (new Object[]{new Object()})); - } - - @Test(expected = AssertionError.class) - public void arraysDifferentLengthDifferingAtStartNotEqual() { - assertArrayEquals("not equal", (new Object[]{true}), (new Object[]{false, true})); - } - - @Test(expected = AssertionError.class) - public void arraysDifferentLengthDifferingAtEndNotEqual() { - assertArrayEquals("not equal", (new Object[]{true}), (new Object[]{true, false})); - } - - @Test(expected = AssertionError.class) - public void arraysDifferentLengthDifferingAtEndAndExpectedArrayLongerNotEqual() { - assertArrayEquals("not equal", (new Object[]{true, false}), (new Object[]{true})); + assertArrayEqualsFailure( + "not equal", + new Object[]{"right"}, + new Object[]{"wrong"}, + "not equal: arrays first differed at element [0]; expected:<[right]> but was:<[wrong]>"); } @Test public void arraysExpectedNullMessage() { try { - assertArrayEquals("not equal", null, (new Object[]{new Object()})); + assertArrayEquals("not equal", null, new Object[]{new Object()}); } catch (AssertionError exception) { assertEquals("not equal: expected array was null", exception.getMessage()); + return; } + fail("should have thrown an exception"); } @Test public void arraysActualNullMessage() { try { - assertArrayEquals("not equal", (new Object[]{new Object()}), null); + assertArrayEquals("not equal", new Object[]{new Object()}, null); } catch (AssertionError exception) { assertEquals("not equal: actual array was null", exception.getMessage()); + return; } + fail("should have thrown an exception"); } @Test public void arraysDifferentLengthDifferingAtStartMessage() { - try { - assertArrayEquals("not equal", (new Object[]{true}), (new Object[]{false, true})); - } catch (AssertionError exception) { - assertEquals("not equal: array lengths differed, expected.length=1 actual.length=2; arrays first differed at element [0]; expected: but was:", exception.getMessage()); - } + assertArrayEqualsFailure( + "not equal", + new Object[]{true}, + new Object[]{false, true}, + "not equal: array lengths differed, expected.length=1 actual.length=2; arrays first differed at element [0]; expected: but was:"); } @Test public void arraysDifferentLengthDifferingAtEndMessage() { - try { - assertArrayEquals("not equal", (new Object[]{true}), (new Object[]{true, false})); - } catch (AssertionError exception) { - assertEquals("not equal: array lengths differed, expected.length=1 actual.length=2; arrays first differed at element [1]; expected: but was:", exception.getMessage()); - } + assertArrayEqualsFailure( + "not equal", + new Object[]{true}, + new Object[]{true, false}, + "not equal: array lengths differed, expected.length=1 actual.length=2; arrays first differed at element [1]; expected: but was:"); } @Test public void arraysDifferentLengthDifferingAtEndAndExpectedArrayLongerMessage() { - try { - assertArrayEquals("not equal", (new Object[]{true, false}), (new Object[]{true})); - } catch (AssertionError exception) { - assertEquals("not equal: array lengths differed, expected.length=2 actual.length=1; arrays first differed at element [1]; expected: but was:", exception.getMessage()); - } + assertArrayEqualsFailure( + "not equal", + new Object[]{true, false}, + new Object[]{true}, + "not equal: array lengths differed, expected.length=2 actual.length=1; arrays first differed at element [1]; expected: but was:"); } - @Test(expected = ArrayComparisonFailure.class) + @Test public void arraysElementsDiffer() { - assertArrayEquals("not equal", (new Object[]{"this is a very long string in the middle of an array"}), (new Object[]{"this is another very long string in the middle of an array"})); + assertArrayEqualsFailure( + "not equal", + new Object[]{"this is a very long string in the middle of an array"}, + new Object[]{"this is another very long string in the middle of an array"}, + "not equal: arrays first differed at element [0]; expected: but was:"); } @Test public void arraysDifferAtElement0nullMessage() { - try { - assertArrayEquals((new Object[]{true}), (new Object[]{false})); - } catch (AssertionError exception) { - assertEquals("arrays first differed at element [0]; expected: but was:", exception - .getMessage()); - } + assertArrayEqualsFailure( + new Object[]{true}, + new Object[]{false}, + "arrays first differed at element [0]; expected: but was:" + ); } @Test public void arraysDifferAtElement1nullMessage() { - try { - assertArrayEquals((new Object[]{true, true}), (new Object[]{true, - false})); - } catch (AssertionError exception) { - assertEquals("arrays first differed at element [1]; expected: but was:", exception - .getMessage()); - } + assertArrayEqualsFailure( + new Object[]{true, true}, + new Object[]{true, false}, + "arrays first differed at element [1]; expected: but was:" + ); } @Test public void arraysDifferAtElement0withMessage() { - try { - assertArrayEquals("message", (new Object[]{true}), (new Object[]{false})); - } catch (AssertionError exception) { - assertEquals("message: arrays first differed at element [0]; expected: but was:", exception - .getMessage()); - } + assertArrayEqualsFailure( + "message", + new Object[]{true}, + new Object[]{false}, + "message: arrays first differed at element [0]; expected: but was:" + ); } @Test public void arraysDifferAtElement1withMessage() { - try { - assertArrayEquals("message", (new Object[]{true, true}), (new Object[]{true, false})); - fail(); - } catch (AssertionError exception) { - assertEquals("message: arrays first differed at element [1]; expected: but was:", exception.getMessage()); - } + assertArrayEqualsFailure( + "message", + new Object[]{true, true}, + new Object[]{true, false}, + "message: arrays first differed at element [1]; expected: but was:" + ); } @Test @@ -229,22 +226,88 @@ public void multiDimensionalArraysDeclaredAsOneDimensionalAreEqual() { @Test public void multiDimensionalArraysAreNotEqual() { - try { - assertArrayEquals("message", (new Object[][]{{true, true}, {false, false}}), (new Object[][]{{true, true}, {true, false}})); - fail(); - } catch (AssertionError exception) { - assertEquals("message: arrays first differed at element [1][0]; expected: but was:", exception.getMessage()); - } + assertArrayEqualsFailure( + "message", + new Object[][]{{true, true}, {false, false}}, + new Object[][]{{true, true}, {true, false}}, + "message: arrays first differed at element [1][0]; expected: but was:"); } @Test public void multiDimensionalArraysAreNotEqualNoMessage() { + assertArrayEqualsFailure( + new Object[][]{{true, true}, {false, false}}, + new Object[][]{{true, true}, {true, false}}, + "arrays first differed at element [1][0]; expected: but was:"); + } + + @Test + public void twoDimensionalArraysDifferentOuterLengthNotEqual() { + Object[] extraArray = {true}; + assertArrayEqualsFailure( + "not equal", + new Object[][]{extraArray, {}}, + new Object[][]{{}}, + "not equal: array lengths differed, expected.length=1 actual.length=0; arrays first differed at element [0][0]; expected: but was:"); + assertArrayEqualsFailure( + "not equal", + new Object[][]{{}, extraArray}, + new Object[][]{{}}, + "not equal: array lengths differed, expected.length=2 actual.length=1; arrays first differed at element [1]; expected:<" + extraArray.toString() + "> but was:"); + assertArrayEqualsFailure( + "not equal", + new Object[][]{{}}, + new Object[][]{extraArray, {}}, + "not equal: array lengths differed, expected.length=0 actual.length=1; arrays first differed at element [0][0]; expected: but was:"); + assertArrayEqualsFailure( + "not equal", + new Object[][]{{}}, + new Object[][]{{}, extraArray}, + "not equal: array lengths differed, expected.length=1 actual.length=2; arrays first differed at element [1]; expected: but was:<" + extraArray.toString() + ">"); + } + + @Test + public void twoDimensionalArraysDifferentInnerLengthNotEqual() { + assertArrayEqualsFailure( + "not equal", + new Object[][]{{true}, {}}, + new Object[][]{{}, {}}, + "not equal: array lengths differed, expected.length=1 actual.length=0; arrays first differed at element [0][0]; expected: but was:"); + assertArrayEqualsFailure( + "not equal", + new Object[][]{{}, {true}}, + new Object[][]{{}, {}}, + "not equal: array lengths differed, expected.length=1 actual.length=0; arrays first differed at element [1][0]; expected: but was:"); + assertArrayEqualsFailure( + "not equal", + new Object[][]{{}, {}}, + new Object[][]{{true}, {}}, + "not equal: array lengths differed, expected.length=0 actual.length=1; arrays first differed at element [0][0]; expected: but was:"); + assertArrayEqualsFailure( + "not equal", + new Object[][]{{}, {}}, + new Object[][]{{}, {true}}, + "not equal: array lengths differed, expected.length=0 actual.length=1; arrays first differed at element [1][0]; expected: but was:"); + } + + private void assertArrayEqualsFailure(Object[] expecteds, Object[] actuals, String expectedMessage) { + try { + assertArrayEquals(expecteds, actuals); + } catch (ArrayComparisonFailure e) { + assertEquals(expectedMessage, e.getMessage()); + return; + } + fail("should have thrown an exception"); + } + + private void assertArrayEqualsFailure(String message, Object[] expecteds, Object[] actuals, String expectedMessage) { try { - assertArrayEquals((new Object[][]{{true, true}, {false, false}}), (new Object[][]{{true, true}, {true, false}})); - fail(); - } catch (AssertionError exception) { - assertEquals("arrays first differed at element [1][0]; expected: but was:", exception.getMessage()); + assertArrayEquals(message, expecteds, actuals); + } catch (ArrayComparisonFailure e) { + assertEquals(expectedMessage, e.getMessage()); + return; } + fail("should have thrown an exception"); } @Test From 755afe847b6e96938d93def6a1d768e4ec65eb56 Mon Sep 17 00:00:00 2001 From: Tom Anderson Date: Mon, 16 May 2016 00:06:52 +0100 Subject: [PATCH 3/3] Produce a more legible failure message when multidimensional arrays are of different lengths --- .../junit/internal/ComparisonCriteria.java | 40 ++++++++++++++----- .../junit/tests/assertion/AssertionTest.java | 36 +++++++++++++---- 2 files changed, 60 insertions(+), 16 deletions(-) diff --git a/src/main/java/org/junit/internal/ComparisonCriteria.java b/src/main/java/org/junit/internal/ComparisonCriteria.java index 8cbcfc9c7d48..92781cf0e586 100644 --- a/src/main/java/org/junit/internal/ComparisonCriteria.java +++ b/src/main/java/org/junit/internal/ComparisonCriteria.java @@ -81,8 +81,8 @@ private void arrayEquals(String message, Object expecteds, Object actuals, boole } if (actualsLength != expectedsLength) { - Object expected = getArrayElementOrSentinel(expecteds, expectedsLength, prefixLength); - Object actual = getArrayElementOrSentinel(actuals, actualsLength, prefixLength); + Object expected = getToStringableArrayElement(expecteds, expectedsLength, prefixLength); + Object actual = getToStringableArrayElement(actuals, actualsLength, prefixLength); try { Assert.assertEquals(expected, actual); } catch (AssertionError e) { @@ -91,15 +91,37 @@ private void arrayEquals(String message, Object expecteds, Object actuals, boole } } - private static final Object END_OF_ARRAY_SENTINEL = new Object() { - @Override - public String toString() { - return "end of array"; + private static final Object END_OF_ARRAY_SENTINEL = objectWithToString("end of array"); + + private Object getToStringableArrayElement(Object array, int length, int index) { + if (index < length) { + Object element = Array.get(array, index); + if (isArray(element)) { + return objectWithToString(componentTypeName(element.getClass()) + "[" + Array.getLength(element) + "]"); + } else { + return element; + } + } else { + return END_OF_ARRAY_SENTINEL; } - }; + } - private Object getArrayElementOrSentinel(Object array, int length, int index) { - return index < length ? Array.get(array, index) : END_OF_ARRAY_SENTINEL; + private static Object objectWithToString(final String string) { + return new Object() { + @Override + public String toString() { + return string; + } + }; + } + + private String componentTypeName(Class arrayClass) { + Class componentType = arrayClass.getComponentType(); + if (componentType.isArray()) { + return componentTypeName(componentType) + "[]"; + } else { + return componentType.getName(); + } } private boolean isArray(Object expected) { diff --git a/src/test/java/org/junit/tests/assertion/AssertionTest.java b/src/test/java/org/junit/tests/assertion/AssertionTest.java index 7116b3789e65..e56b0cbc78df 100755 --- a/src/test/java/org/junit/tests/assertion/AssertionTest.java +++ b/src/test/java/org/junit/tests/assertion/AssertionTest.java @@ -243,27 +243,49 @@ public void multiDimensionalArraysAreNotEqualNoMessage() { @Test public void twoDimensionalArraysDifferentOuterLengthNotEqual() { - Object[] extraArray = {true}; assertArrayEqualsFailure( "not equal", - new Object[][]{extraArray, {}}, + new Object[][]{{true}, {}}, new Object[][]{{}}, "not equal: array lengths differed, expected.length=1 actual.length=0; arrays first differed at element [0][0]; expected: but was:"); assertArrayEqualsFailure( "not equal", - new Object[][]{{}, extraArray}, + new Object[][]{{}, {true}}, new Object[][]{{}}, - "not equal: array lengths differed, expected.length=2 actual.length=1; arrays first differed at element [1]; expected:<" + extraArray.toString() + "> but was:"); + "not equal: array lengths differed, expected.length=2 actual.length=1; arrays first differed at element [1]; expected: but was:"); assertArrayEqualsFailure( "not equal", new Object[][]{{}}, - new Object[][]{extraArray, {}}, + new Object[][]{{true}, {}}, "not equal: array lengths differed, expected.length=0 actual.length=1; arrays first differed at element [0][0]; expected: but was:"); assertArrayEqualsFailure( "not equal", new Object[][]{{}}, - new Object[][]{{}, extraArray}, - "not equal: array lengths differed, expected.length=1 actual.length=2; arrays first differed at element [1]; expected: but was:<" + extraArray.toString() + ">"); + new Object[][]{{}, {true}}, + "not equal: array lengths differed, expected.length=1 actual.length=2; arrays first differed at element [1]; expected: but was:"); + } + + @Test + public void primitiveArraysConvertedToStringCorrectly() { + assertArrayEqualsFailure( + "not equal", + new boolean[][]{{}, {true}}, + new boolean[][]{{}}, + "not equal: array lengths differed, expected.length=2 actual.length=1; arrays first differed at element [1]; expected: but was:"); + assertArrayEqualsFailure( + "not equal", + new int[][]{{}, {23}}, + new int[][]{{}}, + "not equal: array lengths differed, expected.length=2 actual.length=1; arrays first differed at element [1]; expected: but was:"); + } + + @Test + public void twoDimensionalArraysConvertedToStringCorrectly() { + assertArrayEqualsFailure( + "not equal", + new Object[][][]{{}, {{true}}}, + new Object[][][]{{}}, + "not equal: array lengths differed, expected.length=2 actual.length=1; arrays first differed at element [1]; expected: but was:"); } @Test