An example of unit testing working for me

Some specific feedback I was given regarding my unit testing talk at VTM: iPhone fall conference was that the talk was short on real-world application of unit testing. That statement is definitely true, and it’s unfortunate that I didn’t meet the attendee’s expectations for the talk. I hope to address that deficiency here, by showing you a real bug that I really fixed via unit testing.

The code is in the still-MIA Rehearsals app. I started developing Rehearsals before I was a full believer in test-driven development, so there were places in the app where I could go back and add tests to existing code. This is actually a pretty useful displacement activity – if you get bored of doing the real code, you can switch to writing the tests. This is enough of a change of pace (for me) to provide a welcome distraction, and still makes the product better.

So, in order to understand the problem, I need to understand the requirements. I’m a fan of the user story approach to requirements documentation, and so Rehearsals has the following story written on a 5×3 index card:

A musician can rate tunes in her tunebook, by giving it a number of stars between 1 and 5. Tunes that the musician has not rated have no star value.

As is often the case, there are other stories that build from this one (any such collection is called an “epic”), but I can concentrate on implementing this user story for now. One feature in Rehearsals is an AppleScript interface, so clearly if the musician can rate a tune, she must be able to do it via AppleScript. Glossing over the vagiaries of the Cocoa Scripting mechanism, I define a property scriptRating on the tune that had the following KVC-compliant accessors:

- (NSNumber *)scriptRating {
    if (!self.rating) {
        return [NSNumber numberWithInteger: 0];
    }
    return self.rating;
}

- (void)setScriptRating: (NSNumber *)newRating {
    NSInteger val = [newRating integerValue];
    if (val < 0 || val > 5) {
        //out of range, tell AppleScript
        NSScriptCommand *cmd = [NSScriptCommand currentCommand];
       [cmd setScriptErrorNumber: TORatingOutOfRangeError];
        //note that AppleScript is never localised
        [cmd setScriptErrorString: @"Rating must be between 0 and 5."];
    }
    else {
        self.rating = newRating;
    }
}

(No prizes for having already found the bug – it’s really lame). So testing that the getter works is ultra-simple: I can just -setRating: and verify that -scriptRating returns the same value. You may think this is not worth doing, but as this functionality is using indirect access via a different property I want to make sure I never break the connection. I decided that a single test would be sufficient (tune is an unconfigured tune object created in -setUp):

- (void)testTuneRatingIsSeenInAppleScript {
    tune.rating = [NSNumber numberWithInteger: 3];
    STAssertEquals([tune.scriptRating integerValue], 3,
                   @"Tune rating should be visible to AppleScript");
}

Simples. Now how do I test the setter? Well, of course I can just set the value and see whether it sticks, that’s the easy bit. But there’s also this bit about being able to get an error into AppleScript if the user tries to set an out of range error. That seems like really useful functionality, because otherwise the app would accept the wrong value and give up later when the persistent store gets saved. So it’d be useful to have a fake script command object that lets me see whether I’m setting an error on it. That’s easy to do:

@interface FakeScriptCommand : NSScriptCommand
{
    NSString *scriptErrorString;
    NSInteger scriptErrorNumber;
}
@property (nonatomic, copy) NSString *scriptErrorString;
@property (nonatomic, assign) NSInteger scriptErrorNumber;
@end

@implementation FakeScriptCommand
@synthesize scriptErrorString;
@synthesize scriptErrorNumber;
- (void) dealloc {...}
@end

OK, but how do I use this object in my test? The tune class sets an error on +[NSScriptCommand currentScriptCommand], which is probably returning an object defined in a static variable in the Foundation framework. I can’t provide a setter +setCurrentScriptCommand:, because I can’t get to the innards of Foundation to set the variable. I could change or swizzle the implementation of +currentScriptCommand in a category, but that has the potential to break other tests if they’re not expecting me to have broken Foundation.

So the solution I went for is to insert a “fulcrum” if you will: a point where the code changes from asking for a script command to being told about a script command:

- (void)setScriptRating: (NSNumber *)newRating {
    [self setRating: newRating fromScriptCommand: [NSScriptCommand currentScriptCommand]];
}

- (void)setRating: (NSNumber *)newRating fromScriptCommand: (NSScriptCommand *)cmd {
    NSInteger val = [newRating integerValue];
    if (val < 0 || val > 5) {
       [cmd setScriptErrorNumber: TORatingOutOfRangeError];
        //note that AppleScript is never localised
        [cmd setScriptErrorString: @"Rating must be between 0 and 5."];
    }
    else {
        self.rating = newRating;
    }
}

This is the Extract Method refactoring pattern. Now it’s possible to test -setRating:fromScriptCommand: without depending on the environment, and be fairly confident that if that works properly, -setScriptRating: should work properly too. I’ll do it:

- (void)testAppleScriptGetsErrorsWhenRatingSetTooLow {
    FakeScriptCommand *cmd = [[FakeScriptCommand alloc] init];
    [tune setRating: [NSNumber numberWithInteger: 0]
  fromScriptCommand: cmd];
    STAssertEquals([cmd scriptErrorNumber], TIRatingOutOfRangeError,
                   @"Should get a rating out of range error");
    STAssertNotNil([cmd scriptErrorString], @"Should report error to the user");
}

Oh-oh, my test failed. Why’s that? I’m sure you’ve spotted it now: the user is supposed to be restricted to values between 1 and 5, but the method is actually testing for the range 0 to 5. I could fix that quickly by changing the value in the test, but I’ll take this opportunity to remove the magic numbers from the code too (though you’ll notice I haven’t done that with the error string yet, I still need to do some more refactoring):

- (void)setRating: (NSNumber *)newRating fromScriptCommand: (NSScriptCommand *)command {
    if (![self validateValue: &newRating forKey: @"rating" error: NULL]) {
        [command setScriptErrorNumber: TIRatingOutOfRangeError];
        //note that AppleScript is never localised
        [command setScriptErrorString: @"Rating must be a number between 1 and 5."];
    }
    else {
        self.rating = newRating;
    }
}

OK, so now my test passes. Great. But that’s not what’s important: what’s important is that I found and fixed a bug by thinking about how my code is used. And that’s what unit testing is for. Some people will say they get a bad taste in their mouths when they see that I redesigned the method just so that it could be used in a test framework. To those people I will repeat the following: I found and fixed a bug.