views:

232

answers:

1

Today I ran into the following problem with NUnit.

I have a class, that derives from a generic class. I started to do some serialization tests and tested for equality using the Is.EqualTo() function of NUnit.

I started to suspect something is wrong, when a test that should have failed passed instead. When I used obj1.Equals(obj2) instead it failed as it should.

To investigate I created the following tests:

namespace NUnit.Tests

{

using Framework;

    public class ThatNUnit
    {
        [Test]
        public void IsNotEqualTo_ClientsNotEqual_Passes()
        {
            var client1 = new DerrivedClient();
            var client2 = new DerrivedClient();

            client1.Name = "player1";
            client1.SomeGenericProperty = client1.Name;
            client2.Name = "player2";
            client2.SomeGenericProperty = client2.Name;

            Assert.That(client1.Equals(client2), Is.False);
            Assert.That(client1, Is.Not.EqualTo(client2));
        }

        [Test]
        public void IsNotEqualTo_ClientsAreEqual_AlsoPasses_SomethingWrongHere()
        {
            var client1 = new DerrivedClient();
            var client2 = new DerrivedClient();

            client1.Name = "player1";
            client1.SomeGenericProperty = client1.Name;
            client2.Name = client1.Name;
            client2.SomeGenericProperty = client1.Name;

            Assert.That(client1.Equals(client2), Is.True);
            Assert.That(client1, Is.Not.EqualTo(client2));
        }
    }

    public class DerrivedClient : Client<string>
    {
    }

    public class Client<T>
    {
        public string Name { get; set; }

        public T SomeGenericProperty { get; set; }

        public override bool Equals(object obj)
        {
            if (ReferenceEquals(null, obj))
            {
                return false;
            }
            if (ReferenceEquals(this, obj))
            {
                return true;
            }
            if (obj.GetType() != typeof(Client<T>))
            {
                return false;
            }
            return Equals((Client<T>)obj);
        }

        public bool Equals(Client<T> other)
        {
            if (ReferenceEquals(null, other))
            {
                return false;
            }
            if (ReferenceEquals(this, other))
            {
                return true;
            }
            return Equals(other.Name, Name) && Equals(other.SomeGenericProperty, SomeGenericProperty);
        }

        public override int GetHashCode()
        {
            unchecked
            {
                return ((Name != null ? Name.GetHashCode() : 0) * 397) ^ SomeGenericProperty.GetHashCode();
            }
        }

        public override string ToString()
        {
            return string.Format("{0}, {1}", Name, SomeGenericProperty);
        }
    }
}

The two (actually conflicting Asserts) in the second test show the problem:

Assert.That(client1.Equals(client2), Is.True);
Assert.That(client1, Is.Not.EqualTo(client2));

This test should fail one way or the other, but it doesn't!

So I dug a little bit into NUnit's source code, only to find, that after some if()s for some special conditions, the ObjectsAreEqual(object x, object y) method (which eventually gets called via Assert.That(x, Is.EqualTo(y)), comes to this line of code:

return x.Equals(y);

I find that very perplexing, since I must now think, that Is.EqualTo() just takes a longer route, but basically should be doing the same as x.Equals(y)

Here the full method for whoever is interested (inside the NUNit.Framework.Constraints namespace):

  public bool ObjectsEqual(object x, object y)
    {
        this.failurePoints = new ArrayList();

        if (x == null && y == null)
            return true;

        if (x == null || y == null)
            return false;

        Type xType = x.GetType();
        Type yType = y.GetType();

        if (xType.IsArray && yType.IsArray && !compareAsCollection)
            return ArraysEqual((Array)x, (Array)y);

        if (x is ICollection && y is ICollection)
            return CollectionsEqual((ICollection)x, (ICollection)y);

        if (x is IEnumerable && y is IEnumerable && !(x is string && y is string))
            return EnumerablesEqual((IEnumerable)x, (IEnumerable)y);

        if (externalComparer != null)
            return externalComparer.ObjectsEqual(x, y);

        if (x is string && y is string)
            return StringsEqual((string)x, (string)y);

        if (x is Stream && y is Stream)
            return StreamsEqual((Stream)x, (Stream)y);

        if (x is DirectoryInfo && y is DirectoryInfo)
            return DirectoriesEqual((DirectoryInfo)x, (DirectoryInfo)y);

        if (Numerics.IsNumericType(x) && Numerics.IsNumericType(y))
            return Numerics.AreEqual(x, y, ref tolerance);

        if (tolerance != null && tolerance.Value is TimeSpan)
        {
            TimeSpan amount = (TimeSpan)tolerance.Value;

            if (x is DateTime && y is DateTime)
                return ((DateTime)x - (DateTime)y).Duration() <= amount;

            if (x is TimeSpan && y is TimeSpan)
                return ((TimeSpan)x - (TimeSpan)y).Duration() <= amount;
        }

        return x.Equals(y);
    }

So what is going on here and how can it be fixed?

I want to be able to trust my tests and thus necessarily NUnit again.

I also don't want to start using Equals() instead of Is.EqualTo() (the former doesn't give me such a nice output when the test fails).

Thanks in advance.

Thorsten

UPDATE

In the meantime I wrestled further with this problem and found a similar problem here and posted a possible workaround.

+4  A: 

The problem is that the second assertion of the second test calls the Equals overload that accepts an object rather than a Client<T>, so this comparison returns false:

// obj.GetType() returns Client.DerrivedClient

if (obj.GetType() != typeof(Client<T>))
{
    return false;
}

To fix this, you can change the comparison operation to this:

if (obj.GetType() != this.GetType())
Jeff Sternal
Thanks Jeff, this seems to be on the right track.My simple example got fixed that way, but for the real case I'm still struggling.This will teach me in the future to not just take the validity of generated code for granted.
Thorsten Lorenz
My pleasure - I imagine the real case is a serious pain; my only advice is to take a break (even a short one!) so you can look at it with fresh eyes. Good luck!
Jeff Sternal