Assert.Equal on nullable IEnumerable

[treziac]

Hi,

With latest xunit 2.4.2, the following

Assert.Equal(expected.Collection, actual.Collection);

with Collection being a IEnumerable<string>? doesn't compile anymore because of nullable check.
In my casen Collection is expected to be null in some cases (optional field, not the case of "empty collection"). This is part of a global method I reuse in our tests, so sometimes it is an instantiated collection and sometimes it's null.

That's because this method is called
https://github.com/xunit/assert.xunit/blob/main/CollectionAsserts.cs#L470
which does not accept nullable ernumerable.

However, the base method (called by CollectionAssert) does allow null
https://github.com/xunit/assert.xunit/blob/1cacd928ce604173281587b16ca945bcff1c5f64/EqualityAsserts.cs#L75

So my code compiles and works if I write this
Assert.Equal<IEnumerable>(expected.Collection, actual.Collection); (but can get quite verbose on some type)
or this
Assert.Equal(expected.Collection!, actual.Collection!); (but I hate adding bang operator on nullable)

Is this a wanted behavior to disallow enumerable null in CollectionAssert (assuming usually an Enumerable should not be null)?

[treziac]

Actually, the question is perhaps more global on nullable

In my production code, I prefer to never have the bang operator (!) as it opens to potential bug.
In test, do you perform NotNull check or simply use bang operator everywhere? (as anyway, at worse we will end up comparing something to null or generate an exception which will make the test fail)

[bradwilson]

This was an intentionally breaking change.

Previously, without the nullable system in place, it was possible to pass null values into Assert.Equal<T>(IEnumerable<T> expected, IEnumerable<T> actual) and they would get forwarded onto the generic Assert.Equal<T>(T expected, T actual), which would allow two null values to be equal (this is clearly the correct behavior for generic types of T which are not enumerable).

When enabling nullability, I decided that null collections are a degenerate case, and wanted the nullability flag to trigger if you tried to pass them in.

There are two possibilities for your tests here, and the code you write differs depending on which situation you're in:

1. If you believe that null collections are legitimate, and that two null collections should be equal, then you must use ! to bypass the null check with the compiler. The existing code path continues to pass the collections into the generic Assert.Equal<T>(), so this will restore the pre-nullable behavior from 2.4.1. Trying to use Assert.NotNull on a null collection will obviously cause your test to fail; the nullability bypass (!) is the only way to avoid this.
2. If you believe that null collections are not legitimate, then you should use Assert.NotNull on any suspect collection values to verify they're not null before passing them into Assert.Equal.