Assert.AreEqual behaves very differently in 3.0 from 2.5.10

[Paul Adams]

We're in the process of upgrading our tests from 2.5.10 to 3.0.1.  We make extensive use of a struct that has an overloaded Equal method so it can be compared with an int.  Post upgrade we see lots of failures that look like this:

Expected: -1

  But was:  -1

These errors are all from a call to Assert.AreEqual where the first argument is an integer and the second is in instance of our struct with various implicit operators and Equal method overload.  I've searched the known issues but have only found things tangentially related (like #275: NUnitEqualityComparer fails to compare IEquatable<T> where second object is derived from T ).  Is this a known problem?  Is there any workaround where we could add something to our code?  

Thanks.

Paul Adams

p...@cbord.com

FYI, our class has an overridden Equals method:

/// <summary>

/// Allow equality tests with ObjectId's , int's and short's.

/// </summary>

/// <param name="id"></param>

/// <returns></returns>

public override bool Equals(Object id)

{

if (id is ObjectId)

{

return this == (ObjectId)id;

}

if (id is int)

{

return this == (ObjectId)(int)id;

}

if (id is short)

{

return this == (ObjectId)(int)(short)id;

}

return false;

}

It also has an implicit conversion to/from an int:

/// <summary>

/// convert from an int to us.

/// </summary>

/// <param name="val"></param>

/// <returns></returns>

public static implicit operator ObjectId(int val)

{

ObjectId returnVal;

returnVal.mOid = val;

returnVal.mIsValid = true;

return returnVal;

}

/// <summary>

/// covert from us to an int.

/// </summary>

/// <param name="val"></param>

/// <returns></returns>

public static implicit operator int(ObjectId val)

{

if (val.IsNull)

{

throw new CBORDException("Reference to null ObjectId");

}

return val.mOid;

}

[Charlie Poole]

I would expect code like...
Assert.AreEqual(5, myStruct)
to fail, since it will use int.Equals to make the comparison.

However, I would have expected that failure in 2.5.10 as well.
However, since you are jumping 5 or 6 releases here, there may be
something that has changed in between.

Can you attach a snippet of a test that works in NUnit 2.5.10 but fails in 3.0?

Charlie

> --
> You received this message because you are subscribed to the Google Groups
> "NUnit-Discuss" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to nunit-discus...@googlegroups.com.
> To post to this group, send email to nunit-...@googlegroups.com.
> Visit this group at https://groups.google.com/group/nunit-discuss.
> For more options, visit https://groups.google.com/d/optout.

[Paul Adams]

Sure. I think if int.Equals was called it would be fine because of the implicit conversion operator to int.  FYI, I found if that I reversed the arguments to Assert.AreEqual things were OK.  In other words:

Assert.AreEquals(a, b) fails, but Assert.AreEquals(b, a) works.

I expect equality to be symmetric.

Using our struct the following used to work:

{

ObjectId oid = -1;

const int PEANUT_ALLERGY_OID = -1;

Assert.AreEqual(PEANUT_ALLERGY_OID, oid)

}

now if I rewrite it to use

Assert.AreEqual(oid, PEANUT_ALLERGY_OID) 

things are ok.

I attached a copy of our ObjectId struct if you want to try it.  It basically wraps an int and a bool and then has implicit operators for conversion to/from an int, CompareTo, Equals methods (and more).

Thanks.

Paul

[Paul Adams]

Looking at the older version of Nunit (2.5.10) I see that the old Assert class had overloaded 'AreEqual' methods for all the value types (e.g. int).  This I think explains why things used to work:  The C# compiler would find the best match and use whatever implicit type conversion was available.  So in my example, the C# compiler would pick the Assert.AreEqual(int32,int32) overload and handle the conversion of our ObjectId type to an int32.  We have several other types that behave the same way (e.g. Currency has an automatic conversion to decimal).  These types have the same problem with the new Nunit3 version of the Assert class.  

I see 2 solutions:

1. Try to handle type conversions deep inside the Assert.That methods.  This can get complicated and messy.  Plus all the reflective access could slow things down (I have not yet dived into the code here, so I'm just guessing).

2. Bring back all the overloaded methods in the Assert class.  Very verbose, but it makes the C# compiler do all the work.

Not sure what I would pick.  It's too bad that the older Assert methods are not backwards compatible.

For us to move forward, we can either:

1. Rewrite our tests to use cast expressions 

2. Make a custom version of Nunit with the overloads we need (a pretty small list for us as we tend to make the same assertion patterns over and over again).  

3. Explore the use of implementing IEquatable<T> which has shown some promise to solve our issues.

I like #3 if it works and as a fallback use #1 (we don't have a lot of time left to spend on the upgrade). 

[Rob Prouse]

All equality in NUnit tunnels down to NUnitEqualityComparer if you want to look there. I don't have the code in front of me right now, but we likely call actual.Equals(expected) or the other way around, but not both. As you say, equality is supposed to be symetric and probably generally is in nunit, but at that level we are working with objects so your implicit conversion operator isn't being called.

As you say, IEquatable is likely your best option.

--

Rob Prouse

 

I welcome VSRE emails. Learn more at http://vsre.info/

[Charlie Poole]

@rprouse The only thing that bothers me is that it apparently worked
before. I don't recall anything we intentionally changed around this,
although we have made some changes to that code.

[Rob Prouse]

Charlie, it worked before because we had an int overload to AreEqual which triggered the implicit conversion to int so we ended up comparing two ints. We are now working with two objects so the implicit conversion doesn't happen. We could check for the presence of an implicit conversion operator in NUnitEqualityComparer and use it if found on actual or expected.

Personally, I dislike implicit conversions so don't like encouraging them by supporting them, but I won't force that on others if people want it :)

[Charlie Poole]

Paul,

You're right. Equality is supposed to be symmetric and NUnit assumes
it is by picking one of the args (the expected value) on which to call
Equals. However, what seems likely here is that we are using object
equality rather than int.

IIRC (I'm not actually looking at the code right now) we make tests to
find out if both values are "numeric" and apply conversions in each
case. Unfortunately, we don't recognize your custom type as numeric.
We have had issues filed on this in the past. Folks have asked for a
way to indicate to us that their own special type should be treated as
numeric. Up to now, we have not found a clean way to do do so - and
frankly have not spent a lot of time on it.

I'd be curious to know if you get the same result using Assert.That.
An error in how Assert.That works would get higher priority than one
in the classic (legacy) asserts.

Charlie

[Charlie Poole]

Hi Rob,

So NUnitEqualityComparer actually would have gotten two boxed ints,
discovered their actual type and acted accordingly. That makes sense.

I'd hate to add back all those overloads that we removed. I think it
would be better if we could provide a clear path to defining one's own
pseudo-numeric types. I think that would have to include support for
implicit conversion though - it's hard to create a new numeric without
it.

Charlie