How does TDD behave when the tested unit needs to be expanded?

Question

I want to write some invoicing logic, and I start coding it, using TDD.

The following example is silly, but I'm confident it represents well the everyday dilemma I'm facing

function createTestInvoice(client) {
  return Invoice.new(
    {
      client: client, rows: [
        {item: 'apple', quantity: 1, unit_price_ 1}
        {item: 'banana', quantity: 2, unit_price: 2}
      ]
    }
  )
}

al = Client.new({name: 'Al', can_buy_apples: true)

assert(
  createTestInvoice(al).total
).to(equal(5))

john = Client.new({name: 'John', can_buy_apples: false)

assert(
  createTestInvoice(al).total
).to(equal(4))

When I start with the implementation I realise that just the following

is not enough, since I need some kind of InvoiceRowFactory that decides how and if to allow the row to be stored.

At this point the logic I was testing in this unit would be testing some logic that is included in the InvoiceRowFactory. A test for it would be:

/* remember: John cannot buy apples */
assert(InvoiceRowFactory.call(john, {item: 'apple', quantity: 1, unit_price_ 1}).to(be(null))

How to proceed now? keep the tests in the specs of Invoice, or move them to a brand new test file of InvoiceRowFactory ? Both cases feel wrong to be, because

• if I keep the tests here, I'll be testing here something that belongs to another unit, that can and should be tested separately
• if I move the tests, I'll be stubbing the dependency. At that point, in the future, I realise this code is horrible OOP, and want to replace InvoiceRowFactory with InvoiceRow. So a refactorer would love not to break anything while he's refactoring, keeping all the tests all green, but how? He can't, because my test of Invoice is too coupled with the InvoiceRowFactory stub

I just cannot reach the point where I can look at some code and make some changes keeping confident I'll not break anything, because I write such small units, that the only way to improve them is change the way they interact.

Thank you

Answer 1

If the tests feel wrong in either place, then you might be discovering an architectural problem with your code. Fixing the architectural problem will likely fix your test dilemma as well. Focus on this negative use case and write a test for it. Write the code you would like other code authors to use in this case. Should it throw an exception? Return null? In using TDD you get to discover these things as you write your tests. Since no code exists for this use case, you get to invent how the code looks.

So to answer your question, there is nothing wrong with your tests. By starting with TDD you discovered a flaw in your design. This is an opportunity to correct that design flaw by writing a test for this use case and defining how it should behave. This test will inform other code authors about what they should expect, and what error handling is necessary.

Answer 2

The most direct implementation for a function like total does probably not require another component or class, that looks a little bit overdesigned to me at a first glance. But for the sake of this example, let us assume for a moment total would be something complex enough that is justifies the creation of such a second class.

I would recommend to go through the following steps:

• first, implement function total(){return 0;} - should give you a failing test.

• second, implement the summation in the constructor, store the result in some member variable, and return that member's content from total() - even if you think this is not the ideal design, and makes the constructor too big. Now, the test is "green".

• third, and that is step which is pretty important in TDD: refactor. Now, you may decide to move parts of the logic in some separat class InvoiceRowFactory, and do the summation not in the constructor any more, but in total() (by utilizing InvoiceRowFactory).

It should be clear that for refactoring without breaking the existing code, one has to keep the tests inside the spec of Invoice. From this point of view, InvoiceRowFactory is not much more than an implementation detail of Invoice, so it does not necessarily require any tests on its own.

The former approach is a "top down" strategy of successive refinement. Alternatively, one can also decide to use a "bottom up" approach. In case one starts to foresee the need for something like an InvoiceRowFactory, maybe halfway in the middle of implementing total(), it is possible to postpone the creation of total(). For this, I would deactivate all tests for it for a moment, maybe comment out the half-baked code for total() (if that is necessary to get the code into a compilable state) and restart with InvoiceRowFactory first. In TDD, that means, of course, writing tests first, then implement InvoiceRowFactory.

When this class is complete, one can activate the tests for total() and the already written code for that function again, and finish it's implementation. This will end up with tests for both components, and InvoiceRowFactory more than just "an implementation detail", it has become now an independently reusable component.

Which of these route to prefer is often a judgement call, it depends on the specific situation, the complexity of the required components and also on your personal skills, experience and taste.

Answer 3

How to proceed now?

A common pattern is to create new tests for your new API, and then refactor the old implementation to use the new API.

So there is a period of time where there are "redundant" tests. For "programmer tests", meaning the tests that we re-run after each refactor to prove that we are still green, redundant tests aren't typically a big deal - tests without I/O are generally pretty fast to run anyway, and if the tests are isolated from each other you can run them in parallel.

As long as the required behavior hasn't changed, leaving the test in place is fine.

If you come to discover that you have made a test redundant, you can mark it as deprecated. That can act as a signal to your future self that you are confident that the verification of this requirement is handled elsewhere.

You retire the deprecated test when it gets in the way (because you want to retire the API it used, because the test is brittle, and so on).

In effect, tests have a draft->supported->deprecated->retired life-cycle similar to your implementations.

As long as your changes to the implementations are extensions (adding more stuff, as explained by Rich Hickey), the old tests can continue to run and pass.

What that normally looks like is making the api of the subject more flexible (optional arguments, a new named parameter, overloading) and having the original API delegate its work to the new one. This will often mean introducing a null object into your design to support the existing API

# Old version
def do_the_thing(a,b,c):
    # generate value

# New version
def do_the_thing(a,b,c,d = None):
    if d is None:
        d = InertSubstitute()
    # generate value

Added bonus: if you learn how to extend your API in a backwards compatible way to support your tests, you'll also be learning how to extend your API in a way that is backwards compatible for the "real" clients of your API.