Quality Coding
Shares

How a TDD Mistake Revealed an Error in My Design

Shares

My TDD has improved since I first started in 2001. But even today, I make mistakes. The trick is to learn to recognize TDD mistakes. Then, learn to “listen” to them: what is it trying to tell me about the design?

Follow along as I recount the latest steps in Marvel Browser, the iOS TDD sample app. Can you spot the errors before I point them out?

[This post is part of the series TDD Sample App: The Complete Collection …So Far]

Adding authentication… poorly

In the last screencast, I showed how to begin converting the Request Model into a URL query string. I’ve added the other parameters, and am now ready to add the Marvel Comics API authentication.

Marvel’s authentication requires us to pass the following parameters with each request:

  • ts: a timestamp
  • apikey: a public key
  • hash: an MD5 hash of the timestamp, private key, and public key

We get a string with these parameters by calling [QCOMarvelAuthentication URLParameters].

So now I’m picturing the implementation in my head. We can take the URL string generated so far, and append the authentication parameters.

How to test this? QCOMarvelAuthentication will take care of generating the parameters. So rather than test the parameter values, what if I just test for their names? Here’s a test that specifies an “allOfIn” matcher for the URL. Every matcher in the “allOfIn” array argument must be satisfied. I’m using “hasQuery” matchers to test for query names and values. I don’t care about the values, so they all have an “anything” matcher:

- (void)testFetchCharacters_ShouldIncludeAuthentication
{
    QCOFetchCharactersRequestModel *requestModel = [self dummyRequestModel];

    [sut fetchCharactersWithRequestModel:requestModel];

    [verify(mockSession) dataTaskWithURL:allOfIn(@[
                                            hasQuery(@"ts", anything()),
                                            hasQuery(@"apikey", anything()),
                                            hasQuery(@"hash", anything()),
                                        ])
                       completionHandler:anything()];
}

That test fails. Now we’ll get it to pass, by calling QCOMarvelAuthentication and appending it to the URL string:

urlString = [urlString stringByAppendingString:[QCOMarvelAuthentication URLParameters]];

Before you read any further… can you spot the TDD mistake?

My TDD mistake

I made a failing test, then I made it pass. What did I do wrong?

I didn’t write the simplest code that would pass.

Let’s look at Uncle Bob’s Three Laws of TDD:

  1. You are not allowed to write any production code unless it is to make a failing unit test pass.
  2. You are not allowed to write any more of a unit test than is sufficient to fail; and compilation failures are failures.
  3. You are not allowed to write any more production code than is sufficient to pass the one failing unit test.

My implementation breaks the Third Rule.

So now let’s write the simplest code that passes the test. Ready?

urlString = [urlString stringByAppendingString:@"&ts=FOO&apikey=BAR&hash=BAZ"];

This of course isn’t helpful.

That in turn leads me to question the test, and the assumptions I made. I’m trying to “listen to the test.” What is it telling me about my design? What assumptions led to this test?

Before you read any further… can you spot the design mistake?

My design mistake

I thought QCOFetchCharactersMarvelService should call [QCOMarvelAuthentication URLParameters].

That’s it. That is the mistake. …But how can that be wrong?

“Mistake” may be overstating it. There’s a weakness in this design that may not bother most people. But it bothers me.

Class methods lock you to a specific class. We feel the tension most strongly with singletons. (If singletons don’t make you nervous, you probably need to write more unit tests.) But that tension applies to any class method. It even applies to alloc.

I’m not saying one should never call another class’s class method. But I am saying that you should recognize the limitations of doing so.

In this example, it makes it hard to write a good unit test. What I really want is to supply an alternate implementation, so that I have full control over the test. I want Dependency Injection. That’s easy when an object is passed in. But what do we do when we need to call a class?

That really depends on the purpose of the class method. For authentication, I want to get a new string every time I need authentication parameters.

In the past, I might have made a factory. But these days, why not use a block?

Adding authentication… with an injected block

So let’s start over. This time, we’ll use Dependency Injection with a block. As usual, we prefer constructor injection. We’ll add a block argument to the initializer.

Here’s the new failing test:

- (void)testFetchCharacters_ShouldIncludeGeneratedAuthenticationParameters
{
    QCOFetchCharactersMarvelService *sutWithAuthParameters =
            [[QCOFetchCharactersMarvelService alloc] initWithSession:mockSession
                                             authParametersGenerator:^NSString * {
                                                 return @"&FOO=BAR";
                                             }];
    QCOFetchCharactersRequestModel *requestModel = [self dummyRequestModel];

    [sutWithAuthParameters fetchCharactersWithRequestModel:requestModel];

    [verify(mockSession) dataTaskWithURL:hasQuery(@"FOO", @"BAR")
                       completionHandler:anything()];
}

Note that I don’t reuse the sut (System Under Test) from the test fixture. Instead, I create a new one. I could have defined the authParametersGenerator block in the test’s setUp. But that would have made it hard to read the test, because it would have hidden where FOO and BAR come from.

So now, I’m not checking for the authentication parameters. I’m simply checking that the block is wired up correctly. Basically, I’ve deferred the problem out of my way. It’ll be up to the application configuration to provide a block that calls QCOMarvelAuthentication. (Later, we’ll be able to ensure this by writing an acceptance test.)

The production code to pass this test is straightforward. If you want to peek, here’s my commit.

Conclusion

On my first attempt, I had trouble writing a good test. Then I made a TDD mistake: I broke the Third Rule by implementing more production code than was required by the test.

Whenever you have trouble writing a test, ask yourself if there’s something wrong with the design. Listen to your tests. It’s not just the test results that are providing feedback — it’s the ease of the test code itself. This pressure leading to cleaner designs is one of the biggest benefits of TDD.

Watch out for class methods. They make tests harder to write, by locking in specific dependencies. See if there’s a way to inject the dependencies instead.

Can you find any places in your unit-tested production code that exceed the requirements of the test code? Are you missing tests, or was it too hard to write them? Leave a comment below.

[This post is part of the series TDD Sample App: The Complete Collection …So Far]

About the Author Jon Reid

Jon is a coach and consultant on iOS Clean Code (Test Driven Development, unit testing, refactoring, design). He’s been practicing TDD since 2001. You can learn more about his background, or see what services he can bring to your organization.

follow me on:
Disclosure: The book links below are affiliate links. If you buy anything, I earn a commission, at no extra cost to you.

Leave a Comment:

1 comment
Roger Wernersson says last year

I think you example would have been easier to follow if you used a simple made-up example instead of actual code. I find the test hard to follow. The names are too long. Maybe it’s just me.

Reply
Add Your Reply