If Tolerance is used a different comparer is used

[manfred-brands]

As a result of the changes for #4436 and #4312, we are now in a situation that if a type implements IEquatable<T> and no Tolerance is specified, the type's Equal(T) method is called.

Howevever, if Tolerance is specified, #4312 would throw an InvalidOperationException
After the change #4436, the PropertiesComparer.Equals method is called which calls Equals on all properties, passing in the specified Tolerance.

I wanted to align NUnit.Analyzer with the new PropertiesComparer, relaxing the {Within does not make sense rule](nunit/nunit.analyzers#617), when I stumbled over this odd behaviour.

What behaviour do we want if a type implements IEquatable<T> (or overrides Equals(object)) and Tolerance is specified:

1. throw InvalidOperationException?
2. Fallback to PropertiesComparer changing the comparison used and using the Tolerance

I'm leaning to 1. Which requires a minor change to NUnit4.

[OsirisTerje]

Would alternative 1 mean that if I create my own generic numeric type, I could not use Tolerance?

[manfred-brands]

Would alternative 1 mean that if I create my own generic numeric type, I could not use Tolerance?

How could it? Tolerance is passed to NUnitEqualityComparer and only applied to types it knows about.
If your type is unknown to NUnit, NUnit calls Equals(object) on your type.

[OsirisTerje]

How could it?

@manfred-brands What is the problem? This is an issue you have worked on, not me. I haven't looked at the code at all. I am just asking.

[manfred-brands]

What is the problem?

NUnit doesn't know anything about your type and your type does know nothing about NUnit (I hope).
If your type implements IEquatable<T> you tell users how to test instances of your class for equality.
There is no notion of Tolerance in standard .NET, the Tolerance passed into NUnit test cannot be passed to the Equals method.

If your type does not implement IEquatable<T> it would fall back to object.Equals which for a class does reference comparison and for a struct exact value comparisons, neither support Tolerance.
The change in NUnit4 is that it then tries to compare all public properties, which could have basic types like double to which Tolerance is applied.

This issue is that if a type defined IEquality and the NUnit test passes in Tolerance an error is no longer given,
but silently a different comparison is used than anticipated.

[manfred-brands]

Note that option 1 is what NUnit 3.13 does.

[manfred-brands]

Whilst testing this out on a repository for work, I noticed we are inconsistent.
We don't throw if Tolerance is passed to string or char comparers, but we do for enum.
I added a commit to also not do this for the latter.

However, should NUnit throw or should only NUnit.Analyzers warn?
As the latter is optional, this defeats the original issue #4312.
But that issue is not complete as it doesn't throw for string nor for Stream

[OsirisTerje]

Do you mean that it should throw for string and stream too, which makes sense to me? Shouldn't that then be added to the PR ?

[manfred-brands]

Thanks, I will update the PR.

[manfred-brands]

Update the PR:

Comparers determine if Tolerance is supported.

Treatment is now consistent.
It will complain about 'string', 'char', 'enum' and 'IEquatable'
For Tuples and user defined types, if one member supports Tolerance,
the whole Tuple/Class/Struct does.