Is.SupersetOf and Is.SubsetOf no longer work with IImmmutableDictionary<TKey,TValue> in NUnit 3.13.3

[danlyons-home]

The following test passes in 3.13.2, but it fails in 3.13.3:

using System.Collections.Immutable;
using NUnit.Framework;

namespace testimmutabledictionary;

[TestFixture]
public class Class1
{
    [Test]
    public void TestImmutableDictionary()
    {
        var numbers = Enumerable.Range(1, 3);
        var test1 = numbers.ToImmutableDictionary(t => t);
        var test2 = numbers.ToImmutableDictionary(t => t);

        Assert.That(test1, Is.SubsetOf(test2));
    }
}

Output:

  Failed TestImmutableDictionary [54 ms]
  Error Message:
     Expected: subset of < [1, 1], [2, 2], [3, 3] >
  But was:  < [1, 1], [2, 2], [3, 3] >
  Extra items: < [1, 1], [2, 2], [3, 3] >

  Stack Trace:
     at testimmutabledictionary.Class1.TestImmutableDictionary() in C:\danl\testimmutabledictionary\Class1.cs:line 16


Failed!  - Failed:     1, Passed:     0, Skipped:     0, Total:     1, Duration: 54 ms - testimmutabledictionary.dll (net6.0)

I notice that if both calls are instead ToDictionary, the test passes, but if either one is ToImmmutableDictionary, it fails.
I also noticed that using any of ToImmutableList, ToImmutableArray, or ToImmutableHashSet works (even when paired with non-immutable counterparts), so this seems limited to immutable dictionaries in particular.

project files attached:
testimmutabledictionary.zip

[danlyons-home]

Oops - I got my wires crossed and used Is.SubsetOf in my example, while the tests I was originally troubleshooting were using Is.SupersetOf. Both have the same behavior between the different versions, though.

[stevenaw]

Thanks @danlyons-softek for the clear repro.
There were some changes in 3.13.3 to add some optimized fast paths for these. It should've been backwards compatible, but it's possible this was missed.

[stevenaw]

Can confirm this is present on v4 as well. Thanks for spotting this.

It looks like the attempt to fast-path building a sortable ArrayList if the expected argument is an ICollection will also convert the underlying type of the items from KeyValuePair<T1,T2> to a non-generic DictionaryEntry:

nunit/src/NUnitFramework/framework/Constraints/CollectionTally.cs

Lines 132 to 133 in 2e39a96

	if (items is ICollection ic)
	return new ArrayList(ic);

The conversion itself is done within the ImmutableDictionary, when CopyTo is called:
https://github.com/dotnet/runtime/blob/main/src/libraries/System.Collections.Immutable/src/System/Collections/Immutable/ImmutableDictionary_2.cs#L764

This is then causing the slowpath comparer to fail when trying to do a deep-equality check here:

nunit/src/NUnitFramework/framework/Constraints/CollectionTally.cs

Line 92 in 2e39a96

if (ItemsEqual(_missingItems[index], o))

since the slowpath skips converting the actual argument to an ArrayList here:

nunit/src/NUnitFramework/framework/Constraints/CollectionTally.cs

Line 108 in 2e39a96

var remove = ToArrayList(c);

Leaving the constraint as trying to compare two fundamentally different types. The simplest fix here is probably to remove this fastpath check from within NUnit so that we always add the items to the ArrayList individually

if (items is ICollection ic) 
     return new ArrayList(ic); 

[mikkelbu]

Closed by #4097

[stevenaw]

Thanks! I should be able to get the v4 merge PR up within the next few days

[manfred-brands]

@stevenaw This bug has been in v3.13.3 for over a year, can we release a v3.13.4

[stevenaw]

Agreed, I'd been thinking about this just last week that we should get 3.14 out.