another interesting TDD episode

The classic TDD cycle says that you should start with a test for new functionality and see it fail.
There is real value in not skipping this step; not jumping straight to writing code to try to make it pass.
  • One reason is improving the diagnostic. Without care and attention diagnostics are unlikely to diagnose much.
  • A second reason is to be sure the test is actually running! Suppose for example, you're using JUnit and you forget its @Test annotation? Or the public specifier?
  • A third reason is because sometimes, as we saw last time, you get an unexpected green! Here's another nice example of exactly this which happened to me during a cyber-dojo demo today.
I was doing the fizz-buzz practice in C.
I started by writing my first test, like this:
static void assert_fizz_buzz(const char * expected, int n)
{
    char actual[16];
    fizz_buzz(actual, sizeof actual, n);
    if (strcmp(expected, actual) != 0)
    {
        printf("fizz_buzz(%d)\n", n);
        printf("expected: \"%s\"\n", expected);
        printf("  actual: \"%s\"\n", actual);
        assert(false);
    }
}

static void numbers_divisible_by_three_are_Fizz(void)
{
    assert_fizz_buzz("Fizz", 3);
}

I made this fail by writing the initial code as follows (the (void)n is to momentarily avoid the "n is unused" warning which my makefile promotes to an error using the -Werror option):
void fizz_buzz(char * result, size_t size, int n)
{
    (void)n;
    strncpy(result, "Hello", size);
}

which gave me the diagnostic:
...: assert_fizz_buzz: Assertion `0' failed.
fizz_buzz(3)
expected: "Fizz"
  actual: "Hello"

I made this pass with the following slime
void fizz_buzz(char * result, size_t size, int n)
{
    if (n == 3)
        strncpy(result, "Fizz", size);
}

Next, I returned to the test and added a test for 6:
static void numbers_divisible_by_three_are_Fizz(void)
{
    assert_fizz_buzz("Fizz", 3);
    assert_fizz_buzz("Fizz", 6);
}

I ran the test, fully expecting it to fail, but it passed!
Can you see the problem?
.
.
.
.
.
.
.
.
.
.
.
.
.
.
.
.
.
.
.
.
.
.
.
.
.
.
The problem is in assert_fizz_buzz which starts like this:
static void assert_fizz_buzz(const char * expected, int n)
{
    char actual[16];
    ...
}

Here's what's happening:
  • assert_fizz_buzz("Fizz", 3) is called
  • char actual[16] is defined
  • fizz_buzz(actual, sizeof actual, 3) is called
  • if (n == 3) is true
  • "Fizz" is strncpy'd into actual
  • fizz_buzz(actual, sizeof actual, 3) returns
  • strcmp says that expected equals actual
  • ...
  • assert_fizz_buzz("Fizz", 6) is called
  • char actual[16] is defined
  • actual exactly overlays its previous location so its first 5 bytes are still 'F','i','z','z','\0'
  • fizz_buzz(actual, sizeof actual, 6) is called
  • if (n == 3) is false
  • fizz_buzz(actual, sizeof actual, 6) returns
  • strcmp says that expected equals actual

My mistake was in the test; actual has automatic storage duration so does not get initialized. It's initial value is indeterminate. The first call to assert_fizz_buzz is accidentally interfering with the second call. Tests should be isolated from each other. I tweaked the test as follows:
static void assert_fizz_buzz(const char * expected, int n)
{
    char actual[16] = { '\0' };
    ...
}

I ran the test again and this time it failed :-)
...: assert_fizz_buzz: Assertion `0' failed.
fizz_buzz(6)
expected: "Fizz"
  actual: ""

I made the test pass:
void fizz_buzz(char * result, size_t size, int n)
{
    if (n % 3 == 0)
        strncpy(result, "Fizz", size);
}

Let's hear it for starting with a test for new functionality and seeing it fail.

No comments:

Post a Comment