`Using<TCollectionType, TMemberType>` is unclear

[RenderMichael]

Excuse the example, it's a little contrived, it's as minimal a repro as I could think of.

I am working with ZIP archives, and I want to make sure a entry I am getting (which comes from another archive) is contained in my reference archive. My custom comparer is failing, though:

using NUnit.Framework.Constraints;

using System.Collections.ObjectModel;
using System.IO.Compression;

namespace NUnitRepro2;

public class Tests
{
    [Test] // fails
    public void ZipArchiveContainsTest()
    {
        using var archiveContents = new MemoryStream();
        using ZipArchive archive = ConstantArchive(archiveContents);
        using var archiveContents2 = new MemoryStream();
        using ZipArchive otherArchive = ConstantArchive(archiveContents2);

        ZipArchiveEntry entry = otherArchive.Entries[0];
        Assert.That(archive.Entries, Does.Contain(entry)
            .Using<ReadOnlyCollection<ZipArchiveEntry>, ZipArchiveEntry>((collection, needle) =>
            {
                foreach (var actualEntry in collection)
                {
                    using var leftValue = actualEntry.Open();
                    using var rightValue = needle.Open();

                    var comparison = new EqualConstraint(leftValue).ApplyTo(rightValue);
                    return comparison.Status is ConstraintStatus.Success;
                }

                return false;
            }));
    }

    [Test] // success!
    public void ZipArchiveWorkaroundTest()
    {
        using var archiveContents = new MemoryStream();
        using ZipArchive archive = ConstantArchive(archiveContents);
        using var archiveContents2 = new MemoryStream();
        using ZipArchive otherArchive = ConstantArchive(archiveContents2);

        ZipArchiveEntry entry = otherArchive.Entries[0];

        ZipArchive entryWrapper = MakeNewArchiveAroundEntry(entry);
        Assert.That(archive.Entries, Is.EqualTo(entryWrapper.Entries).Using<ZipArchiveEntry>((left, right) =>
        {
            using var leftValue = left.Open();
            using var rightValue = right.Open();

            var comparison = new EqualConstraint(leftValue).ApplyTo(rightValue);
            return comparison.Status is ConstraintStatus.Success;
        }));
    }

    private static ZipArchive MakeNewArchiveAroundEntry(ZipArchiveEntry entry)
    {
        var archiveContents = new MemoryStream();

        using (var archive = new ZipArchive(archiveContents, ZipArchiveMode.Create, leaveOpen: true))
        {
            ZipArchiveEntry demoFile = archive.CreateEntry(entry.Name);

            using Stream entryStream = demoFile.Open();
            using Stream entryContents = entry.Open();
            entryContents.CopyTo(entryStream);
        }

        return new ZipArchive(archiveContents, ZipArchiveMode.Read, leaveOpen: false);
    }

    private static ZipArchive ConstantArchive(MemoryStream archiveContents)
    {
        const string fileContentString = "Hello!";

        using (var archive = new ZipArchive(archiveContents, ZipArchiveMode.Create, leaveOpen: true))
        {
            ZipArchiveEntry demoFile = archive.CreateEntry("Hello entry");

            using Stream entryStream = demoFile.Open();

            using var entryFs = new StreamWriter(entryStream);
            entryFs.Write(fileContentString);
        }

        return new ZipArchive(archiveContents, ZipArchiveMode.Read, leaveOpen: false);
    }
}

As you can see, my workaround has been to make a new archive around the entry, and use a custom comparer there. However, there's still an underlying issue with SomeItemsConstraint.Using<TCollectionType, TMemberType> which doesn't exist with EqualConstraint.Using<T>.

Unless I am doing something wrong here and there's no bug, I am happy to try and fix this myself.

[manfred-brands]

@RenderMichael The overload of Using<TCollectionType, TMemberType> has a different parameter constraint than you expected. The first parameter is not the type of the collection, but the type of the entries in the collection. The comparer is called by the EqualConstraint for equality of entries in the collection and the requested entry.

Despite that this is mentioned in the XML doc on the method, the naming is not clear.
It is shown in intellisense when entereing the type parameters itself:

But not when hovering over the Using itself:

In the NUnit documentation, it isn't explained at all:

It would have been clearer if we rename the first type parameter to TCollectionMemberType and the second to TExpectedType

In your case both of types should be ZipArchiveEntry .
The check EqualityAdapter.CanCompare returns false because the parameters don't match your declaration and NUnit actually never calls your method.

A working version of your test passes a Using delegate comparing two ZipArchiveEntries:

        [Test] // succeeds
        public void ZipArchiveContainsTest2()
        {
            using var archiveContents = new MemoryStream();
            using ZipArchive archive = ConstantArchive(archiveContents);
            using var archiveContents2 = new MemoryStream();
            using ZipArchive otherArchive = ConstantArchive(archiveContents2);

            ZipArchiveEntry entry = otherArchive.Entries[0];
            Assert.That(archive.Entries, Does.Contain(entry)
                .Using<ZipArchiveEntry, ZipArchiveEntry>((actualEntry, needle) =>
                {
                    using var leftValue = actualEntry.Open();
                    using var rightValue = needle.Open();

                    var comparison = new EqualConstraint(leftValue).ApplyTo(rightValue);
                    return comparison.Status is ConstraintStatus.Success;
                }));
        }

[manfred-brands]

An example where the types are different would be if you only cared if there was an entry with a specific name:

Assert.That(archive.Entries, Has.Member("Hello Entry")
    .Using<ZipArchiveEntry, string>((actualEntry, needle) => actualEntry.Name == needle));

[RenderMichael]

@manfred-brands thanks for the tip! I confirmed it works.

You mentioned the comparer isn't getting invoked because EqualityAdapter.CanCompare returns false. Is it possible for an exception to be thrown in that case? It seems to me that an invalid external comparer is always a bug, and in cases less obvious than mine, might be a silent issue which nobody notices.

[manfred-brands]

@RenderMichael The Equality constraint allows for multiple comparer functions. It checks them all until it finds one where CanCompare returns true.

In the below (unrealistic) example, there are two external comparers registered, one which compares an int to an int and one which compares a double to an int and uses rounding.

var expected = new[] { 1, 2, 3 };
var actual = new object[] { 1.4, 2, 2.8 };

Assert.That(actual, Is.EqualTo(expected)
                      .Using<int, int>((actual, expected) => actual == expected)
                      .Using<double, int>((actual, expected) => actual >= expected - 0.5 && actual < expected + 0.5));

I can actually remove the <int, int> comparer as that is the same as the built-in one and the test still passes and the specified comparer is only called for double, not for int.

var expected = new[] { 1, 2, 3 };
var actual = new object[] { 1.4, 2, 2.8 };

Assert.That(actual, Is.EqualTo(expected)
                      .Using<double, int>((actual, expected) => actual >= expected - 0.5 && actual < expected + 0.5));

Changing NUnit to throw when none of the specified external comparers match, even if there is only one, would be a breaking change.

I have no idea if that fallback or even the multiple comparers is actually used by anyone, but it could be.

[RenderMichael]

In that case, this issue can be about the renaming suggestion/documentation of Using<TCollectionType, TMemberType>. Again, happy to implement that, as long as that change would be accepted.

[manfred-brands]

@RenderMichael Yes, I think that renaming the type parameters to TCollectionElementType and TExpectedType would clear things up.

[RenderMichael]

@manfred-brands opened #4729 for this, let me know if there's any improvements I could make.