Testing a function that only calls one or two other functions [duplicate]

Question

The short version

The code

As part of TDD, we often end up with functions that follow this pattern:

function onSendRequestForSelected() {
    this.deselectStepsWithRequest();
    this.sendRequestForSelected();
}

Each of the internally called functions has their own unit tests.

The tests

Our unit tests for this function end up pretty much ensuring the function body calls were made:

expect( iStep.deselectStepsWithRequest ).toHaveBeenCalled();
expect( iStep.sendRequestForSelected ).toHaveBeenCalled();

The problem

The problem is:

• Our tests end up testing function calls rather than behaviour.
• We're unsure what is exactly the purpose of these tests - they protect from nothing other than any future changes.
• Nor do they serve as a design documentation.
• They clearly test the how rather than what.

---

The long version

Starting point

We start with the test:

it( 'should send a request for each of the selected steps', ... )

And then implement:

function onSendRequestForSelected() { ... }

So far so good.

New requirement, refactoring

Then we realise we need to deselect steps that already have a request.

So we refactor:

it( 'should send a request for each of the selected steps', ... )

becomes the test for

function sendRequestForSelected() { ... }

and create

it( 'should deselect steps that already has a request', ... )

implemented by

function deselectStepsWithRequest() { ... }

The rubbish test

and then introduce:

function onSendRequestForSelected() {
    this.deselectStepsWithRequest();
    this.sendRequestForSelected();
}

but no idea how to test it.

it( 'should send a request for each of the selected steps', ... )

makes little sense as the test itself only ensure a function is called.

it( 'should call sendRequestForSelected()', ... )

seems like a rubbish test.

The question

So it seems you end up with two options, neither is ideal. Which one is better?

Given TDD we start with:

it( 'should deselect steps that already has a request', ... )
it( 'should send a request for each of the selected steps', ... )

function onSendRequestForSelected() {
    ...
}

Option 1

it( 'should deselect steps that already has a request', ... )
it( 'should send a request for each of the selected steps', ... )

function onSendRequestForSelected() {
    this.deselectStepsWithRequest();
    this.sendRequestForSelected();
}

Problems:

• The actual sub-functions are not tests (if they would they would be redundant). Dangerous if they will be reused (consider them being needed by other functions).

Option 2

// No test

function onSendRequestForSelected() {
    // Test for each of the following
    this.deselectStepsWithRequest();
    this.sendRequestForSelected();
}

Problems:

• This is not really TDD; this is not how we started.
• No test to serve as a design documentation. We do not define the behaviour.

In graphical form

Where do you drop the tests?

• Testing F1 and F2 only will yield redundant tests for S2.
• Testing for S1, S2, and S3 will not test for the behaviour of F1 and F2.

Answer 1

This is obviously a controversial topic.

Having consulted a multitude of opinions, the conclusion is that this is correct:

Given the method

function onSendRequestForSelected();

and its pre-implementation tests:

it( 'should deselect steps that already has a request', ... )
it( 'should send a request for each of the selected steps', ... )

the eventual implementation shall be:

function onSendRequestForSelected() {
    this.deselectStepsWithRequest();
    this.sendRequestForSelected();
}

and the eventual tests shall be:

it( 'should deselect steps that already has a request', function(){
    expect( iStep.deselectStepsWithRequest ).toHaveBeenCalled();
});

it( 'should send a request for each of the selected steps', function(){
    expect( iStep.sendRequestForSelected ).toHaveBeenCalled();
});

Reasoning

Much of the reasoning behind this is based on my argument why you should write a test for every method (well nearly), including setters and getters.

In short:

• TDD is not only about defence, but also design and documentation.
• For design and documentation purposes, tests shall be written before implementation.
  • This is a fundamental principle of TDD so it may sound elementary.
  • But the question needed to be asked is: "Can someone implement this part of the system based on the provided tests?".
  • So if there is no test, there will be no implementation.
• Test are subject to refactoring and extractions much as implementation code is.

Test extraction

So basically, the expect line here:

it( 'should deselect steps that already has a request', function(){
    expect( iStep.deselectStepsWithRequest ).toHaveBeenCalled();
});

is a way of saying:

The test condition, instead of being satisfied here will be satisfied by calling deselectStepsWithRequest()

(for which there is a separate test).

Is like code extraction

much in a way this.deselectStepsWithRequest() in here:

function onSendRequestForSelected() {
    this.deselectStepsWithRequest();
}

is a way of saying

Instead the execution code being here, it is in deselectStepsWithRequest().