DRY code, DRY tests?

Question

At some point I had to create some class "Class1" and that class needs a method "method". So I have the following:

Class1MethodTest: A total of N tests that check the behavior of Class1.method
Class1 method: A full implementation of the method

But a bit later I need a different class "Class2" to have a completely similar method "method". Now I have several approaches:

Approach 1

Class1MethodTest: A total of N tests
Class1 method: Full implementation
Class2MethodTest: Another set of identical tests
Class2 method: Another full implementation

Pros: Stupid simple

Cons: not DRY

At least that's the first attempt and I might even write this before doing any refactoring, right?

Approach 2:

_hidden_private_implementation_function: Full implementation of required method
Class1MethodTest: A total of N tests
Class1 method: Call hidden_private_whatever
Class2MethodTest: Another set of identical tests
Class2 method: Also call hidden_private_stuff

Pros: DRY code, still stupid simple

Cons: Tests aren't DRY, "Test interface, not implementation"

Approach 3:

MethodTest: A total of N tests
TotallyPublicCommonMethod: Full implementation of required method
Class1MethodTest: Just one test to verify that Class1 method calls the Public one
Class1 method: Call public common method
Class2MethodTest: One more test
Class2 method: Also call common method

Pros: DRY, stupid simple

Cons: .. any other than "You're testing implementation, not interface"?

Approach 4:

This is where it gets a bit exotic. Python allows me to DRY the "Approach 3" directly:

_hidden_private_implementation_function: Full implementation of required method
makeTestForClass(cls): return a total of N tests for class cls
Class1MethodTest = makeTestForClass(Class1)
Class1 method: Call hidden_private_whatever
Class2MethodTest = makeTestForClass(Class2)
Class2 method: Also call hidden_private_stuff

Pros: DRY, "Don't test implementation"

Cons: Not that simple. Too hard to modify if I ever decide to change something in Class1.method, but not Class2.method

I can think of a couple more approaches but those are not very different from these above.

Right now I have some code looking something like "Approach 1" and I am thinking on which way I should go next to make it all cleaner and better.

Answer 1

I generally write enough tests to give me confidence that my implementation is correct, but no more than that. How many tests this is depends on the problem at hand.

If you're feeling very unsure about the correct implementation of the behaviour, you'll probably end up writing a full set of tests for each endpoint and refactoring at the end: going from Approach 1 to Approach 2, as you've labelled them. Kent Beck calls this sort of programming "driving in a low gear".

If your problem is not so complex, you might want to change up a gear or two. Remember, TDD is about doing "the simplest possible thing" at every step. If you can see that the simplest way to make your next test pass is just to call the method you've already implemented, then you can go home early.

If you get an unexpected bug or test failure, you're free to slow down, write more tests, and drive in a lower gear again. TDD often feels like this: constantly adjusting your pace to match your confidence and fear levels. It's one of the themes of Kent Beck's book.

Remember, you're not aiming for 100% coverage of every public method; just enough to give you confidence that your code is correct.

Answer 2

You're right when you tell that you may use the first approach before refactoring. While personally, I disagree with this approach, that's the rule of three from Refactoring by Martin Fowler (page 58):

Here's a guideline Don Roberts gave me: The first time you do something, you just do it. The second time you do something similar, you wince at the duplication, but you do the duplicate thing anyway. The third time you do something similar, you refactor.

It remains that the duplication in code is annoying. From here, you have to think about the exact context of the duplication:

• Either two classes are related (for example both Cat and Dog can be fed, and it takes the same steps to feed them both, except the food which will change), in which case, create an inheritance (in my example, a parent class Pet),

• Or two classes are unrelated, and just rely on the same method. For example, WebRequest class may rely on GetSlug¹ to find the slug of the current request; Customer may also rely on GetSlug to normalize the name of the customer for further search purposes. However, WebRequest and Customer are totally unrelated, and it makes no sense to create a common object for those classes.

  Here, the solution would be to call a common method from those two classes, by instantiating the third class containing this method. For example, both RequestUri and CustomerName may become objects implementing ISlug interface.

In unit tests, things are different. If you find yourself duplicating lots of code, it indicates that your unit tests contain too much logic. Thus, they should be shortened.

For example:

@Test
public void ensureCatHungryByDefault()
{
    var cat = new Cat();
    assertTrue(cat.Hungry);
}

@Test
public void ensureCatFed()
{
    var cat = new Cat();
    var food = new Whiskas(100.Gramm);
    cat.feed(food);
    assertFalse(cat.Hungry);
}

@Test
public void ensureCatThrowsOnNull()
{
    var cat = new Cat();
    exception.expect(IllegalArgumentException.class);
    cat.Feed(null);
}

and:

@Test
public void ensureDogHungryByDefault()
{
    var dog = new Dog();
    assertTrue(dog.Hungry);
}

