Unit testing/How to assert a non-mocked method was called/C#/Rhino Mocks/NUnit

Question

Hi,

I have a MailService which permit me to send Email which implement the following Interface.

public interface IMailService
  {
    bool SendRegisteringEmail(RegisterModel registerModel);
    bool SendMail(MailMessage mailMessage);
    MailMessage CreateRegisteringMailMessage(RegisterModel registerModel);

    IAppSettingsRepository AppSettingsRepository { get; set; }

    ISmtpClient SmtpClient { get; set; }
  }

The function SendRegisteringEmail should call CreateRegisteringMailMessage then give the return MailMessage to the SendMail function and SendRegisteringEmail should return the return Boolean of SendMail.

I'm using NUnit and Rhino Mocks to do my test, I'm new to testing (1week) and I'm practicing TDD (At least I try). My problem is I don't know how to assert that CreateRegisteringMailMessage was called when I call SendRegisteringEmail, because MailService isn't a Mocked Object. Here is my test:

[Test]
public void SendRegisteringEmail_ShouldCallCreateRegisteringMailMessage()
{
  //Arrange
  //MailMessage mailMessage = new MailMessage();
  IMailService mailService = new MailService { AppSettingsRepository = CreateAppSettingsMockReposotory() };
  //ISmtpClient smtpClientStub = MockRepository.GenerateStub<ISmtpClient>();
  //mailService.SmtpClient = smtpClientStub;
  //smtpClientStub.Stub(f => f.Send(mailMessage)).Return(true);


  //Act
  mailService.SendRegisteringEmail(ValidRegisterModel);

  //Assert
  mailService.AssertWasCalled(f => f.CreateRegisteringMailMessage(ValidRegisterModel));
}

I get the following error when I launch my test: FtpWeb.Tests.MailServiceTests.SendRegisteringEmail_ShouldCallCreateRegisteringMailMessage: System.InvalidOperationException : The object 'FtpWeb.Models.MailService' is not a mocked object.

I understand why I'm getting this error but now how to test my call. Since it's the same object I can't mock it to test it. If anybody can give me some lead to resolve this.

Thanks.

Answer 1

I'm not sure you should be testing that CreateRegisteringMailMessage is called. You should just be checking the outcome of calling SendRegisteringEmail. What should that do with your MailService implementation? Presumably it will have some effect on another service - so test that.

It would be an odd interface design which insisted that calling one method would require another method in the interface to be called - quite often you might make both methods call one private, common method for example.

Answer 2

This is often referred to an 'sensing' within a class under test: you have to find a way to tell whether a method has been called. These kinds of things are covered well in the book 'Working Effectively with Legacy Code' which everyone should have (disclaimer: I have no affiliation with the book's author, I just think it's a good book to have).

What does the CreateRegisteringMailMessage method do? Does it affect the content of the RegisterModel instance passed in during the call?

Answer 3

The methods CreateRegisteringMailMessage and SendMail seem on a lower abstraction level than SendRegisteringEmail. You could consider creating a higher level class containing SendRegisteringEmail, and this class would use IMailService, which you could mock and assert as usual.

If the first solution is not an option, then you should treat SendRegisteringEmail as a whole, so effectively you have to test the side effects of both CreateRegisteringMailMessage and SendMail in a single test (a bit of a code smell, hence my suggestion to extract another level of indirection) - see Jon's answer.

Answer 4

@JonSkeet: CreateRegisteringMailMessage is only called by SendRegisteringEmailm so it doesn't need to be in the public interface. What does IMO means?

I know it's against TDD but since I don't know how to write tests I decided to implement my functions.

public bool SendRegisteringEmail(RegisterModel registerModel)
{
  MailMessage mailMessage = CreateRegisteringMailMessage(registerModel);

  return SendMail(mailMessage);
}

Do I have to only test the return of SendRegisteringEmail. If yes, is it the only test I have to do on this function because there isn't lot of logic inside?

@Grzenio: Was about to send this post when I saw your answer which can be really great. I will try that when I have time today.

Answer 5

(First sorry if I did wrong by posting another answer to my post, I don't know if editing a post tell people that my thread has been changed)

Here the solution I implemented. I split my IMailService interface in three part:

First one is IMailService itself, I made this one alone because I will need to send mail in another part of my apps, it looks like :

 public interface IMailService
  {
    ISmtpClient SmtpClient { get; set; }
    bool SendMail(MailMessage mailMessage);
  }

The second is IRegisterRepository, I didn't know how to name it, but since repository is responsible for saving add and do operation on this object I thought it was a good name:

public interface IRegisterRepository
  {
    IAppSettingsRepository AppSettingsRepository { get; set; }
    IMailMessageRepository MailMessageRepository { get; set; }
    IMailService MailService { get; set; }
    bool SendRegisteringEmail(RegisterModel registerModel);
  }

The last one is IMailMessageRepository, same problem with name, I will have to create more mailMessage so I thounght creating this interface will serve me in future. (Finding the good name for objet and interface is really hard... Don't have enough experience on that):

public interface IMailMessageRepository
  {
    IAppSettingsRepository AppSettingsRepository { get; set; }

    MailMessage CreateRegisteringMailMessage(RegisterModel registerModel);
  }

With this three class I'm able to do all my tests.

So Thanks to Jon Skeet for the idea about the wrong use of my interface, thanks to Grzieno for the idea of the higher level class(Worked for me :D). Thanks too to Dr Herbie, I will keep your idea in a corner because it can help me on another future test.

PS: As new to TDD I made a (good for learning) mistake, I began to refactor my code... my production code... Instead of beginning by my tests, took me an hour to get the job done. Next time I have to refactor first I will refactor my test so I will not loose myself into where to put which function or property.