Don’t Let the Unit Tests Coverage Cheat you

Doing unit testing right is pretty difficult, most of us try to avoid it letting them to the end of the development, besides, our projects usually have some unit tests coverage requirements, so, we oriented our tests to accomplish those constrains.

In this post, I am going to talk about unit tests coverage, what that means, and why we usually believe a good coverage is the solution for all our problems.

TRY IT YOURSELF: You can find the source code of this post here.

Unit Testing Series

What Unit Test Coverage Is

Well, as our code is composed by classes, methods and lines of instructions, the unit tests coverage means, how many classes/methods/lines we stress using our unit tests.

That measure is usually defined as a percentage, for instance:

  • Classes: We must stress at least 95 %.
  • Methods: We must stress at least 90 %
  • Lines: We must stress at least 80 %

So, as we define this constrains in our software project, we need to create enough unit tests to accomplish those goals.

However, having a lot of unit tests and a good coverage, don’t mean our unit tests have quality.

Let’s See A Wrong Coverage Example

Okey, let’s define our project constrains:

  • Classes: We must stress at least 100 %.
  • Methods: We must stress at least 100 %.
  • Lines: We must stress at least 80 %

Now, we have the following Service class:

class Service {
  private final Repository repository;

  public Service(Repository repository) {
    this.repository = repository;
  }

  public User save(User user){
    if(isValid(user)){
      return repository.save(user);
    }

    throw new IllegalArgumentException("User is not valid");
  }

  private boolean isValid(User user) {
    return Objects.nonNull(user);
  }
}

As we can see, we have a method to save a User, however, we first check if the User is valid and then save it, otherwise, we throw an exception.

Next, we create the following unit test to accomplish our coverage goals:

@RunWith(MockitoJUnitRunner.class)
public class ServiceTest {
  @Mock
  private Repository repository;
  @InjectMocks
  private Service service;

  @Test
  public void testSave(){
    User user = new User("id", "name");

    when(repository.save(user)).thenReturn(user);

    User userResult = service.save(user);

    assertThat(userResult).isEqualTo(user);
  }
}

We just have one unit tests, if we run it in coverage mode in Intellj, we get:

Initial coverage for Service class

As we can see, we accomplish our coverage goals for Service class:

  • Classes: 100 %
  • Methods: 100 %
  • Lines: 85 %

NOTE: If you want to run the coverage process from your CI/CI pipeline, you can use some Maven plugins like JaCoCo.

Changing Some Logic

Now, a developer by mistake (or laziness) changes the isValid method to accomplish a new requirement, as we can see here:

class Service {
  private final Repository repository;

  public Service(
          Repository repository) {
    this.repository = repository;
  }

  public User save(
          User user){
    if(isValid(user)){
      return repository.save(user);
    }

    throw new IllegalArgumentException("User is not valid");
  }

  private boolean isValid(
          User user) {
    return true;
  }
}

Now, any User will be valid doesn’t matter if it is null.

Later, we execute the unit test to guarantee everything is fine:

Unit test passes

And the coverage to see if we still are in the constrains:

Coverage after changes the isValid method

Awesome, we still are in the constrains and our unit test is green.

However, there is something weird here, the unit test passes….. what????, this new requirement breaks my previous one, that should fail, we just change important production code.

Well, it is simple, let’s see what we did:

  1. We modified the production code (Service class) to have any User object valid.
  2. We run our unit tests and they are in green, so, we think we are good.
  3. We run our unit tests with coverage and they are into the constrains, so, we think we are good.
  4. As we are lazy, we just go forward with the next feature and forget this one.

However, our current unit test doesn’t have good quality, or, we don’t have enough unit tests to stress our complete logic. In this case, we missed to really stress isValid method.

NOTE: We usually have multiple branches/paths in our business logic, in this case, we have two, one when the User is valid, and one when the User is not valid. As we just tested one branch, we missed an important business logic validation.

REMEMBER: You should stress your code to have coverage by class/method/line, however, you must stress your code to have full business logic validation, this last one is most important, and it usually moves us to accomplish the first one automatically.

Adding Tests to Fully Cover our Logic

Now, let’s do this right. We need to fully stress our logic (multiple branches), DO NOT think about coverage, think about business cases we must validate:

