Unit Testing / TDD: not testing stuff

G'day:
It's about bloody time I got back to this series on TDD and unit testing. I've already got the next few articles semi-planned in my head: topic, if not content. I have to say I am not in the most inspired writing mood today, and the words aren't exactly flowing from fingers through keyboard to screen - this is the third attempt at this paragraph - but we'll see how I go.

To find inspiration and to free up the fingers a bit, I'm at my local pub in Merlin Park in Galway, which - today - has shown its true colours as a football pub (to you Americans, that's "soccer". To me, it's "shite"). I've tried to like football, but it all just seems too effeminate to me. The place is chock-full of colours-wearing lads yelling at the screen. Either for or against one of Chelsea or Liverpool (the former lead 2-1 at half time). It's not conducive to writing, but the Guinness next to me will be.

I'll not list the previous articles in the series as it will take up too much room. They're all tagged with "TDD" and "unit testing" though.

On with the show...


One lesson to learn when implementing one's tests is to be able to work out what needs testing. This isn't always immediately obvious, and it's not just a case of working out all the various cases to test for (I'll get to that in a later article), but also to pay attention to what not to test. And also how to get around having to test it. Work avoidance! Fantastic.

I have to admit the code for today's article is already written, and I'm not really focusing on TDD today per-se, so I figure that's OK. I'm borrowing some code for another article I'm writing on another topic today. Crafty.

OK, so in this article I'm going to be looking at a UserService. This is used to hit the DB and retrieve User objects, and/or authenticate them, etc (perhaps usually the remit of an "Authentication Service", but hey). The UserService calls a UserDAO which performs the DB activity. That DAO is initialised with a TransactionLog object which implements a Logger interface. And the UserService is also initialised with an AuditLog (which will log failed authentication attempts). The AuditLog encrypts its entries, so requires a CFC which implements the Encryptor Interface. Got that?

How about some code to initialise a UserService object, which demonstrates its dependencies:

userService = new testapp.users.UserService(
     userDAO = new testapp.users.MockedUserDAO(
          transactionLog = new testapp.loggers.TransactionLog(logFile="transactionLog")
     ),
     auditLog = new testapp.loggers.AuditLog(
          logFile          = "transactionLog",
          encrypter     = new testapp.security.StubEncrypter()
     )
);

What a mouthful. This is not as bad as it could be. At work we have service CFCs which need to be initialised with probably half a dozen other sub-service CFCs for various reasons, most of which take their own DAOs, and their own helper services. There's one or two egregiously bad examples which perhaps require about 50 lines of code to initialise (at one argument per one CFC per line). Ludicrous. This is at least partially down to poor architecture, and some limitations forced upon us by a questionable coding standard (we are working on both of these happenstances), but some of it is down to just the code being complicated. Anyway, let's be thankful our UserService is not too egregious.

Let's work through the methods we want to test. Here's the init() method:

public UserService function init(required IUserDAO userDAO, required testapp.loggers.Logger auditLog){
    structAppend(variables, arguments);
    return this;
}

On initial look, to test even just the init() method of this, we need to do this:

public void function testInit(){
    var userService = new testapp.users.UserService(
        userDAO = new testapp.users.UserDAO(
            transactionLog = new testapp.loggers.TransactionLog(logFile="transactionLog")
        ),
        auditLog = new testapp.loggers.AuditLog(
            logFile          = "transactionLog",
            encrypter     = new testapp.security.StubEncrypter()
        )
    );
    assertIsTypeOf(userService, "testapp.users.UserService");
}

That's ten lines of code, only one of which is needed for the actual test. Because what are we testing in the init() method? Really: nothing. It contains no code that impacts the outside world at all. Really, it's arguable whether we need to test init() at all. I always run a test on it simply to make sure it doesn't error, as much as testing it returns the correct type.

Fair enough, we can predict that all the tests are gonna need a UserService, so we could refactor the object creation into a setup() method, which simplifies our code slightly:

// TestUserService.cfc
component extends="mxunit.framework.TestCase" {

    public void function setup(){
        variables.userService = new testapp.users.UserService(
            userDAO = new testapp.users.UserDAO(
                transactionLog = new testapp.loggers.TransactionLog(logFile="transactionLog")
            ),
            auditLog = new testapp.loggers.AuditLog(
                logFile          = "transactionLog",
                encrypter     = new testapp.security.StubEncrypter()
            )
        );
    }

    public void function testInit(){
        assertIsTypeOf(variables.userService, "testapp.users.UserService");
    }

}

