A sort of fix for issue #1127

[tomwhoiscontrary]

In issue #1127, @sf105 noted that when comparing two arrays which differ in length, assertArrayEquals() will only report that they differ in length, which he found insufficient for easy diagnosis. As a fix, he suggested printing the complete actual array contents.

Rather than doing that, this PR changes assertArrayEquals() to do the usual array comparison even when arrays differ in length, producing a failure message which combines the difference in length and the first difference in content. I think this should ease diagnosis, as @sf105 wanted, and it doesn't require making big changes to the code, adding Hamcrest, introducing a new behaviour (JUnit doesn't print complete array contents anywhere else), or blowing people's IDEs up when they compare multi-megabyte byte arrays.

[kcooney]

The previous code just used Assert.fail() here. Any reason we should do something different? I seen to recall this code being a bit sensitive when handling nested arrays.

[tomwhoiscontrary]

@kcooney I need to do an assertEquals to get the right failure message. I could use fail, but then i would have to construct the failure message here, and that would duplicate the code in assertEquals.

[kcooney]

I will let @marcphilipp decide, but if the previous code used fail with a hard-coded message, then I think we should do the same here, to minimize the observable change on behavior.

[kcooney]

I really like this solution!

Can you add tests to verify the failure message for arrays of arrays where the second nested array has differing lengths?

[tomwhoiscontrary]

Will do. I did think briefly about the nested arrays case, and i am 70% sure it will just work. It should definitely have tests, though!

[tomwhoiscontrary]

Okay, i've changed the tests as suggested - being more careful to verify failure in each case, and adding tests for multidimensional arrays.

The testing did bring up a case where the message is poor:

        Object[] expecteds = new Object[][]{{}, new Object[]{true}};
        Object[] actuals = new Object[][]{{}};
        assertArrayEquals(expecteds, actuals);

The message is:

array lengths differed, expected.length=2 actual.length=1; arrays first differed at element [1]; expected:<[Ljava.lang.Object;@2b71fc7e> but was:<end of array>

Using the toString of an array in the message is not really very useful (although still better than what you get now, which is nothing). But i'm not sure what would be better, or how to make that happen.

[tomwhoiscontrary]

Okay! @kcooney, i think i've got good failure messages for all kinds of arrays now. Where the mismatching element is itself an array, the element comparison part of the message looks like:

// primitive array:
... expected:<int[1]> but was:<end of array>
// object array:
... expected:<java.lang.Object[1]> but was:<end of array>
// multidimensional array:
... expected:<java.lang.Object[][1]> but was:<end of array> // multidimensional array

That is, the type of the array, and its length. That should give users a bit of a clue as to where the arrays are different, without overwhelming them with detail.

I played around with dumping the complete array contents, but it requires a disproportionate amount of code, and, again, risks blowing people's IDEs up when they compare arrays which contain multi-megabyte byte arrays!

Apologies for the delay, i was hibernating. Can you (or anyone else) suggest any further improvements?

[marcphilipp]

I am closing this pull request because it targets the junit5 branch which we will delete in the next few days. Please open a pull request targeting master instead.

[tomwhoiscontrary]

@marcphilipp I'll happily raise a new PR against master if there's a chance that it will actually be merged.

Would it be okay to make the PR by merging this into master, or would you prefer a completely new branch that doesn't involve the junit5 branch in its history?

[marcphilipp]

@junit-team/junit-committers Do you guys have objections to such a pull request for master?

[kcooney]

@marcphilipp No objections

[marcphilipp]

@tomwhoiscontrary Feel free to provide a new PR against master. Can you rebase your commits on master first?

[tomwhoiscontrary]

Will do.