Double Dabble

Posted: 2011-12-19 by Roger Wernersson in Best Practices, Design, Programming, Refactoring, Unit Tests

I’ve been trying out a bit of TDD.

Basically, I think TDD is the only way forward.

I’ll start at the beginning.

Code evolve, and as it does, it also decays if not pruned, a bit like a garden in that respect. Pruning mean refactoring to make it fulfill its evolving purpose.

Refactoring is only ever safe if we have a good unit test coverage. Unit tests we write after the production code is written are OK at best, because we tend to test things we know works. We also tend to test in a way that makes it harder to change the implementation, because as we are aware of the implementation, we take shortcuts. I’ve seen this a lot of times.

TDD forces us to write the test code first, before the production code. Hence, we can’t test the implementation, because there isn’t any. This makes it much easier to refactor the production code.

However.

The TDD cycle is something like this: write a test, test fails, write production code until the test succeeds, refactor the code.

Now, most people skip that last step.

But from what I’ve seen, even people who actually do the last step doesn’t do it fully; they refactor the production code, but not the test code. This will be their downfall. As test code decays, the production code will go stale and then everything goes downhill.

Here’s the problem: How do you ensure that you don’t break the test code when you refactor it?

I’ve mostly used TestNG. It might be a TestNG only problem, but I doubt it.

Let’s say I’ve got some production code I want to test, in this case equals(), a method actually generated by Eclipse.

    public boolean equals( Object obj )
    {
        if (this == obj) return true;
        if (obj == null) return false;
        if (getClass() != obj.getClass()) return false;
        Person other = (Person) obj;
        if (myId != other.myId) return false;
        return true;
    }

Here’s my test method, using FEST asserts for the nice flow:

    @Test
    public void equalsIfAllFieldsAreEqual()
    {
        Person actual = createPerson();
        Person expected = createPerson();
        
        boolean isEqual = actual.equals(expected);
        
        assertThat(isEqual).isTrue();
    }

Not much to refactor, but it’s only an example. If I make a mistake when I refactor the production code, the test will fail and I’ll know.

But if I make a mistake when I refactor the test code so that I now test something else, or even worse, I don’t actually test anything, how would I know?

I gave this some thought and came up with a solution, but it’s somewhat convoluted. Here it is:

I’ve added a new class of which I need an instance for each testable feature:

public class Testable
{
    private boolean myState = true;
    private boolean myWasDoubleChecked = false;
    private String myName;

    public Testable(String name)
    {
        myName = name;
    }
    
    public boolean asBoolean()
    {
        return myState;
    }

    public void activate()
    {
        myState = true;
    }

    public void deactivate()
    {
        myState = false;
        myWasDoubleChecked = true;
    }

    public boolean wasDoubleChecked()
    {
        return myWasDoubleChecked;
    }

    public String getName()
    {
        return myName;
    }
}

Then I need to add an instance for each testable feature.

public class Person
{
    private int myId;
    public static Testable FEATURE_EQUAL_IF_ALL_FIELDS_ARE_EQUAL = new Testable("equal if all fields are equal");
    public static Testable FEATURE_EQUAL_IF_SAME_OBJECT = new Testable("equal if same object");

    public boolean equals( Object obj )
    {
        if (this == obj) return FEATURE_EQUAL_IF_SAME_OBJECT.asBoolean();
        if (obj == null) return false;
        if (getClass() != obj.getClass()) return false;
        Person other = (Person) obj;
        if (myId != other.myId) return false;
        return FEATURE_EQUAL_IF_ALL_FIELDS_ARE_EQUAL.asBoolean();
    }
}

I added a new class which must be the base class for all tests:

public abstract class DoubleTester
{
    private boolean myDoubleCheck = false;
    private List myFeatures = new ArrayList();

    @BeforeClass
    public void createFeatureList()
    {
        addFeatures();
    }
    
    protected abstract void addFeatures();
    
    protected void add(Testable feature)
    {
        myFeatures.add(feature);
    }

    @AfterMethod
    public void activateAllFeatures()
    {
        for(Testable feature : myFeatures)
        {
            feature.activate();
        }
        
        myDoubleCheck = false;
    }

    protected void check(Testable feature)
    {
        if (myDoubleCheck)
        {
            feature.deactivate();
        }
    }