@Test
public void ensureDogFed()
{
    var dog = new Dog();
    var food = new Pedigree(600.Gramm);
    dog.feed(food);
    assertFalse(dog.Hungry);
}

@Test
public void ensureDogThrowsOnNull()
{
    var dog = new Dog();
    exception.expect(IllegalArgumentException.class);
    dog.Feed(null);
}

contain some duplication, but it's acceptable. The two flaws of duplication are the duplication of logic and duplication of data. There is no business logic inside unit tests. As for the data, the data related to a dog is different from the data related to a cat, thus we consider that there is no duplication.

By keeping cat and dog tests separate, you gain both readability and flexibility:

• You gain readability, because if you later break code, you can easily understand that the change broke only Cat, only Dog or both.

• You gain flexibility, because you can work on changing Cat class and deal with the corresponding unit tests, without having to deal with other pets. If tests were common to all pets, it could have created a difficulty when your business logic changes, such as all cats mutate, and should be fed very differently from now.

---

^{¹ A slug is a transformation of text which consists of removing all diacritics and replacing all consecutive special characters or whitespace by dashes. Example: Il était une fois un... hérisson gives il-etait-une-fois-un-herrison. This technique is usually used in URIs, and can also be used for search purposes where searching for herisson should also include hérisson in search results.}

Answer 3

TL;DR: Extracting common functionality is fine as long as the stronger coupling is warranted by code cohesion.

Refactoring often occurs in areas which already have strong cohesion, and as a result should also have strong coupling. When you extract common functionality into some shared structure you increase the coupling. When code is duplicated it typically means the duplicated piece can be modified without impacting the other, ie. they are decoupled. If 2 or more pieces of code now delegate to a shared piece of code they are now coupled to that shared code. If the shared code changes, then all code that delegates to the shared code are affected, ie. they are strongly coupled.

This is all fine and dandy as long as the all the code that delegate to the shared code are all strongly cohesive. If they aren't then you are creating bad strong coupling.

What's all this mean? That it's fine to extract and refactor in tests as long as you aren't creating strong coupling that shouldn't be there. To put it another way. If you have to separate methods that are incidentally similar then they should not be refactored to use shared code because they shouldn't be coupled together. If, on the other hand, the separate methods are similar because they are related in some meaningful way (cohesive) then extracting shared code is fine.

Answer 4

My guiding principle here would be that test code has to meet the same level of quality as implementation code. If your test code can't reach that level, then something is wrong with the implementation design.

In the specific context of a method that is now common to two classes, I would try to refactor that into a common class that both Class1 and Class2 inherit. As noted by @MainMa, if you had Cat and now have Dog, refactor their feed method into class Pet.

If you feel this is not right, because Class1 and Class2 conceptually have nothing in common, then you need to define a new class that the others use in order to attain their purpose. As you ask in a comment, if the method to refactor is save_to_database, then you may need to introduce a class called Database, which classes Cat and Screwdriver use internally to persist their state.

Then your question turns into testing the interface of this new class, whose sole purpose is to store things, regardless if they come from a cat or from a screwdriver.

The first approach gives something like this:

class Cat:
    def feed(self):
        """
            >>> cat = new Cat()
            >>> cat.feed()
        """
        pass

# turns into

class Pet:
    def feed(self):
        """
            >>> pet = new Pet()
            >>> pet.feed()
        """
        pass

class Dog(Pet):
    pass

class Cat(Pet):
    pass

The second approach, which is I guess more relevant to you, would look like this:

class Customer:
    def save(self):
        """
            >>> customer = new Customer()
            >>> customer.save()
            >>> another_db_conn = db.connect()
            >>> another_db_conn.read(customer.id) == customer.name
            True
        """
        conn = db.connect()
        conn.write(self.name)

# turns into

class Database:
    def save(self, data):
        """
            >>> db = new Database()
            >>> db.save("whatever")
            >>> another_db_conn = db.connect()
            >>> another_db_conn.read() == "whatever"
            True
        """
        conn = db.connect()
        conn.write(self.name)

class Customer:
    def __init__(self, database):
        self.db = database

    def save(self):
        self.db.save(self.name)

class Whale:
    def __init__(self, database):
        self.db = database

     def save(self):
         self.db.save("the whales!")

Notice I have used doctests here in order to show implementation and tests mixed together. You may prefer to use unittests or whatever. Also, the Database example is too generic and as it stands the class does not add value: it should do something interesting on the data it gets and that behavior is what you want to test; but this is just an example.

If you have a concern that defining new classes to be used may "open up" your package with a lot more public API than you would like to, remember you can alway define such classes as package-private (in Java), or just not export them out of your package (in Python). Tests defined within the package will still be able to access them.

Answer 5

Your test module could contain an interface common for both classes and corresponding adaptors.

Then you could wrap class under test and pass it to the common test code.