Improve failure message from UniqueItemsConstraint

[mikkelbu]

Given the test below, then I expect that is is easy to see the item that is not unique.

[TestFixture]
public class Class1
{
    [Test]
    public void Test()
    {
        int[] actual = { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 12, 13 };
        Assert.That(actual, Is.Unique);
    }
}

But this gives

  Message:   Expected: all items unique
      But was:  < 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 12, 13 >

If the list is even longer then it can be that we cannot see the duplicated element

[Test]
public void Test()
{
    int[] actual = { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 12, 13 };
    var longList = actual.ToList();
    longList.AddRange(Enumerable.Range(0, 20));
    Assert.That(longList, Is.Unique);
}

Gives

  Message:   Expected: all items unique
      But was:  < 1, 2, 3, 4, 5, 6, 7, 8, 9, 10... >

[rprouse]

Sounds good. Should we switch the second line of the message to something like,

Not unique items: < 12 >

[stevenaw]

If a collection has multiple sets of non-unique values:

int[] actual = { 1, 2, 3, 4, 4, 5, 6, 7, 8, 9, 10, 11, 12, 12, 13 };

should this output all non-unique values:

Not unique items: < 4, 12 >

or just the first non-unique found?

Not unique item found: < 4 >

[rprouse]

I think showing all non-unique values would be best, but either is an improvement 😄

[stevenaw]

Thanks, makes sense to me @rprouse 😀
I'd be happy to take this on.

[Dreamescaper]

I would suggest to output actual collection as is, and output non-unique only items as third line (via WriteAdditionalLinesTo).
E.g.

Expected: all items unique
But was:  < 1, 2, 3, 4, 4, 5, 6, 7, 8, 9, 10, 11, 12, 12, 13 >
Not unique items: < 4, 12 >

That would be more informative, and consistent with other similar cases.

[stevenaw]

That makes sense to me. @mikkelbu what are your thoughts? I'm happy to implement in either direction, but as the issue creator I'd appreciate your input.

[CharliePoole]

What of extreme cases? For example...

• Collection with 1000 items in it.
• Collection with 100 non-unique items.

Seems to me we used to truncate all these lists automatically to some reasonable size that fit on a single line but had a special switch that could force the full display if it should be needed on a re-run. Do we still have that feature? Or is it only for equality tests?

[Dreamescaper]

Agree with @CharliePoole, it should be limited (I think that 10 items is default in such cases).
It's certainly not for equality checks only, I think it is truncated in all cases where collection is displayed in assertion message.

[mikkelbu]

I just took a look at the code. And the only place where we truncate the third line of text, i.e. after Expected: ... But was: .... is in CollectionEquivalentConstraintResult where we output at most 10 missing elements and at most 10 additional elements. In several places we also truncate the expected and the actual value (search for MsgUtils.FormatCollection), here we again use 10 as the max.

So I think it makes sense to add the third line as proposed by @Dreamescaper, but at most present 10 non-unique values

[stevenaw]

Great, thanks everyone for the input!