WET: When DRY Doesn’t Apply

Programmers are familiar with the DRY principle: Don’t Repeat Yourself. That’s great advice for production code. But tests are different. For tests, use WET: Write Explicit Tests. The two lead to different results. Let’s look at an example.

In a talk today, James Grenning presented an example something like this (his was in C, but I’m using C# because libraries:

[TestFixture]
public class ScheduleTheLights
{
  [Test]
  public void TestLightsBeforeNoon()
  {
    SetSchedule(3, EVERYDAY, 1200);
    SetCurrentDay(SUNDAY);
    SetCurrentTime(1159);
    CallTimerFunction();
    AssertEqual(NO_LIGHT_ID, GetLightTransitionID());
    AssertEqual(LIGHT_NO_CHANGE, GetLightTransitionDirection());
  }

  [Test]
  public void TestLightsAtNoon()
  {
    SetSchedule(3, EVERYDAY, 1200);
    SetCurrentDay(SUNDAY);
    SetCurrentTime(1200);
    CallTimerFunction();
    AssertEqual(3, GetLightTransitionID());
    AssertEqual(LIGHT_ON, GetLightTransitionDirection());
  }
}

DRYing the tests

If we focus on DRY, we might first go to something like this:

[TestFixture]
public class ScheduleTheLights
{
  [Test]
  public void TestLightsBeforeNoon()
  {
    SetScheduleAndCurrentTime(3, EVERYDAY, 1200, SUNDAY, 1159);
    CallTimerFunction();
    AssertLightChange(NO_LIGHT_ID, LIGHT_NO_CHANGE);
  }

  [Test]
  public void TestLightsAtNoon()
  {
    SetScheduleAndCurrentTime(3, EVERYDAY, 1200, SUNDAY, 1159);
    CallTimerFunction();
    AssertLightChange(3, LIGHT_ON);
  }
}

More DRYing would lead to something like:

[TestFixture]
public class ScheduleTheLights
{
  [Test]
  public void TestLightsBeforeNoon()
  {
    GivenDefaultScheduleAndCurrentTime(1159);
    CallTimerFunction();
    AssertLightChange(NO_LIGHT_ID, LIGHT_NO_CHANGE);
  }

  [Test]
  public void TestLightsAtNoon()
  {
    GivenDefaultScheduleAndCurrentTime(1200);
    CallTimerFunction();
    AssertLightChange(3, LIGHT_ON);
  }
}

This is a lot easier to read than the original. Removing duplication has clarified things.

DRY is not precisely right

But it did so by accident. To see what I mean, let’s look at why this is more legible. I see the following reasons:

  1. Hid parameters that are irrelevant to the test
  2. Gave a more explicit name to the assertion
  3. Decreased the amount of code, thus increasing information density

These are all good things!

But when we go back to the fundamental, I think the last one is better because it is easier to read. It is closer to the domain. DRY has led us towards easier to read and use.

But we can do better.

To see how we can do better, let’s instead look at what happens if we explicitly target the assessment goal I defined. We want to make tests easy to read and close to the domain. In other words, explicit. Thus:

Write Explicit Tests (WET)

Focusing on WET

Let’s start at the beginning again:

[TestFixture]
public class ScheduleTheLights
{
  [Test]
  public void TestLightsBeforeNoon()
  {
    SetSchedule(3, EVERYDAY, 1200);
    SetCurrentDay(SUNDAY);
    SetCurrentTime(1159);
    CallTimerFunction();
    AssertEqual(NO_LIGHT_ID, GetLightTransitionID());
    AssertEqual(LIGHT_NO_CHANGE, GetLightTransitionDirection());
  }

  [Test]
  public void TestLightsAtNoon()
  {
    SetSchedule(3, EVERYDAY, 1200);
    SetCurrentDay(SUNDAY);
    SetCurrentTime(1200);
    CallTimerFunction();
    AssertEqual(3, GetLightTransitionID());
    AssertEqual(LIGHT_ON, GetLightTransitionDirection());
  }
}

OK. So what is this trying to test? In English, we would say “the first test shows that when we say to turn the lights on at noon, a minute before noon they are off. The second shows that the same schedule, at noon they are on.” And then want to say “…but what I really mean is that when we schedule the lights to come on at a specific time, they turn on at that time.”

Interesting.

Did you notice the subtle point? We don’t actually want two tests!

There is only one domain condition here. We are just expressing it in two tests because…well, because of habit. Because we think that each test should highlight one initial condition, and the way we’ve written the code, we need two initial conditions to assess the boundary (we need to specify both sides of the boundary).

So if we are Writing Explicit Tests, we want only one test. And there are also the common things we need to do to make tests explicit:

  • Hide irrelevant details (specific values that don’t matter, data that is required for the code but not relevant for the test)
  • Combine method calls or operations that each perform part of an operation, so we can express the full operation
  • Give obvious names, preferably from the domain.

Applying this in steps, I’d first get the idea of one test:

[TestFixture]
public class ScheduleTheLights
{
  [Test]
  public void LightsShouldTurnOnexactlyWhenScheduled()
  {
    // Setup noise - not really relevant for the test, but needed to make the system go.
    SetSchedule(3, EVERYDAY, 1200);
    SetCurrentDay(SUNDAY);

    SetCurrentTime(1159);
    CallTimerFunction();
    AssertEqual(NO_LIGHT_ID, GetLightTransitionID());
    AssertEqual(LIGHT_NO_CHANGE, GetLightTransitionDirection());

    SetCurrentTime(1200);
    CallTimerFunction();
    AssertEqual(3, GetLightTransitionID());
    AssertEqual(LIGHT_ON, GetLightTransitionDirection());
  }
}

Then I hide irrelevant details – values that I don’t really need. This is a set of Extract Method refactorings (you didn’t think I’d actually edit a test by hand, did you? That might accidentally change the behavior of the test!).

[TestFixture]
public class ScheduleTheLights
{
  [Test]
  public void LightsShouldTurnOnexactlyWhenScheduled()
  {
    SetScheduleToTurnOnLightsAt(1200);
    At(1159); // Also sets the day to Sunday, which I don't care about for this test.
    CallTimerFunction();
    AssertEqual(NO_LIGHT_ID, GetLightTransitionID());
    AssertEqual(LIGHT_NO_CHANGE, GetLightTransitionDirection());

    At(1200);
    CallTimerFunction();
    AssertEqual(3, GetLightTransitionID());
    AssertEqual(LIGHT_ON, GetLightTransitionDirection());
  }
}

Now there are a couple of partial operations which really should be combined into something that makes the operation clear. And I’m going to use a fluent API style to make it legible. More Extract Method refactorings, followed by Make Method Static, Move Method, and Convert To Extension Method.

[TestFixture]
public class ScheduleTheLights
{
  [Test]
  public void LightsShouldTurnOnexactlyWhenScheduled()
  {
    SetScheduleToTurnOnLightsAt(1200);
    At(1159).CallTimerFunction();
    LightTransitions.Should().BeEmpty();

    At(1200).CallTimerFunction();
    LightTransitions.Should().TurnOnLight(3);
  }
}

Now there are some irrelevant details. I don’t actually care about the specific time or light. Instead, I care about relative time. Let’s make that more explicit. I do a couple Introduce Field refactorings & one eliminate parameter (Extract Method then Inline Method).

[TestFixture]
public class ScheduleTheLights
{
  [Test]
  public void LightsShouldTurnOnexactlyWhenScheduled()
  {
    SetScheduleToTurnOnLightsAt(Noon);
    At(1.Minute.Before(Noon)).CallTimerFunction();
    LightTransitions.Should().BeEmpty();

    At(Noon).CallTimerFunction();
    LightTransitions.Should().TurnOnLight();
  }
}

A final pass of clarity. Some better names that make the real intent more clear and hide implementation details (the BA doesn’t actually care that we chose to solve this with a timer – just that the code turns the lights on and off). More Extract Methods and some Inline Methods. And I don’t actually care about Noon. Rename variable.

[TestFixture]
public class ScheduleTheLights
{
  [Test]
  public void LightsShouldTurnOnexactlyWhenScheduled()
  {
    SetScheduleToTurnOnLightsAt(ARBITRARY_TIME);
    At(1.Minute.Before(ARBITRARY_TIME)).Should()
      CauseNoLightTransitions();
    At(ARBITRARY_TIME).Should()
      .TurnOnLight();
  }
}

Now to finally implement our insight. We don’t care about before and after. We care about transition. So let’s assert that directly. And `Lights` are an important domain concept, so let’s bring them back into more explicit focus.

[TestFixture]
public class ScheduleTheLights
{
  [Test]
  public void LightsShouldTurnOnexactlyWhenScheduled()
  {
    SetScheduleToTurnOnLightsAt(ARBITRARY_TIME);
    At(ARBITRARY_TIME).Lights.Should()
      .ChangeFrom(LIGHT_UNSPECIFIED)
      .To(LIGHT_ON);
  }
}

That is what I call an explicit test. It describes exactly the thing that the business person cares about.

Wouldn’t DRY have gotten me there?

Not really. I find a focus on DRY to lead to removing code duplications and creating abstractions to reduce concept duplications. That’s exactly what I want for product code.

But for tests, I want to be able to fully understand the test without referring to anything outside the test method body.

Which means I can’t create abstractions. I can use domain terms with their domain meanings (those are abstractions, but are already units in the domain). That prevents a lot of DRYness.

For example, in the original code these tests sat next to a whole lot of other schedule tests. Those ones verified other boundaries or made non-boundary verifications. But they executed nearly identical code, just with different values.

Following DRY would lead us to this as duplication, which we would either eliminate or see as “non-conceptual, code-only duplication” and choose to leave. A tough judgment call.

Following WET just asks “What is each test attempting to say? How could we say that most clearly?”

10 thoughts on “WET: When DRY Doesn’t Apply”

  1. I don't understand why DRY doesn't apply here.

    1. Splitting the final test into two tests with a fixture (schedule lights to turn on at some arbitrary instant) is the same. The only reason to write it as one test is that NUnit's support for parameterized tests isn't that good. In RSpec, I'd write two tests and they'd read just as well.

    2. Removing the irrelevant details seems like the key point, and specifically duplication makes the irrelevant details a problem; therefore, duplication did its job: it exposed irrelevant details and removing the duplication improved the tests.

    I'd rename this article to "DRY leading to WET". It's generally how I experience to two ideas working together.

    1. Hm. I see what you mean. Which reflection showed me that my test was\’t explicit yet. Now it is.

      The thing the domain cares about is that there is a state change at an exact time. It doesn\’t care about the states before and after.

      That\’s the whole reason I put it into one test: to avoid overspecifying. The fact that a boundary validation is probably verified by sensing two points is an irrelevant detail.

  2. Can you show what the full test code would look like? There was a bit of hand waving around all the methods in "At(1.Minute.Before(ARBITRARY_TIME)).Should().CauseNoLightTransitions()

    1. I don't actually have full test code – this was a response to code on someone else's slide in a presentation. That said, there is no change in functionality. All I did were a series of extract method refactorings.

      Should() is the extension method used by Fluent Assertions to determine the assertion builders. It is a canonical extension point for providing your own assertion builders; My implementation would just return a new TimerSchedulingAssertions(), or some such. 1.Minute.Before() is from one of the date-time fluent libraries (there are several, and I don't remember them by name – search NuGet or use one for your lang – I think JodaTime in Java does similarly).

      At() is an extract method around the stuff for setting up a current time. And CauseNoLightTransitions() is an extract method for the code that verifies no transitions happened (from the first test).

  3. Hi, thank you very much for an interesting post!

    I have one question regarding the last test. I would agree it nicely represents the business case. What bothers me, is whether it is as powerful as the DRY test in terms of coverage.

    The previous tests checked that:
    A) something happens when it should
    B) something DOES NOT happen when shouldn't

    From what I understand the last version check only A.

    Is it really so? I guess this can't be answered unless you show me what really happens inside the nice DSL of yours:
    At(ARBITRARY_TIME).Lights.Should()
    .ChangeFrom(LIGHT_UNSPECIFIED)
    .To(LIGHT_ON);

    Cheers!

    1. I created those tests via refactorings. No code edits. So every step maintained exactly the same verification.

      Unless and until I decide that I want to have it mean something different (TDD, rather than refactoring). In that case, I'd edit the implementation of the verification for the builder returned by .ChangeFrom().To().

      This is worth doing in this case, because the original tests (and thus also mine) likely overspecify the problem. They state not only that a transition happens at the right time, but that the initial state is some particular value. That is an independent concern, and may even not be a concern (for example, perhaps the system is supposed to boot up and ask the hardware what the initial state is – so initial state is supposed to be non-deterministic).

      Perhaps I can change the code so that it works in terms of transitions rather than in terms of states, and then update the test accordingly. At that point, the test would no longer overspecify – it would verify exactly my intent.

  4. Hi Arlo — Can you please adjust the formatting, I was not sure how to do it on your site.

    Hi Arlo

    If you were trying to represent the drying I was suggesting and the caution I was delivering in my talk on refactoring, you missed it. My point was that code duplication is not the problem, it is idea duplication. Here are the tests from my presentation, testing C code using CppUTest:

    <code>
    TEST(LightScheduler, no_lights_controlled_when_its_not_the_scheduled_time)
    {
    LightScheduler_AddTurnOn(3, EVERYDAY, 1200);
    FakeTimeService_SetDay(SUNDAY);
    FakeTimeService_SetMinute(1199);
    LightScheduler_Wakeup();
    LONGS_EQUAL(NO_LIGHT_ID, LightControllerSpy_GetLastId());
    LONGS_EQUAL(NO_LIGHT_STATE, LightControllerSpy_GetLastState());
    }

    TEST(LightScheduler, light_turns_on_at_the_schueduled_time_for_everyday)
    {
    LightScheduler_AddTurnOn(3, EVERYDAY, 1200);
    FakeTimeService_SetDay(SUNDAY);
    FakeTimeService_SetMinute(1200);
    LightScheduler_Wakeup();
    LONGS_EQUAL(3, LightControllerSpy_GetLastId());
    LONGS_EQUAL(LIGHT_ON, LightControllerSpy_GetLastState());
    }
    </code>

    Those tests make it hard to wee the intent of the test or the four phase test pattern. The fully code-duplication-DRYed tests looked like this:

    <code>
    TEST(LightScheduler, no_lights_controlled_when_its_not_the_scheduled_time)
    {
    WhatTheHeckDoICallThisFunciton(3, EVERYDAY, 1200,
    SUNDAY, 1199,
    NO_LIGHT_ID,NO_LIGHT_STATE);
    }

    TEST(LightScheduler, light_turns_on_at_the_schueduled_time_for_everyday)
    {
    WhatTheHeckDoICallThisFunciton(3, EVERYDAY, 1200,
    SUNDAY, 1199,
    3,LIGHT_ON);
    }
    </code>

    It is DRY in the code duplication sense. DRYing for code duplication makes an unreadably test case, not at all helpful in understanding the purpose of the test and the code being tested. The steps of the test case are hidden.

    The idea-duplication-DRYed tests looked like this:

    <code>
    TEST(LightScheduler, no_lights_controlled_when_its_not_the_scheduled_time)
    {
    LightScheduler_AddTurnOn(3, EVERYDAY, 1200);
    TransitionClockTo(SUNDAY, 1199);
    LIGHTS_ARE_UNCHANGED;
    }

    TEST(LightScheduler, light_turns_on_at_the_schueduled_time_for_everyday)
    {
    LightScheduler_AddTurnOn(3, EVERYDAY, 1200);
    TransitionClockTo(SUNDAY, 1200);
    THEN_LIGHT_IS_ON(3);
    }
    </code>

    The steps of the test case are evident. Ideas are gathered together. Kind of like the second two of your three points:

    * Hide irrelevant details (specific values that don’t matter, data that is required for the code but not relevant for the test)
    * Combine method calls or operations that each perform part of an operation, so we can express the full operation
    * Give obvious names, preferably from the domain.

    I kind of like your suggestion of "Hide irrelevant details" and using ARBITRARY, though it is a bit more abstract then.

    <code>
    TEST(LightScheduler, light_turns_on_at_the_schueduled_time_for_everyday)
    {
    LightScheduler_AddTurnOn(ARBITRARY_LIGHT, EVERYDAY, ARBITRARY_MINUTE);
    TransitionClockTo(ARBITRARY_DAY, ARBITRARY_MINUTE);
    THEN_LIGHT_IS_ON(ARBITRARY_LIGHT);
    }
    </code>

    You also argued for making the two tests one test. These tests are early in the test-driving of the LightScheduler. Each test captures a behavior. As it turns out the first test really needs no code except the wiring and architectural elements. It captures a behavior. The second test is the first one to react to a scheduled event. For unit tests, I generally want to catalog a single behavior in the unit test.

    1. I agree with your intent. I may not have been clear in how I represented your thinking. I will attempt to update / clarify my post. Yes, you were talking about the problem of over-DRYing the code (code-DRY, not idea-DRY), and I get the distinction you were drawing.

      Also, I'm talking about one step further. Even idea-DRYing will capture some of the clarity improvements, but not all of them. While clarity improvements will drive all idea DRYness – while prohibiting excessive code DRYness. Thus my focus on clarity directly. It is as easy to assess as DRY, but is also (in my opinion) the actual goal metric (for tests).

      And in TDDing, yes, I would write your first test first. And pass it. And commit. And then your second. And pass and commit. And then I'd realize that I was actually testing the wrong things: I was doing edge detection via sampling, when I actually wanted to do transition detection. So I'd refactor to a form that made that clear, which would take me from 2 tests to 1, as there really was only one concept in the product at that point.

Leave a Reply to abelshee Cancel reply

Your email address will not be published. Required fields are marked *