Can we add methods which is easier for assertion in test and which is only used in tests when TDD?

Question

Here let me take a simple parking boy example.

A parking boy can manage several parking lots, and when a car comes, he will park the car into some parking lot and can also help to un-park the car.

So basically in my mind, there will be a ParkingBoy class like this:

class ParkingBoy {
    public ParkingBoy(List<ParkingLot> parkingLots) {}
    public Ticket park(Car car) {}
    public Car unpark(Ticket ticket) {}
}

But in the practise of TDD, we design the api from the user's perspective, and won't presume what the actual class is and what methods it has. Let the tests drive them.

So I first split the requirement into several small tasks:

1. A parking boy can manage parking lots
2. A parking boy can park a car to any avaiable parking lot, and return a ticket
3. A parking boy can unpark the car for a ticket

When doing the first task, my test is:

class ParkingBoySpec {
    @Test public void should_manage_parking_lots() {
        List<ParkingLot> parkingLots = list(parkingLot1, parkingLot2);
        ParkingBoy boy = new ParkingBoy(parkingLots);
        assertThat(boy.getParkingLots()).containsExactly(pakrkingLot1, parkingLot2);
    }    
}

But my friend has questions about the getParkingLots method.

In his opinion, this method is just used in the assertion of test, and will not be used in anywhere of the implementation, so we should not provide it. And even, we actually don't care the parking lots managed by the boy, what we care is he can park and unpark the car. So we should remove the first task and start from the task 2(park).

My opinion is:

1. Since we write tests before implementation in TDD, we actually don't know if the getParkingLots will be used in the later implementation.
2. Although the main purpose of the parking boy is to park and unpark a car, but I want to start from a simple task
3. The test is the "user" of the code, so when the test thinks if there is a getParkingLots, it's a good reason to provide a getParkingLots in ParkingBoy

How do you think this question?

Answer 1

A parking boy can manage several parking lots, and when a car comes, he will park the car into some parking lot and can also help to un-park the car. [...]

So I first split the requirement into several small tasks:

1. A parking boy can manage parking lots
2. A parking boy can park a car to any avaiable parking lot, and return a ticket
3. A parking boy can unpark the car for a ticket

In my opinion, the root cause of the problem is the way that you split up the requirement is smaller tasks.
For example, what does it mean that a parking boy "manages" parking lots? Most likely it means that the parking boy knows about multiple parking lots to be able to park/unpark cars, but does it also mean that he can provide you information about those lots?

When decomposing the requirement about the parking boy into smaller tasks, you should try to keep those tasks closely related to the tasks that the final software will likely want to do with the parking boy.
If the end product only cares about the ability to park/unpark cars, then a breakdown like this would be more suitable:

1. A parking boy can park a car to any available parking lot that the boy knows about, and return a ticket
2. A parking boy will refuse to park a car if there are no available lots to the boy's knowledge
3. A parking boy can unpark the car when presented with a ticket

The biggest leap that you need to make here is that the first task/test requires two interfaces to be made: One to tell the parking boy about parking lots (could be the constructor) and another to park a car. If that bothers you, you can build up the test in multiple stages: First verify, by having compilation failures, that you must provide a list of parking lots and after that add the call to park a car to the same test.

Answer 2

1. A parking boy can manage parking lots

Wrong. The very fact there is "manage" in that sentence should be huge red flag. This sentence says absolutely nothing about behavior of parking boy. If there is no interesting behavior, then there is no need to implement a test for this. Having this requirements can actually be dangerous, because different people imagine different things when reading this sentence. Actually, this kind of sentence would be high-level user story, that gets divided into specific tasks, like what happens if all lots are full or if customer looses a ticket.

Although the main purpose of the parking boy is to park and unpark a car, but I want to start from a simple task

Then you start with "Parking boy can park a car to a parking lot and return a ticket." This would mean from the start, Parking Boy only know about single parking lot. Only after you get requirement like "Parking boy can park multiple cars" should you realize he needs a list of Parking lots.

Answer 3

What you should ask yourself is whether the simple fact that a ParkingBoy is managing a set of ParkingLots is something that you should be testing. What you probably should be testing is what effect does the fact that a ParkingBoy has a set of ParkingLots have on the behaviour of your class?

Here are a few approaches that I often consider (none perfect), which may give you the ability to test your class without requiring test-accessors (but with other trade-offs).

1. Test the effects on your collaborators' state.

Instead of testing whether parkingBoy is managing parkingLots directly, you could test the effect of parking a car has on the state of parkingBoy's parkingLots:

public void mustParkCarInManagedParkingLot() {
    ParkingBoy parkingBoy = new ParkingBoy(parkingLot1, parkingLot2);
    parkingBoy.park(car);
    assertThat(parkingLot1.isOccupied(car), equalTo(true));
    assertThat(parkingLot2.isOccupied(car), equalTo(false));
}

