From Legacy Code to Testable Code: Overcoming Redundant Validations and Mutation Testing Challenges

Achieving 100% test coverage is often seen as a hallmark of software quality. However, our experience with the Drink Water API project taught us that reaching full coverage is not just about writing more tests—it’s about designing code that is inherently testable. In our journey to improve testability and kill all mutants with Pitest, we encountered overlapping validations between our mapper and our domain entity. This article explores how we identified and resolved these issues by refactoring our code and tests, and it explains the broader lessons learned about code design.

Project Overview

Drink Water API provides a robust set of RESTful endpoints powering a hydration monitoring platform. This API enables client applications to track users' water intake, manage personalized hydration goals, and access detailed consumption analytics. Built with Spring Boot and secured with Keycloak, it supports user profile management, daily water tracking with timestamp precision, flexible filtering, analytics, and more.

For those interested in exploring or contributing to the project, you can access the repository here: Drink Water API Repository

The Overlapping Validations Problem

In our Drink Water API, the WaterIntakeMapper is responsible for converting a WaterIntakeDTO into a domain entity (WaterIntake). Initially, the mapper contained multiple validations:

    public WaterIntake toEntity(WaterIntakeDTO dto, User user, Long id) {

        if (dto == null) {
            throw new IllegalArgumentException("Water intake DTO cannot be null.");
        }

        if (user == null) {
            throw new IllegalArgumentException("User cannot be null.");
        }

        if (id != null && id <= 0) {
            throw new IllegalArgumentException("ID must be greater than zero.");
        }

        if (id != null) {
            return new WaterIntake(
                    id,
                    dto.dateTimeUTC(),
                    dto.volume(),
                    dto.volumeUnit(),
                    user
            );
        }

        return new WaterIntake(
                dto.dateTimeUTC(),
                dto.volume(),
                dto.volumeUnit(),
                user
        );
    }        

At the same time, the WaterIntake constructor in our domain model enforced similar validations:

    public WaterIntake(OffsetDateTime dateTimeUTC, int volume, VolumeUnit volumeUnit, User user) {

        if (dateTimeUTC == null) {
            throw new IllegalArgumentException("Date and time cannot be null");
        }

        if (volume <= 0) {
            throw new IllegalArgumentException("Volume must be greater than zero");
        }

        if (volumeUnit == null) {
            throw new IllegalArgumentException("Volume unit cannot be null");
        }

        if (user == null) {
            throw new IllegalArgumentException("User cannot be null");
        }

        this.dateTimeUTC = dateTimeUTC;
        this.volume = volume;
        this.volumeUnit = volumeUnit;
        this.user = user;
    }        

Because both the mapper and the entity were checking for a null user—and both threw an IllegalArgumentException with the identical message—the tests could not differentiate whether the error came from the mapper or later in the entity constructor. This duplication led Pitest to report a surviving mutant when a conditional in the mapper was removed, as the entity’s validation still produced the same observable behavior.

The Pitest Mutation Issue

Pitest introduces small mutations into the code (such as removing a conditional) and runs the tests to see if the change is detected. In our case, when a mutant removed the null check for the user in the mapper, the WaterIntake constructor still threw an exception with the message "User cannot be null". Since the tests only checked for the exception type and message, the mutant was not "killed"—even though the responsibility for the error had shifted from the mapper to the entity. This situation clearly indicated that our overlapping validations were masking issues in our test suite.

Introducing a Custom Exception

To resolve the ambiguity and make the source of errors explicit, we introduced a custom exception for the mapper:

public class WaterIntakeMapperIllegalArgumentException extends IllegalArgumentException {

    public WaterIntakeMapperIllegalArgumentException(String message) {
        super(message);
    }
}        

We then refactored the mapper to use this custom exception:

    public WaterIntake toEntity(WaterIntakeDTO dto, User user, Long id) {

        if (dto == null) {
            throw new WaterIntakeMapperIllegalArgumentException("Water intake DTO cannot be null.");
        }

        if (user == null) {
            throw new WaterIntakeMapperIllegalArgumentException("User cannot be null.");
        }

        if (id != null && id <= 0) {
            throw new WaterIntakeMapperIllegalArgumentException("ID must be greater than zero.");
        }

        if (id != null) {
            return new WaterIntake(
                    id,
                    dto.dateTimeUTC(),
                    dto.volume(),
                    dto.volumeUnit(),
                    user
            );
        }

        return new WaterIntake(
                dto.dateTimeUTC(),
                dto.volume(),
                dto.volumeUnit(),
                user
        );
    }        

