TDD and anti-patterns - Chapter 6

21 Jun 2022

This is a follow up on a series of posts around TDD anti patterns. The first of this series covered: the liar, excessive setup, the giant and slow poke, those four are part of 22 more, the anti-patterns is a list formalised in James Carr post and also discussed in a stackoverflow thread.

In this blog post, we are going to focus on the last two anti-patterns from those lists and also two new ones: the flash and the jumper. Both of them are focused on the practice of test driving code instead of the pure code itself.

(In the previous links you can see the videos of the sessions on demand. You just have to leave your email and you will be able to access the video of the session).

Takeaways

  • The one is an aggregation of anti-patterns
  • Having to deal with global state during test execution brings complexity
  • Most of the anti-patterns covered in this series are related to coding, the flash focuses on the practice of writing tests
  • The jumper is related to learning how to test drive code 

The one

Despite the name, The one is the anti-pattern that aggregates different anti-patterns, by definition, it is related to The Giant and The Free Ride. So far, we have seen that the anti-patterns are usually combined and it is not easy to draw a clear line when one ends and the other starts.

As we already saw, The Giant appears when a test case tries to do everything at once, in a single test case, in the first episode we saw a nuxtjs snippet that fit the ‘the giant’ definition.

Therefore, there are some variants to that. The following snippet is extracted from the book xunit and depicts the giant (even though the number of lines is not as many as in the first episode), but instead, the giant is figured here because the test case is trying to exercise all the methods at once: 

public void testFlightMileage_asKm2() throws Exception {
    // set up fixture
    // exercise constructor
    Flight newFlight = new Flight(validFlightNumber);
    // verify constructed object
    assertEquals(validFlightNumber, newFlight.number);
    assertEquals("", newFlight.airlineCode);
    assertNull(newFlight.airline);
    // set up mileage
    newFlight.setMileage(1122);
    // exercise mileage translator
    int actualKilometres = newFlight.getMileageAsKm();
    // verify results
    int expectedKilometres = 1810;
    assertEquals( expectedKilometres, actualKilometres);
    // now try it with a canceled flight
    newFlight.cancel();
 
    try {
        newFlight.getMileageAsKm();
        fail("Expected exception");
    } catch (InvalidRequestException e) {
        assertEquals( "Cannot get cancelled flight mileage",
        e.getMessage());
    }
}

 

The comments even give us a hint on how to split the single test case into multiple ones. Likewise, the free ride also can be noted in this example, as for each setup, there are assertions that follow.

The next code example might be clearer to see when the free ride appears, as already depicted in the previous episode, the example that follows was extracted from the Jenkins repository. In this case, the mix between the free ride and the giant is a bit blurry, but still, it is easy to stop that a single test case is doing too much.

public class ToolLocationTest {
    @Rule
    public JenkinsRule j = new JenkinsRule();
 
    @Test
    public void toolCompatibility() {
        Maven.MavenInstallation[] maven = j.jenkins.getDescriptorByType(Maven.DescriptorImpl.class).getInstallations();
        assertEquals(1, maven.length);
        assertEquals("bar", maven[0].getHome());
        assertEquals("Maven 1", maven[0].getName());
 
        Ant.AntInstallation[] ant = j.jenkins.getDescriptorByType(Ant.DescriptorImpl.class).getInstallations();
        assertEquals(1, ant.length);
        assertEquals("foo", ant[0].getHome());
        assertEquals("Ant 1", ant[0].getName());
 
        JDK[] jdk = j.jenkins.getDescriptorByType(JDK.DescriptorImpl.class).getInstallations();
        assertEquals(Arrays.asList(jdk), j.jenkins.getJDKs());
        assertEquals(2, jdk.length); // JenkinsRule adds a 'default' JDK
        assertEquals("default", jdk[1].getName()); // make sure it's really that we're seeing
        assertEquals("FOOBAR", jdk[0].getHome());
        assertEquals("FOOBAR", jdk[0].getJavaHome());
        assertEquals("1.6", jdk[0].getName());
    }
}
 

The peeping tom

Having to deal with global state in test cases is something that brings more complexity, for example, it requires a proper clean-up before each test and even after each test case is executed in a way to avoid side effects.

The peeping Tom depicts the issue that using global state brings during test execution, in stack overflow, there is a thread dedicated to this subject which has a few comments that helps to understand better what it is. Christian Posta also blogged about static methods being code smells.

In there, there is a snippet that was extracted from this blog post that depicts how the use of singleton and static properties can harm the test case and keep state between tests. Here, we are going to use the same example with minor changes to make the code compile.

The idea behind the singleton is to create and reuse a single instance from any kind of object. So to achieve that we can create a class (in this example called MySingleton) and block the creation of an object through its constructor and allow only the creation inside the class, controlled by the method getInstance:

public class MySingleton {
 
    private static MySingleton instance;
    private String property;
 
    private MySingleton(String property) {
        this.property = property;
    }
 
    public static synchronized MySingleton getInstance() {
        if (instance == null) {
            instance = new MySingleton(System.getProperty("com.example"));
        }
        return instance;
    }
 
    public Object getSomething() {
        return this.property;
    }
}
 

When it comes to testing, there is not much to deal with, so for example, the method exposed in the MySingleton called getSomething can be invoked and asserted against a value as shown in the following snippet:

import org.junit.jupiter.api.Test;
 
import static org.assertj.core.api.Assertions.assertThat;
 
class MySingletonTest {
    @Test
    public void somethingIsDoneWithAbcIsSetAsASystemProperty(){
        System.setProperty("com.example", "abc");
        MySingleton singleton = MySingleton.getInstance();
        assertThat(singleton.getSomething()).isEqualTo("abc");
    }
 
}
 

