IsEquivalent on default ImmutableArray throws exception.

[manfred-brands]

NUnit version 3.13.0

An ImmutableArray is a structure, but can contained a null array.
The following test fails with an InvalidOperationException: This operation cannot be performed on a default instance of ImmutableArray

ImmutableArray<string> expected = ImmutableArray.Create("Test");
ImmutableArray<string> actual = default(ImmutableArray<string>);

Assert.That(actual, Is.EquivalentTo(expected));

Same for Assert.That(actual, Is.Not.Empty);

It is a pity that Microsoft doesn't treat this the same as ImmutableArray<string>.Empty.

[mikkelbu]

It is a bit unfortunate that we need to have special cases for all these structures, but I think we should also handle this.

[stevenaw]

Agreed, this is unfortunate.
Should default(ImmutableArray<T>) be treated as semantically equivalent to Empty or to null? NUnit makes a distinction between these for reference-type collections:

string[] expected = Array.Empty<string>();
string[] actual = null;
Assert.That(actual, Is.Not.EqualTo(expected));

Though I would also find it unusual for the following test to pass:

Assert.That(default(ImmutableArray<string>), Is.Null);

I wonder if this is case solved via a DefaultConstraint (#3457) ?

[manfred-brands]

It looks like Assert.That(default(ICollection), Is.Empty) throws an ArgumentException. The actual value must be a string, ...
and Assert.That(default(ICollection), Is.EquivalentTo(Array.Empty<string())) throws with mesage Expected IEnumerable But was: null

So it looks like in all cases, one would need to check for null (or Default) first, so maybe this does not need changing in NUnit.
In my case that would mean inserting an Assert.That(actualValue.IsDefault, Is.False) or use the new DefaultConstraint when available.

And maybe we need to add an nunit.analyzer rule warning about passing a possible null values to these constraints.

[jnm2]

I don't think NUnit should treat default(ImmutableArray<T>) as though it was ImmutableArray<T>.Empty. This will hide problems in my code.

If we work against the way ImmutableArray was intended to differentiate between .IsDefault and .IsEmpty, what about a user-defined struct type that implements IEnumerable<T> and is also invalid when default?

Is.Default sounds good to me.

[stevenaw]

I'm unsure if there's still interest in this getting fixed, but I noticed that we solved a similar case in the EqualConstraint for ArraySegment in the past in #477

[stevenaw]

I haven't checked the scenarios described in #477 (comment) for ImmutableArray but if they don't hold here then I'm wondering if this should be closed given the presence of Is.Default and @jnm2 's points above

[jnm2]

This type of error is an occupational hazard of using default structs which themselves throw these exceptions when you call through IEnumerable interfaces on them, not unique to NUnit. We could always provide hints added onto assertion messages if we detect a misunderstanding though.

[manfred-brands]

Note that the IsEmpty property on ImmutableArray will throw a NullReferenceException:

get { return this.array!.Length == 0; }

There is even a comment on the Length property: We intentionally do not check this.array != null, and throw NullReferenceException if this is called while uninitialized.

[stevenaw]

I agree that NUnit special casing "empty" doesn't make sense here. I'm a little less sure about the "Equivalent" case.

Perhaps it's just me, but I've always thought of NUnit collection equivalence as a looser form of collection equality in that all the same rules apply except ordering doesn't matter with equivalence. From that perspective, I would expect that any test which passes for Is.EqualTo() to also pass for Is.EquivalentTo(). However it seems neither ArraySegment<> or ImmutableArray<> follow this.

In other words, the below tests apply what I would consider increasingly looser equality definitions and so I would not expect the second or third assertion to ever fail if the first passes. An interesting note here is that not only does the third fail in both, they also do not output the custom message that I would expect.

[Explicit, Test]
public void TestImmutableArray()
{
    var a = default(ImmutableArray<int>);
    var b = default(ImmutableArray<int>);

    Assert.That(a.Equals(b), "Not equals");
    Assert.That(a, Is.EqualTo(b), "Not NUnit equals");
    Assert.That(a, Is.EquivalentTo(b), "Not NUnit equivalent");
}

[Explicit, Test]
public void TestArraySegment()
{
    var a = default(ArraySegment<int>);
    var b = default(ArraySegment<int>);

    Assert.That(a.Equals(b), "Not equals");
    Assert.That(a, Is.EqualTo(b), "Not NUnit equals");
    Assert.That(a, Is.EquivalentTo(b), "Not NUnit equivalent");
}

[jnm2]

Perhaps Is.EquivalentTo could first check EqualityComparer<T>.Default.Equals and return true if that returns true?

[stevenaw]

I think that's a nice easy win for the happy path here, for all types :)

If we're in agreement on approach here for the equivalent scenario, I'd like to suggest this again as a good first issue.