By doing so, errors thrown by the mapper are now clearly distinguishable from those thrown by the entity's constructor. Our tests could now specifically verify that the mapper behaves as expected, ensuring that mutations in the mapper are detected by Pitest.

Refactoring the Tests

We also centralized test data in a constants file (WaterIntakeTestConstants) to simplify our tests:

public final class WaterIntakeTestConstants {

    public static final Long WATER_INTAKE_ID = 5L;

    public static final OffsetDateTime DATE_TIME_UTC = OffsetDateTime.now()
            .withOffsetSameInstant(ZoneOffset.UTC)
            .withSecond(0)
            .withNano(0);

    public static final int VOLUME = 250;

    public static final VolumeUnit VOLUME_UNIT = VolumeUnit.ML;

    public static final WaterIntakeDTO WATER_INTAKE_DTO = new WaterIntakeDTO(DATE_TIME_UTC, VOLUME, VOLUME_UNIT);

    public static final WaterIntake WATER_INTAKE = new WaterIntake(WATER_INTAKE_ID, DATE_TIME_UTC, VOLUME, VOLUME_UNIT, USER);
}        

Our tests were updated to use these constants and check for the new custom exception. For example:

@Test
void givenNullUser_whenToEntity_thenShouldThrowException() {
    assertThatThrownBy(() -> this.mapper.toEntity(WATER_INTAKE_DTO, null))
            .isInstanceOf(WaterIntakeMapperIllegalArgumentException.class)
            .hasMessage("User cannot be null");
}        

And for invalid IDs:

@ParameterizedTest
@ValueSource(longs = {0, -1, -100})
void givenInvalidId_whenToEntity_thenShouldThrowException(Long invalidId) {
    assertThatThrownBy(() -> this.mapper.toEntity(WATER_INTAKE_DTO, USER, invalidId))
            .isInstanceOf(WaterIntakeMapperIllegalArgumentException.class)
            .hasMessage("ID must be greater than zero");
}        

These changes allowed us to eliminate the ambiguity that was causing surviving mutants and improved the overall clarity of our code and tests.

Lessons Learned and Best Practices

This case study reinforces several important points:

1. Testability Reflects Code Design: As highlighted in Clean Code (Martin, 2008) and Working Effectively with Legacy Code (Feathers, 2004), code that is hard to test often suffers from poor design. Overlapping validations are a red flag that responsibilities might be too entangled.
2. Avoid Redundancy in Validations: When the same validation exists in multiple layers (mapper and entity), it can mask errors and confuse mutation testing tools. Our experience showed that using a custom exception in the mapper provides clear separation of concerns.
3. Centralize Test Data: Utilizing a constants file for test data minimizes duplication and ensures consistency across tests. This approach makes tests more maintainable and easier to understand.
4. Clear Error Reporting: Distinguishing errors by using custom exceptions not only improves test clarity but also aids in debugging and logging, as the source of an error becomes immediately apparent.

Conclusion

Achieving 100% test coverage is not merely a numerical target—it reflects the quality and maintainability of your code. Our experience with the Drink Water API's WaterIntakeMapper demonstrated that overlapping validations between the mapper and the domain entity can cause ambiguity in tests and allow mutants to survive in mutation testing. By introducing a custom exception specifically for mapper errors, we clarified the source of errors and improved test effectiveness.

This process, supported by insights from Beck (2003), Feathers (2004), and Martin (2008), underscores that code design directly impacts testability. In our case, resolving the redundancy in validations not only improved our mutation testing results but also led to a more robust, maintainable, and understandable codebase.

For those interested in exploring the project further or contributing to its development, you can download and clone the repository from Drink Water API Repository.

References

• Beck, K. (2003). Test-Driven Development: By Example. Addison-Wesley.
• Feathers, M. (2004). Working Effectively with Legacy Code. Prentice Hall.
• Martin, R. C. (2008). Clean Code: A Handbook of Agile Software Craftsmanship. Prentice Hall.

By aligning our code design with best practices and ensuring each layer has a clear, singular responsibility, we not only achieve better test coverage but also build a foundation for long-term software quality and resilience.