A single test case will pass without any problem, the test case creates the singleton instance and invokes the getSomething to retrieve the property value defined when the test was defined. The issue arises when we try to test the same behaviour but with different values in the System.setProperty.

import org.junit.jupiter.api.Test;
 
import static org.assertj.core.api.Assertions.assertThat;
 
class MySingletonTest {
    @Test
    public void somethingIsDoneWithAbcIsSetAsASystemProperty(){
        System.setProperty("com.example", "abc");
        MySingleton singleton = MySingleton.getInstance();
        assertThat(singleton.getSomething()).isEqualTo("abc");
    }
 
    @Test
    public void somethingElseIsDoneWithXyzIsSetAsASystemProperty(){
        System.setProperty("com.example", "xyz");
        MySingleton singleton = MySingleton.getInstance();
        assertThat(singleton.getSomething()).isEqualTo("xyz");
    }
}
 

Given the nature of the code, the second test case will fail and shows that it still holds the value abc as depicted in the following image:

As the singleton guarantees only one instance from a given object, during the test execution, the first test that is executed, creates the instance and for the following executions, it reuses the instance previously created.

In this test case is “easy” to see as one test case is executed after the other, but, for testing frameworks that execute tests in parallel or that do not guarantee the test order, it also can fail for different reasons.

As the singleton class has a private property that controls the instance created, it is not possible to clean it without changing the code itself (which would be a change just for testing purposes). Therefore, another approach would be to use reflection to reset the property and always start with a fresh instance, as the following code depicts:

class MySingletonTest {
 
    @BeforeEach
    public void resetSingleton() throws SecurityException, NoSuchFieldException, IllegalArgumentException, IllegalAccessException {
        Field instance = MySingleton.class.getDeclaredField("instance");
        instance.setAccessible(true);
        instance.set(null, null);
    }
}
 

Making use of reflection, it is possible to reset the instance before each test case is executed (using the annotation @BeforeEach).  Although this approach is possible, it should bring attention to the extra code and possible side effects while trying to test the application using such a pattern.

Depicting the peeping tom like that, besides having to use reflection to reset a property might not seem to be harmful to test drive code, but it can become even harder when a piece of code that we want to test depends on a singleton.

As well shared by Rodaney Glitzel, the problem is not singleton itself, but a code that depends on that, doing so, the code that depends on that becomes harder to test.

The flash

Developers that get blocked trying to practise TDD, and before splitting up the test in smaller chunks, they are thinking about all the edge cases that the test would cover.

The flash usually happens when someone is starting to practice TDD and start to hush into something before getting the basics in place, basics meaning the TDD flow (which links to the next anti-pattern introduced here).

Besides that, the flash also happens as a consequence of the following:

Big steps

Big steps are usually the first barrier to starting with TDD, what is a small step? What is the right size of a step actually? This could be interpreted as a subjective matter, and it is. But, reaching to the least effort possible to write some tests is something that can be practised. In the TDD by example, Kent Beck relates the baby steps as follows:

Test methods should be easy to read, pretty much straight line code. If a test method is getting long and complicated, then you need to play “Baby Steps.” TDD by Example.

Focus on generalisation

Another approach that usually leads to a blocker on the TDD flow is trying to generalise too much from the start and not talk with the test (or the production code).

The jumper

The practice of trying to follow the test driven development approach, but jumping up to the next steps of the flow before completing the one before.

To understand why this is an anti-pattern, first, we will go over the famous TDD flow and it should be used when coding. The classic approach is to start with a failing test (red), write enough code to make it pass (gree) and then refactor to change anything that is needed to make the code better (if applicable, in the episode number 90, Uncle Bob says that he usually go for red-green may times before jumping to the refactor phase).

Usually, the refactor phase is not mandatory as the red-green, refactoring is something that is not that frequent when starting, it comes as the code evolves and as the code needs changes to fit in new requirements.

Therefore, trying to follow those rules strictly usually comes with some assumptions and thus, leads to a difficult flow or even “jumping” steps, so the following section lists what potentially to avoid when practising TDD.

Red

  • Trying to make it “perfect” from the start
  • Get blocked by not knowing what to code
  • Refactor on the red
        Changing class name
        Fixing styles (any kind of style)
        Changing files that are not related to the test

Green

  • Not trusting the hard coded (“I know it will pass”)

Refactor

  • Making changes that break the tests

Those tips, in the end, come down to practice, if you practise TDD you most likely have faced that before and if not you might face them as you progress. The most important thing to keep in mind is to follow the rules in the flow as strictly as possible.

Wrapping up

The last two anti-patterns as defined by James Carl blog post The one and The peeping tom were covered and once again those are strictly related to the way the code is written.

The one is a combination of different anti-patterns and in this case, we used specifically The Giant and The Free Ride and we tried to depict how those are closely related to each other, but remember, any combination is possible.

The peeping tom is the one that depicts how global state can hurt the experience of writing tests with different sets of data, nonetheless, there are contexts in which such behaviour is needed. TO avoid this one, using abstracting comes in handy.

As this is the last episode of the anti-patterns series and we cover a lot of things over those months and if you enjoyed the series as we did, you have faced some of them on your own code base, and if not, don't worry soon you will see them - and this is fine.

This series was an attempt to spread the word on the anti-patterns list and how they impact the experience when testing drive code through open-source projects and also from our experience.

If you want to check our last session, the flash and the jumper, here is a link.

As always, happy testing!