@RunWith(MockitoJUnitRunner.class)
public class ServiceTest {
  @Rule
  public ExpectedException thrown = ExpectedException.none();
  @Mock
  private Repository repository;
  @InjectMocks
  private Service service;

  @Test
  public void save_userValid_saved(){
    User user = new User("id", "name");

    when(repository.save(user)).thenReturn(user);

    User userResult = service.save(user);

    assertThat(userResult).isEqualTo(user);
  }

  @Test
  public void save_userNotValid_throwException(){
    thrown.expect(IllegalArgumentException.class);
    thrown.expectMessage("User is not valid");

    service.save(null);
  }
}

Well, we added interesting changes in our unit tests:

  • We changed the methods names, to describe better its intent, using the when -> given -> then format, so, we can read the method name and understand what business logic is testing.
  • We added a new unit tests to stress the invalid User case, so, we complement our tests suite.

NOTE: The format when -> given -> then is based on Behavior Driven Development, nonetheless, you can use your own format, the important thing here is that you will have docents of unit tests, so, it is wise to name them right.

NOTE2: I know, naming things in software is one of the most difficult part of our work, but, it is unavoidable.

Okey, let’s run our unit tests set:

Test fails

NOTE: See there how the names are important? if some unit test fail, the name will give you clues regarding what could fail.

As we added a new unit tests regarding what should happen when the User is invalid, that test fails, so, we need to fix the production code (Service Class).

class Service {
  private final Repository repository;

  public Service(Repository repository) {
    this.repository = repository;
  }

  public User save(User user){
    if(isValid(user)){
      return repository.save(user);
    }

    throw new IllegalArgumentException("User is not valid");
  }

  private boolean isValid(User user) {
    return Objects.nonNull(user);
  }
}

Next, let’s run the tests again:

Tests pass

Now, let’s see how the coverage report behaves:

New coverage for our test set

As we can see, we have now this metrics for Service class:

  • Classes: 100 %
  • Methods: 100 %
  • Lines: 100 %

CAREFUL: Having 100 % coverage is pretty difficult. Here, we accomplish it due to the simplicity of the example, but, you might have legacy code too difficult to test, or some code that doesn’t make sense to stress like hashCode methods. The lower level of 80 % is a pretty good goal.

Using Mutation Tests to Validate Better Tests

Well, we just saw that coverage doesn’t mean quality, so, we should focus on the business logic we want to validate instead of how many lines we must stress.

As developers, we are lazy, and we can fall in the lie of coverage, so, there are some tools that helps us to validate that there are enough quality unit tests that stress multiple branches in our production code.

Mutation tests is a technique, where a tool modify some production code, and later, executes the unit tests defined for that production code. The expected behavior is tests failure, and that means, those tests are stressing well our code.

For instance, a Mutation tool can modify our Service class like this:

class Service {
  private final Repository repository;

  public Service(
          Repository repository) {
    this.repository = repository;
  }

  public User save(
          User user){
    if(!isValid(user)){
      return repository.save(user);
    }

    throw new IllegalArgumentException("User is not valid");
  }

  private boolean isValid(
          User user) {
    return true;
  }
}

It just changed the isValid(user) if, with a negation, so, when the Mutation tool runs the tests, it will get failures as expected behavior:

Tests failures after modifying some production code

For more information regarding Mutation tests, you can check here.

TDD Is A Better Solution

If you need to add Mutation tests to your project, that means, you are afraid about the quality of the tests your developers can do, and that’s sad.

Using TDD is a pretty good option to guarantee quality tests, as we saw when we added the save_userNotValid_throwException first, and later, we fixed the production code.

However, TDD is not always an easy path, so, Mutation tests can be worthy in some contexts.

NOTE: You can see a good example of why to use TDD in the post A Case in Favor of TDD: Transforming an Entity to DTO

Final Thought

Well, we just saw how tests coverage isn’t a good measure regarding quality testing and how this lie can give us troubles.

However, having coverage requirements in your projects is a good start to improve the quality of your tests, but, don’t stop there, move your team to be better with mutations tests and finally, the main goal should be to use TDD as much as possible.

If you liked this post and are interested in hearing more about my journey as a Software Engineer, you can follow me on Twitter and travel together.

3 comments

Leave a comment