views:

45

answers:

2

Hello all,

(C#, Rhino Mocks, MbUnit).

I have a class called AccountManager that has a RegisterUser() method. This method returns void but does throw an exception for any errors. AccountManager calls into an IDataRepository calling its AddUser() method to do a database insert.

I'm mocking the IDataRepository using a Rhino Mock and throwing and exception for a given set of arguments simulating the exception being raised in the repository.

[Test]
    public void RegisterKnownUser()
    {
        MockRepository mocks = new MockRepository();
        IDataRepository dataRepository = mocks.StrictMock<IDataRepository>();

        using (mocks.Record())
        {
            Expect.Call(() => dataRepository.AddUser("abc", "abc", "[email protected]", "a", "bc")).Throw(
                new InvalidOperationException());
        }

        using (mocks.Playback())
        {
            AccountManager manager = new AccountManager(dataRepository);
            Assert.Throws(typeof (InvalidOperationException), () => manager.RegisterUser("abc", "abc", "[email protected]", "a", "bc"));
        }
    }

This test works fine.

My question is what to do about the situation where the args supplied to RegisterUser are correct and valid. The real IDataRepository would not return anything nor would it thrown any exceptions. So in short AccountManager's state would not have changed. Does this mean I don't need to test AccountManager.RegisterUser when it would result in nothing I can observe directly in the class and method under test. Testing against state in the mock smells a bit to me. I think as long as I test IDataRepository.AddUser seperately then I shouldn't need to test AccountManager.RegisterUser for inputs that would result in nothing observable in the class.

Thanks in advance.

A: 

You may want to test the method with valid arguments to ensure it does not throw any exception. In other words, you cannot observe state change or use a return value (since it's void), but you can observe that the method ran without exceptions.

By the way, if the method does not return a value nor change AccountManager state, it does change something otherwise (if not, than you should probably remove from your code the method which does nothing at all).
For example, it may affect DataRepository. Or add a record in the database. In this case, you can test at least if the data is changed or if the record is added successfully. Or it may log an event saying that the new user was registered, so you will be able, in your tests, to check if the log event is here.

I think as long as I test IDataRepository.AddUser seperately then I shouldn't need to test AccountManager.RegisterUser for inputs that would result in nothing observable in the class

If AccountManager.RegisterUser adds nothing to IDataRepository.AddUser except arguments enforcement, than yes, you don't have to test it if you already tested IDataRepository.AddUser. If it checks arguments, calls AddUser and does something else, it will be good to check if what it does is correct.

Let's say you have:

public void AddUser(string userName, string userMail, string passwordHash)
{
    // [...] Add a user to the database.
}

public void RegisterUser(string userName, string userMail, string passwordHash)
{
    if (string.IsNullOrEmpty(userName)) throw new ArgumentNullException(...);
    if (string.IsNullOrEmpty(userMail)) throw new ArgumentNullException(...);
    if (string.IsNullOrEmpty(passwordHash)) throw new ArgumentNullException(...);
    if (!Checks.IsValidMail(userMail)) throw new ArgumentException(...);

    this.AddUser(userName, userMail, passwordHash);

    this.SaveToLog(LogEvent.UserRegistered, userName, this.IPAddress);
}

In RegisterUser, you test the first four lines by passing wrong arguments and expecting an exception. The fifth line must not be tested, since you already tested AddUser. Finally, the sixth line must be tested to ensure that when you call RegisterUser with valid arguments, the log entry is created.

MainMa
+2  A: 

If AccountManager calls into DataPrepository, then your test case still validates something. The record/playback here validates that a call is made into it. If the call is not made, the test case will fail. If it is made twice/with the wrong args, it will fail.

This may be a very basic test case, but it is still a good one, and doesn't require you to place state in the mock object.

Merlyn Morgan-Graham
Thanks (and to Main Ma as well). I've gone down the route of testing the exception is not thrown for good data.
Simon Rigby