Of course, this assumes that ParkingBay has methods that allows you to query its state. If not, then you are probably just passing the buck.

2. Test the effects on your collaborators interface

There is another popular type of unit testing approach where you don't test the effect of behaviour on the state of the class, but you actually test the interaction of your class with its collaborators. This is called "Behavioural Testing" if I recall correctly.

This approach is most practical when used with a mocking framework (I use Mockito as it is relatively simple):

@Mock ParkingLot parkingLot1;
@Mock ParkingLot parkingLot2;

public void parkingCarMustOccupyManagedParkingLot() {

    ParkingBoy parkingBoy = new ParkingBoy(parkingLot1, parkingLot2);
    parkingBoy.park(car);
    verify(parkingLot1).add(car);
    verify(parkingLot2, never()).add(car);
}

What you are doing here is testing whether the park method has an effect on the ParkingLot collaborator via a behavioural method call rather than inspecting state (which requires accessors). The nice thing with the mocking framework is that your test class is not dependent on implementation details of ParkingLot itself - it simply requires thet the .add() method be invoked at the correct time. So to some extend you are protecting your unit test of ParkingBoy against changes in ParkingLot.

Neither of these approaches are perfect. The first still requires that you access some state, if indirectly. The second ties your test code to some private implementation details - the interaction with the collaborators, but at least this only relies on the public API of those collaborators, which I usually feel I can live with.

Answer 4

Congratulations. You've just discovered White Box Testing.

As your friend has correctly stated, the general consensus around unit testing is that unit tests should only test the public API of a class, method or module. It follows that, if you have internal methods, you test them indirectly, via the external methods.

In other words, unit testing is really Black Box Testing, according to the purists.

There's a problem with this, however: the API that you provide to the user of your class is not necessarily the same API that you want to test with. Can you achieve better test coverage by unit-testing your internal methods directly? Of course you can, that seems obvious.

The purists will say, however, that when you test internal methods, you're really testing what amounts to an implementation detail, not behavior. It is the behavior of your external API that matters, not what goes on under the hood. To which I would counter: integration tests are probably just as good, if not better, than unit tests for that purpose.

In addition, testing private methods can be a pain. How does one test methods that don't expose a public interface? There are several ways: you can use internal-accessor classes, reflection or (in C#) partial classes. None of these ways are ideal.

So what to do?

Lately, I have been refactoring my internal methods into separate classes. Sometimes these are collections of referentially-transparent utility methods in static classes. These methods are highly testable and don't require mocks; all you do is hand them some parameter values and assert their returned result. Sometimes they are simply helper classes which I compose into the main class.

I recognize that these techniques aren't the latest fashion, but they have the virtue of completely eliminating the "don't test your internal methods" problem, because none of the methods are private. They are the only ways that I know of to get Uncle Bob's ideals without compromise: small, testable methods utilizing clean code in well-organized, small classes.

Answer 5

Normally, you should not add methods which only purpose is to violate incapsulation by exposing internal state of the objects. The need of them is a good sign of issues with the design and it's better to review it and find missing entities and operations.

In your example this is exactly the case: in real world there's usually some area identified as parking and split into multiple lots. First question the driver/parking boy wants to ask is "Can I park on this parking?" Sometimes there's even a screen with a number of available parking lots, which is not a property of any specific lot, but an interface to the parking as a whole. This means, that you need to add one more entity: Parking

In this entity you will certainly need operations to query total space and number of available lots (to be able to answer questions like "can I park" and "what is the revenue"), as well as operation to get the next available lot.

interface Parking {
    int getAvailableSpace() { ... } 
    int getSize() { ... } 
    /** returns DTO with number and directions to the taken parking lot */
    Route takeNextFreeLot() { ... } 
    void release(int lot) { ... }
}

This interface can then be extended to allow parking lots with electric chargers, dedicated lots for people with disabilities, lots for trucks etc. The tests for it will look very straightforward:

Parking parking = new ParkingBuilding(15);
assert(15, parking.getSize());
assert(15, parking.getAvailableSpace());
int number = parking.takeNextFreeLot().getNumber();
assert(14, parking.getAvailableSpace());
parking.release(number);
assert(15, parking.getAvailableSpace());

Parking boys then will work with the parking and record lot numbers in tickets, but will not operate with ParkingLot objects which will become the internal state of the Parking. Tests for ParkingBoy will look like following:

@Mock Parking parking;

public void parkingCarMustOccupyParkingLot() {
    when(parking.takeNextFreeLot()).thenReturn(new Route(1));
    ParkingBoy parkingBoy = new ParkingBoy(parking);
    Ticket ticket = parkingBoy.park(car);
    verify(parking).takeNextFreeLot();
    parkingBoy.getCar(ticket);
    verify(parking).release(1);
}