Wednesday, April 23, 2014

Unit testing calls to base class

There is no reasonable way to test whether a method from base class was invoked in derived class. You need to redesign your code to get rid of inheritance so test it thoroughly.

Consider following code where we have a RequestError class responsible for processing that is common to all types of errors. Specialized classes, derive from RequestError and perform additional handling that is specific to the error they represent e.g. show message on dispatcher's console when unauthorized request is made, and use parent handling as well.

public class RequestError
{
  public virtual void Handle(Details requestDetails)
  {
    /*
      Perform logic common to all errors, e.g. logging
      * */
  }
}

public class UnauthorizedRequestError : RequestError
{
  public override void Handle(Details requestDetails)
  {
    ShowOnDispatcherConsole("Unathorized attempt from IP: " + requestDetails.IPAddress);
    base.Handle(requestDetails);
  }

  private void ShowOnDispatcherConsole(string message)
  {
    /*
      ...
    * */
  }
}


To refactor this, I identified all delegations to base class and then removed inheritance and injected dependency object through the constructor. I also extracted interface from this class so it's easier for mocking purposes. The UnauthorizedRequestError now is a kind of decorator around RequestError. The code and test now look like this:

public interface IRequestError
{
  void Handle(Details requestDetails);
}

public class RequestError : IRequestError
{
  public void Handle(Details requestDetails)
  {
    /*
      Perform logic common to all errors, e.g. logging
      * */
  }
}

public class UnauthorizedRequestError : IRequestError
{
  private readonly IRequestError _generalError;

  public UnauthorizedRequestError(IRequestError generalError)
  {
    _generalError = generalError;
  }

  public void Handle(Details requestDetails)
  {
    ShowOnDispatcherConsole("Unathorized attempt from IP: " + requestDetails.IPAddress);
    _generalError.Handle(requestDetails);
  }

  private void ShowOnDispatcherConsole(string message)
  {
    /*
      ...
    * */
  }
}


[Test]
public void ShouldDelegateErrorHandlingToWrappedError()
{
  //GIVEN
  IRequestError generalError = Substitute.For<IRequestError>();
  var unauthorizedError = new UnauthorizedRequestError(generalError);
  var details = new Details();

  //WHEN
  unauthorizedError.Handle(details);

  //THEN
  generalError.Received(1).Handle(details);
}

Saturday, April 12, 2014

Verify event was raised with NSubstitute

I used to unit-test events by subscribing to them with my custom handler which job was to set a flag when the event was called and remember its parameters (if existed). At the end I used assertions to verify if the event was raised and if so whether the parameters were correct.

The behavior I'm going to test is that OfficeDevice should raise Alarm when someone wants to print any document when there is no ink in the device. This is how my tests looked like.

[Test]
public void ShouldAnnounceWhenPrintingWithNoInk()
{
  //GIVEN
  var device = new OfficeDevice(ink: 0);
  InkLevel? level = null;
  device.InkAlarm += (levelParameter) => { level = levelParameter; };

  //WHEN
  device.Print(Any<IDocument>());

  //THEN
  Assert.AreEqual(InkLevel.None, level);
}

This is actually similar to what NSubstitute documentation encourages to do. There's nothing wrong with this. However, a colleague showed me another way of testing events with the spy and verify tactics.

[Test]
public void ShouldAnnounceWhenPrintingWithNoInk()
{
  //GIVEN
  var device = new OfficeDevice(ink: 0);
  var subscriber = Substitute.For<IDummySubscriber>();
  device.InkAlarm += subscriber.React;

  //WHEN
  device.Print(Any<IDocument>());

  //THEN
  subscriber.Received(1).React(InkLevel.None);
}

IDummySubscriber is just a dummy interface for mocking purpose.

public interface IDummySubscriber
{
  void React(InkLevel level);
}


I like the second approach because it's much neater. You can to get rid of flags and lambdas and replace them with mock verification.