CODEWITHSUNDEEP
c#unit-testingmocking
Kenny Eliasson
Kenny Eliasson

Reputation: 2052

Unit testing with Mocks. Test behaviour not implementation

I always had a problem when unit testing classes that calls other classes, for example I have a class that creates a new user from a phone-number then saves it to the database and sends a SMS to the number provided.

Like the code provided below.

public class UserRegistrationProcess : IUserRegistration
{
    private readonly IRepository _repository;
    private readonly ISmsService _smsService;

    public UserRegistrationProcess(IRepository repository, ISmsService smsService)
    {
        _repository = repository;
        _smsService = smsService;
    }

    public void Register(string phone)
    {
        var user = new User(phone);
        _repository.Save(user);
        _smsService.Send(phone, "Welcome", "Message!");
    }
}

It is a really simple class but how would you go about and test it?

At the moment im using Mocks but I dont really like it

    [Test]
    public void WhenRegistreringANewUser_TheNewUserIsSavedToTheDatabase()
    {
        var repository = new Mock<IRepository>();
        var smsService = new Mock<ISmsService>();
        var userRegistration = new UserRegistrationProcess(repository.Object, smsService.Object);

        var phone = "07012345678";

        userRegistration.Register(phone);
        repository.Verify(x => x.Save(It.Is<User>(user => user.Phone == phone)), Times.Once());
    }

    [Test]
    public void WhenRegistreringANewUser_ItWillSendANewSms()
    {
        var repository = new Mock<IRepository>();
        var smsService = new Mock<ISmsService>();
        var userRegistration = new UserRegistrationProcess(repository.Object, smsService.Object);

        var phone = "07012345678";

        userRegistration.Register(phone);

        smsService.Verify(x => x.Send(phone, It.IsAny<string>(), It.IsAny<string>()), Times.Once());
    }

It feels like I am testing the wrong thing here?

Any thoughts on how to make this better?

Upvotes: 4

Views: 1515

Answers (3)

Lunivore
Lunivore

Reputation: 17612

Refactoring the mocks out in the way that @Serghei suggests is good.

I also see that the name of the behaviour isn't actually describing the behaviour. I like to use the word "should", as in, "My class should do some stuff".

Your class shouldn't send the user to the database when it's registering a user. It should ask the repository to save the user. That's all. It doesn't know whether the repository sends it to the database, keeps it in memory or nukes it from orbit. It's not your class's responsibility.

By phrasing the behaviour this way, you can explicitly show - and help others understand - where the scope of your class's responsibility ends.

If you rename your method something like WhenRegisteringANewUser_AsksRepositoryToSaveIt() that might make the example you've given feel more natural.

Upvotes: 4

Sergey K
Sergey K

Reputation: 4114

look at after some refactor

 Mock<IRepository<>> repository;
    private Mock<ISmsService> smsService;
    const string phone = "0768524440";
    [SetUp]
    public void SetUp()
    {
           repository = new Mock<IRepository<>>();
         smsService = new Mock<ISmsService>();
    }
    [Test]
    public void WhenRegistreringANewUser_TheNewUserIsSavedToTheDatabase()
    {

        var userRegistration = new UserRegistrationProcess(repository.Object, smsService.Object);

        userRegistration.Register(phone);

        repository.VerifyAll();
        smsService.VerifyAll();
    }

Upvotes: 0

Sergey K
Sergey K

Reputation: 4114

In your case don't need to write 

  repository.Verify(x => x.Save(It.Is<User>(user => user.Phone == phone)), Times.Once());

because this method doesn't return a value you can write

repository.VerifyAll();

Also for smsService it's a good way to use Moq.

Upvotes: 0

Related Questions