    protected void doubleCheck()
    {
        myDoubleCheck = true;
    }
    
    @Test
    public void makesSureWeRunAfterClass()
    {
    }
    
    @AfterClass
    public void makeSureWeDoubleCheckedAllFeatures()
    {
        for(Testable feature : myFeatures)
        {
            Assert.assertTrue(feature.wasDoubleChecked(),
                    "Add double check for \""+ feature.getName() +'"');
        }
    }
}

This means a test class must do the following:

public class PersonTest extends DoubleTester
{
    
    @Override
    protected void addFeatures()
    {
        add(Person.FEATURE_EQUAL_IF_SAME_OBJECT);
        add(Person.FEATURE_EQUAL_IF_ALL_FIELDS_ARE_EQUAL);
    }
    
    @Test(expectedExceptions=AssertionError.class)
    public void doubleCheckEqualsIfSameObject()
    {
        doubleCheck();
        equalsIfSameObject();
    }
    
    @Test
    public void equalsIfSameObject()
    {
        Person expected = createPerson();
        Person actual = expected;
        
        check(Person.FEATURE_EQUAL_IF_SAME_OBJECT);
        boolean isEqual = actual.equals(expected);
        
        assertThat(isEqual).isTrue();
    }
    
    @Test(expectedExceptions=AssertionError.class)
    public void doubleCheckEqualsIfAllFieldsAreEqual()
    {
        doubleCheck();
        equalsIfAllFieldsAreEqual();
    }
    
    @Test
    public void equalsIfAllFieldsAreEqual()
    {
        Person actual = createPerson();
        Person expected = createPerson();
        
        check(Person.FEATURE_EQUAL_IF_ALL_FIELDS_ARE_EQUAL);
        boolean isEqual = actual.equals(expected);
        
        assertThat(isEqual).isTrue();
    }
}

That’s it, basically.

Now, I’ve got several issues with this.

1. I need to add code to the production code, which makes it a source of errors. I could just add it for tests I want to refactor, but now that I know of the problem, I want to double check all my tests.

2. There is probably a performance impact. How do I get rid of that in production?

3. Will I ever get anyone else to write tests like this?

I think there might be a way of doing this using annotations. It would nice to be able to do the following instead:

@Testable
public class Person
{
    private int myId;

    public boolean equals( Object obj )
    {
        if (this == obj) return true; @Doublecheck EQUAL_IF_SAME_OBJECT
        if (obj == null) return false;
        if (getClass() != obj.getClass()) return false;
        Person other = (Person) obj;
        if (myId != other.myId) return false;
        return true; @Doublecheck EQUAL_IF_ALL_FIELDS_ARE_EQUAL
    }
}

And then have the test code go like this:

public class PersonTest extends DoubleTester
{
    @Doublecheck
    public void equalsIfSameObject()
    {
        Person expected = createPerson();
        Person actual = expected;
        
        @Check=Person.FEATURE_EQUAL_IF_SAME_OBJECT
        boolean isEqual = actual.equals(expected);
        
        assertThat(isEqual).isTrue();
    }
    
    @Doublecheck
    public void equalsIfAllFieldsAreEqual()
    {
        Person actual = createPerson();
        Person expected = createPerson();
        
        @Check=Person.FEATURE_EQUAL_IF_ALL_FIELDS_ARE_EQUAL
        boolean isEqual = actual.equals(expected);
        
        assertThat(isEqual).isTrue();
    }
}

Either way. I’d like to know what everyone else is doing to address this.

How do you make sure you don’t break the test you refactor?

/Roger

Flattr Roger

About these ads
Comments
  1. mortoray says:

    If a test harness becomes too complicated I find that people will stop writing tests, or not write as many of them. This is a significant problem and warrants keeping the test system simple to write.

    Refactoring tests is of course tricky as you say. I generally follow a basic two-rule. The first being that you cannot modify the main program code while altering a test — otherwise you wouldn’t just be refactoring the test. The second is that no assertions in the test code may be removed. Anything which was asserted before must still be asserted after. I simply use code review to do this.

  2. One trick you can do is to:
    1. Introduce error in the production code, making the test go red
    2. Refactor the test, make sure it still goes red when you’re done with it
    3. Change back the production code, make sure the test goes green.

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s