Well: currently it's more code, but we will be saving work with each subsequent test which will also be using setup() to create the test object. OK. Let's run with this for the time being (NB: it's not right, but improving this is part of the journey here).

The next method we need to test is this one:

public User function getUserById(required numeric id){
    var userRecord = userDAO.getUserById(id);
    if (userRecord.recordCount){
        return setFromRecord(user, userRecord);
    }else{
        throw(type="InvalidUserException", message="User does not exist", detail="No user with ID #id# was found");
    }
}

What do we need to test here? Well it hits the DB, and if it gets a user record we create a new User object, and return it. Otherwise we error out.

To test that a user gets returned OK, we need to get a record from the DB. Which record? A valid one. How do we know what will amount to be a valid record? We can't just pick any old ID and use it, as it might not return a record. So we need a known-good record. How do we find one of those? Is there an ID we can guarantee will exist? Quite possibly not. The DB might start with no users in it at all. Or it might install with a default user - eg: Administrator - but the ID might be a UUID set at install time. I guess we could just query for some random user, and then use its ID, eg:

public void function testGetUserById(){
    testRecord = new Query(datasource="something", sql="SELECT TOP 1 userId FROM users").execute().getResult();
    // rest of test
}

But that only works if there are records in the DB. And - let's face it - the whole reason for having a UserService is to decouple it from the DB. We've got a DAO for talking to the DB, haven't we? We do not want to emulate the UserDAO in our tests for the UserService. That's bad coupling.

OK, so let's use the actual UserDAO to get the record. That could possibly work, although that's coupling the DAO to the tests for the UserService, which is only slightly less bad that hitting the DB directly.

And do you want to know a show-stopping problem here? I'm doing TDD. I don't have a DAO yet. So there's no DAO to call. Not only do I not have a DAO, I haven't got a DB yet either. I don't even know what data persistence mechanism I'll be using for this data yet.

Does this mean I need to down-tools until I work out the data-persistence mechanism, then create the DAO?

Well... let's look at this method we're testing again:

public User function getUserById(required numeric id){
    var userRecord = userDAO.getUserById(id);
    if (userRecord.recordCount){
        return setFromRecord(userRecord);
    }else{
        throw(type="InvalidUserException", message="User does not exist", detail="No user with ID #id# was found");
    }
}

What is the logic we are testing in this? Remember unit tests are about testing logic. We need to test what happens if we do or do not get a record from the DB. Hang on... is it? Where does it mention a DB in there? Well the DAO will be hitting the DB, won't it? Or will it? In reality, does the getUserById() function actually care how userDAO.getUserById() works? It cares what it returns: because we need to test for whether it returns a recordCount or not, but the record doesn't need to come from a DB. There's nothing in the logic of getUserById() that cares where this record came from. All we need is for the DAO to have a function called getUserById(), and it needs to return a DB record (or, for the non-happy-path, not return a record).

So we could implement a stub DAO which simply has that function, and returns a contrived record. And because our UserDAO implements an interface, and because the UserService expects something that implements that interface, rather than expecting a concrete implementation, I can just  make my stub implement that same interface, and I'm away!

// StubUserServiceDAO.cfc
component implements="testapp.users.IUserDAO" {

    public query function getUserById(required numeric id){
        return queryNew("");
    }

}

Oops. IUserDAO expects more than just that, so I am currently getting an error in my tests:

Test	Error Info	Output	Result	Speed
testInit	component [StubUserDAO.cfc] does not implement the function [getuserbylogin(string loginid, string password)] of the interface [IUserDAO.cfc] Toggle Stack Trace		Error	196 ms
testGetUserById	component [StubUserDAO.cfc] does not implement the function [getuserbylogin(string loginid, string password)] of the interface [IUserDAO.cfc] Toggle Stack Trace		Error	18 ms

The interface dictates this:

// IUserDao.cfc
interface {

    public query function getUserById(required numeric id);
    public query function getUserByLogin(required string loginId, required string password);

}

Phew. Just the one other method. We'll stub that for the time being:

// StubUserServiceDAO.cfc
component implements="testapp.users.IUserDAO" {

    public query function getUserById(required numeric id){
        return queryNew("");
    }

    public query function getUserByLogin(required string loginId, required string password){
        return queryNew("");
    }

}

Using this stub allows setup() to run, as StubUserDAO.cfc implements the minimum it requires. But what if the interface required a whole bunch of other methods too? That's a lot of stubbing we need to do for our test of one single method. Even with just the one additional method to stub, we simply shouldn't need to do this in our TDD approach to writing tests. We should just be able to dive in, write the minimum amount of code we need to test the method we need to test. We shouldn't need to "know" about all these dependencies.

Fortunately there's a mechanism that has been created with precisely this requirement in mind. Mockbox.

I'll take a TDD approach to introducing Mockbox too: I'll explain just/only what I need to, do write my tests. You can read all about it on its website, but in short it's a system for making stubbed / mocked objects to facilitate focusing on writing just the code necessary to test a specific piece of code.

We initialise our MockBox "factory" by adding a beforeTests() function:

public void function beforeTests(){
    variables.mockbox = new mockbox.system.testing.Mockbox();
}

And then we can use MockBox to create mock versions of our helper objects, to initialise our test object with:

public void function setup(){
    variables.mockedUserDao = variables.mockbox.createEmptyMock("testapp.users.UserDAO");
    variables.mockedAuditLog = variables.mockbox.createEmptyMock("testapp.loggers.AuditLog");

    variables.userService = new testapp.users.UserService(
        userDAO = variables.mockedUserDao,
        auditLog = variables.mockedAuditLog
    );
}

See how I don't need to worry about the dependencies that UserDAO or AuditLog have, I can just create a "fake" version of them to use. I could have created them inline had I wanted to:

public void function setup(){
    variables.userService = new testapp.users.UserService(
        userDAO = variables.mockbox.createEmptyMock("testapp.users.UserDAO"),
        auditLog = variables.mockbox.createEmptyMock("testapp.loggers.AuditLog")
    );
}

But I want to reuse those objects later, hence putting them in the variables scope first. I'll explain in a bit.

I've opted to create empty mocks here. Let's have a look at what one of these entails:

Component (testapp.users.UserDAO)  Only the functions and data members that are accessible from your location are displayed

public $NEVER $never $ONCE $once $ARGS $args $ $ $DEBUG $debug $TIMES $times $GETPROPERTY $getProperty _MOCKCALLLOGGINGACTIVE boolean true MOCKBOX Component (mockbox.system.testing.Mockbox)  Only the functions and data members that are accessible from your location are displayed _MOCKRESULTS Struct _MOCKGENERATIONPATH string /mockbox/system/testing/stubs/ $COUNT $count $VERIFYCALLCOUNT $times _MOCKCURRENTMETHOD string _MOCKORIGINALMD Struct Entries: 13 $ATMOST $atMost $RESET $reset $PROPERTY $property $CALLLOG $callLog _MOCKCALLLOGGERS Struct _MOCKARGRESULTS Struct _MOCKMETHODCALLCOUNTERS Struct $RESULTS $results _MOCKCURRENTARGSHASH string $ATLEAST $atLeast

Very far from "empty", but what you might notice is none of the methods from UserDAO are present. Contrast this with a second option Mockbox has, createMock():

variables.mockedUserDao = variables.mockbox.createMock("testapp.users.UserDAO");

The dump of that one is similar, but has the methods from the original object:

Component (testapp.users.UserDAO)  Only the functions and data members that are accessible from your location are displayed

public GETUSERBYID Public Function getUserById source:C:\webroots\railo-express-win32\webapps\www\shared\git\blogExamples\unittests\examples\mocking\testapp\users\UserDAO.cfc arguments label name required type default hint id true numeric null return type query $ARGS $args $NEVER $never $TIMES $times $DEBUG $debug _MOCKCALLLOGGINGACTIVE boolean true $GETPROPERTY $getProperty MOCKBOX Component (mockbox.system.testing.Mockbox)  Only the functions and data members that are accessible from your location are displayed _MOCKRESULTS Struct _MOCKCURRENTMETHOD string _MOCKORIGINALMD Struct Entries: 13 $ATMOST $atMost $RESET $reset _MOCKCALLLOGGERS Struct $RESULTS $results $ONCE $once $ $ _MOCKGENERATIONPATH string /mockbox/system/testing/stubs/ $COUNT $count GETUSERBYLOGIN Public Function getUserByLogin source:C:\webroots\railo-express-win32\webapps\www\shared\git\blogExamples\unittests\examples\mocking\testapp\users\UserDAO.cfc arguments label name required type default hint loginId true string null password true string null return type query $VERIFYCALLCOUNT $times $PROPERTY $property $CALLLOG $callLog _MOCKARGRESULTS Struct _MOCKMETHODCALLCOUNTERS Struct _MOCKCURRENTARGSHASH string INIT Public Function init source:C:\webroots\railo-express-win32\webapps\www\shared\git\blogExamples\unittests\examples\mocking\testapp\users\UserDAO.cfc $ATLEAST $atLeast

There's cases for both, but I prefer using an empty mock because if any of my code under test calls stuff I don't expect, I like to know about it.

Now that I have an operational - but completely decoupled, dependency-free and mocked ~ - UserDAO, I can start writing my test:

public void function testGetUserById_validId(){
    var result = variables.userService.getUserById();
}

This will error:

Test	Error Info	Output	Result	Speed
testGetUserById_validId	The parameter id to function getUserById is required but was not passed in. Toggle Stack Trace		Error	47 ms

And this is obvious, as I've not passed an ID in. And getUserById() requires an ID. Which ID do we need to pass it? It needs to be an ID which will return a record. From our mocked DAO. Huh? How is that gonna work? Easy. The answer is we can pass any ID we want... we'll just tell our mocked DAO to return a record whatever input it gets:

public void function testGetUserById_validId(){
    variables.mockedUserDao.$(
        "getUserById",
        variables.mockbox.querySim("
            someCol
            someValue
        ")
    );
    var result = variables.userService.getUserById(1);
}

Here we use MockBox's $() method to mock a method. we are mocking MockedUserDao's getUserById() method to return a query (as mocked by querySim()), with a column "someCol" and a value in the first row of that column "someValue". That query is just a placeholder, but it means we can now call getUserById(), pass it any ID (in this case 1), and it'll return a query:

Test	Error Info	Output	Result	Speed
testGetUserById_validId	column [ID] not found in query, columns are [someCol] Toggle Stack Trace UserService.cfc (33)		Error	241 ms

This is predictable... we can't just use any old record set because getUserById() calls setFromRecord(), and on line 33 within setFromRecord() we access the ID column:

private User function setFromRecord(required query record){
    var user = new User();
    user.setId(record.id);
    user.setFirstName(record.firstname);
    user.setLastName(record.lastName);
    user.setLoginId(record.loginId);
    return user;
}

But... we're not testing setFromRecord() are we? No. We are testing getUserById(). So do we want to be messed around by the inner workings of setFromRecord() at the moment? I mean... until I ever mentioned setFromRecord() existed, did you know about it? No. So if we were doing TDD and currently working on just getUserById(), all we'd be likely to know is that: "if there's a record in the DB, then pass it to setFromRecord()". That's the logic in getUserById(), innit? Here it is again, for recap:

public User function getUserById(required numeric id){
    var userRecord = userDAO.getUserById(id);
    if (userRecord.recordCount){
        var user = new User();
        return setFromRecord(userRecord);
    }else{
        throw(type="InvalidUserException", message="User does not exist", detail="No user with ID #id# was found");
    }
}

So that's what we need to test. Just that the record from the DB - if there is one! - gets passed to setFromRecord(). getRecordById() itself never actually cares what the DAO returned, it just passes it directly to another method. So let's test that: "that whatever is returned from userDAO.getUserByID() is passed to setFromRecord()" (but let's ignore any logic in setFromRecord()). Here we mock a method within the very object we're testing:

public void function setup(){    
    variables.mockedUserDao = variables.mockbox.createMock("testapp.users.UserDAO");
    variables.mockedAuditLog = variables.mockbox.createEmptyMock("testapp.loggers.AuditLog");

    variables.userService = new testapp.users.UserService(
        userDAO = variables.mockedUserDao,
        auditLog = variables.mockedAuditLog
    );
    variables.mockbox.prepareMock(variables.userService);
}

public void function testGetUserById_validId(){
    var mockedGetUserByIdResult = variables.mockbox.querySim("MOCKED_COLUMN#CRLF#MOCKED_VALUE");
    variables.mockedUserDao.$("getUserById", mockedGetUserByIdResult);

    var mockedUserObject = variables.mockbox.createEmptyMock("testapp.users.User");
    variables.userService.$("setFromRecord", mockedUserObject);

    var testId = 17; // I generally pick prime numbers, and do not reuse any within a give CFC.
    var result = variables.userService.getUserById(testId);

    assertTrue(variables.mockedUserDao.$once("getUserById"), "DAO's getUserById() should have been called");
    assertEquals(
        [testId],
        variables.mockedUserDao.$callLog().getUserById[1],
        "Incorrect data passed to setFromRecord()"
    );

    assertTrue(variables.userService.$once("setFromRecord"), "setFromRecord() should have been called");
    assertEquals(
        [mockedGetUserByIdResult],
        variables.userService.$callLog().setFromRecord[1],
        "Incorrect data passed to setFromRecord()"
    );

    assertEquals(
        mockedUserObject,
        result,
        "User returned from getUserById() was not the same as user returned from setFromRecord()"
    );
}

There's a fair bit to look at here:

1. prepareMock(). This takes an already existing object and decorates it with all MockBox's trickery. This allows us to mock methods (etc) within that object. One rule of thumb here... only mock methods that the method yer testing uses... don't mock the method itself, otherwise you might end up just testing whether MockBox works. This sounds obvious, but I have seen a lot of situations in which the test mocks the very method being tested.
2. querySim(). It takes a string as an input, where the first line is a comma-separated list of column names, and subsequent lines are pipe separated lists of column values. Where a "line" is defined by being separated by an end-of-line construct (it might just be a CR, it might just be a LF, but I always just set a variable CRLF:
   CRLF = chr(13) & chr(10);
3. $once(). MockBox decorates a mocked object with a bunch of helper methods, like $once(), $never(), $count() etc. $once() does what it suggests: returns true if the specified method was called once (as opposed to zero times, or two or more times).
4. $callLog(). This is more complicated. It returns a struct containing a key for each method of the object that's been mocked, which itself is an array of each call to said method. And the value of the array is the arguments passed to that specific instance of the call to that method. Huh? Like this:
   
   

   Struct
   setFromRecord	Array 1 Scope Arguments 1 1 Query Execution Time: 0 ms  Record Count: 1  Cached: No  Lazy: No  MOCKED_COLUMN 1 MOCKED_VALUE

   
   
   Had I called other mocked methods of UserService, I'd see them in there as well.

So after all that, we have a test that is testing the following:

• getUserById() is called;
• whichever ID is passed to getUserId() is passed to its equivalent DAO method;
• given the DAO returns a record...
• ... that same record is passed to setFromRecord();
• setFromRecord() returns a User object;
• and that very User object is returned by getUserId().

This covers the "happy path" through getUserId().

What about the "unhappy path" wherein no valid user comes back from the DAO call? Easy!

public void function testGetUserById_invalidId(){
    variables.mockedUserDao.$("getUserById",queryNew("")); // NB: that's zero records

    expectException("InvalidUserException");
    variables.userService.getUserById(19);
}

There's nowt new here. We're mocking the return from the DAO's getUserById() method to just return an empty recordset this time. We don't care what the recordset contains - hence just using queryNew() - because no logic within the method we're testing (on this unhappy path) cares what the returned query contains. Simply that it contains no records is the logic we're testing; and what we need to test is that it throws the correct exception. There's a tangential case for testing the detail of the exception - to test the ID it reports matches the ID passed in - but I see stuff like that as being UI fluff which the computer doesn't care about, and humans don't read (being realistic), so [gallic shrug].

So there's a coupla tests using MockBox to mock dependencies out of the way so we can focus on testing the actual logic of the unit, and nothing more. I could cover more about MockBox, but the Ortus docs are pretty to-the-point for Mockbox, so they're easy to understand. And I'm now on my fifth pint and I'm drunk, and still need to go back to the B&B where there's internet (I'm offline and typing this into Evernote), so I can reformat all this as a blog article [which, ultimately, I am currently doing at the airport the following day].

And eat the pizza the man next door must have ready for me by now...

--
Adam

PS: this is actually a good social pub once the footy bods go home. People are just drinking, yapping and playing cards; and they have Alabama 3 playing (CD, not live!) in the background (at "background" volumes), who are my favourite band from back in my Brixton days. You might know of them if you've watched The Sopranos.