views:

31

answers:

4

I have a test which looks a bit like this (I'm simplifying a bit from my real issue):

[Test]
public void Eat_calls_consumption_tracker_OnConsume()
{
   var consumptionTrackerStub = 
      MockRepository.GenerateStub<IConsumptionTracker>();
   var monkey = new Monkey(consumptionTrackerStub);
   var banana = new Banana();

   monkey.Eat(banana);

   consumptionTrackerStub.AssertWasCalled(x => x.OnConsume(banana));
}

This would work fine, except that Monkey disposes Banana after eating it. Therefore, the banana object is no longer in a usable state. In particular, the Banana.Equals implementation cannot work after Dispose is called because it uses unmanaged resources that have been released.

Unfortunately AssertWasCalled will cause Banana.Equals to be called, which blows up, causing the test to fail. What is the best way to fix this?

+1  A: 

What if Monkey.Eat had a parameter of IBanana (or IFood), and IBanana descended from IDisposable? Then you could mock IBanana and even verify that Dispose was called on it.

EDIT: You can do it with Moq:

[Test]
public void Eat_calls_consumption_tracker_OnConsume()
{
    var consumptionTrackerStub = new Mock<IConsumptionTracker>();
    var monkey = new Monkey(consumptionTrackerStub.Object);
    var banana = new Banana();
    monkey.Eat(banana);

    consumptionTrackerStub.Verify(x => x.OnConsume(It.IsAny<Banana>()));
}

public class Banana
{
    public override bool Equals(object obj)
    {
        throw new ObjectDisposedException("Banana");
    }
}

public class Monkey
{
    public Monkey(IConsumptionTracker tracker)
    {
        _tracker = tracker;
    }

    public void Eat(object obj)
    {
        _tracker.OnConsume(obj);
    }

    private readonly IConsumptionTracker _tracker;
}

public interface IConsumptionTracker
{
    void OnConsume(object obj);
}
TrueWill
+1 I was already aware that stubbing everything out except the class under test would make this problem disappear. Unfortunately both `Monkey` and `Banana` are interop wrappers which cannot be abstracted away - the Monkey.Eat method "peels" the managed wrapper of the banana before passing it to its unmanaged equivalent, so it relies on banana being of a specific class type. The need for interop with existing unmanaged code really complicates proper TDD.
Wim Coenen
+1  A: 

I would be inclined to suggest that the class that instantiates an object should dispose of it. It's never a good idea to pass a Banana to a Monkey and then trust it to Dispose properly after consuming.

Can you not pass the data required to instantiate Banana into Monkey, rather than the Banana itself? Alternatively, can you not Dispose of it in your calling class?

pdr
Ownership of an object can be transfered - you just have to document where it happens. Otherwise it would be impossible to have factories for disposable objects.
Wim Coenen
Yeah, maybe my comment should be phrased "the class which requests instantiation" or something like that. I would agree that it's ok to instantiate something disposable and throw it back to another class, releasing your hold over it (factory style). My issue comes with generating a class that requires disposal and then passing it to another class, particularly while holding onto it yourself, meaning that either of two classes may try to Dispose of the object. Seems difficult to debug problems with that.
pdr
A: 

Several possible solutions:

  1. Maybe you could relax your assertion: do you really need to check that what's been eaten is banana?
  2. Use an inline callback on OnConsume and assert it's banana and not some other fruit. This callback would be executed before the banana has been disposed of.

BTW: now that I think about it, I think your code has a conceptual problem: how do you know some implementation of OnConsume will not want to keep the banana peel?

Igor Brejc
There is no conceptual problem - part of the contract of banana is that it raises a Disposing event so that non-owners can abandon their reference right before it is disposed.
Wim Coenen
Well, introducing a callback is still the easiest way to solve your problem. Or you can implement a DummyConsumptionTracker which checks that banana is supplied in the method call. The same goes for asserting that Disposing event was raised.
Igor Brejc
A: 

Turns out you can force Rhino Mocks to check the arguments with object.ReferenceEquals like this:

[Test]
public void Eat_calls_consumption_tracker_OnConsume()
{
   var consumptionTrackerStub = 
      MockRepository.GenerateStub<IConsumptionTracker>();
   var monkey = new Monkey(consumptionTrackerStub);
   var banana = new Banana();

   monkey.Eat(banana);

   consumptionTrackerStub.AssertWasCalled(
      x => x.OnConsume(
         Arg<Banana>.Matches(
            y => object.ReferenceEquals(y, banana))));
}

object.ReferenceEquals still works even if the banana has been disposed, thus fixing the problem.

Wim Coenen