That Can Not Be Tested!: Spring Cache and Retry

"Is it working now?" asked the Product Owner. "Well... I hope so. You know this bug, it can not really be reproduced locally, therefore I can not really test if it works now. The best I can do is deploy it to prod and wait." 

The answer did not make the Product Owner particularly happy, but he also knew the bug appears only when an API called by his application has a quick downtime exactly at the time when the user clicks on a specific button.

The daily stand-up, which was the environment of the small conversation, went on, and nobody wanted to dedicate much time or attention to the bug mentioned - except for Jack, the latest addition to the team, who was concerned about this "hit deploy and roll the dice" approach. Later that day, he actually reached out to Bill - the one who fixed the bug.

"Can you tell me some details? Can not we write some unit tests or so?"

"Well, we can not. I actually did not really write much code. Still, I have strong faith, because I added @Retryable to ensure the API call is being re-tried if it fails. What's more, I added @Cacheable to reduce the amount of calls fired up against the API in the first place. As I said in the daily, we can not really test it, but it will work on prod." 

With that Bill wanted to close this topic and focus on the new task he picked up, but Jack was resistant:

"I would still love to have automated tests on that," stated Jack.

"On what? You should not unit-test Spring. Those guys know what they are doing."

"Well, to be honest, I am not worried about Spring not working. I am worried about us not using it the right way." 

The Challenge

This is the point: when we can abandon Jack and Bill, as we arrived at in the main message of this article, I have seen the following pattern multiple times. Someone resolves an issue by utilizing some framework-provided, out-of-the-box functionality. In many cases, it is just applying an annotation to a method or a class and the following sequence happens:

1. The developer argues there is nothing to write automated tests for, as it is a standard feature of the framework that is being used.
2. The developer might or might not at least test it manually (and like in the example above, the manual test might happen on a test environment, or might happen only on a prod environment).
3. At some point, it breaks, and half of the team does not know why it is broken, the other half does not know why it used to work at all.

Of course, this scenario can apply to any development, but my observation is that framework-provided features (such as re-try something, cache something, etc.) are really tempting the developers to skip writing automated tests. On a side note, you can find my more generic thoughts on testing in a previous article.

Of course, I do not want to argue for testing a framework itself (although no framework is bug-free, you might find actual bugs within the framework). But I am strongly arguing for testing that you are using the framework properly.

In many cases it can be tricky, therefore, in this tutorial, you will find a typically hard-to-test code, tips about how to rework and test it, and the final reworked version of the same code.

Code That Is Hard To Test

Take a look at the following example:

@Slf4j
@Component
public class Before {

    @Retryable
    @Cacheable("titlesFromMainPage")
    public List<String> getTitlesFromMainPage(final String url) {
        final RestTemplate restTemplate = new RestTemplate();

        log.info("Going to fire up a request against {}", url);
        final var responseEntity = restTemplate.getForEntity(url, String.class);

        final var content = responseEntity.getBody();

        final Pattern pattern = Pattern.compile("<p class=\"resource-title\">\n(.*)\n.*</p>");
        final Matcher matcher = pattern.matcher(content);

        final List<String> result = new ArrayList<>();
        while (matcher.find()) {
            result.add(matcher.group(1).trim());
        }

        log.info("Found titles: {}", result);
        return result;
    }
}

It is fair to say it is tricky to test. Probably your best shot would be to set up a mock server to respond to your call (for example, by using WireMock) as follows:

@WireMockTest
public class BeforeTest {

    private static final Before BEFORE_INSTANCE = new Before();

    @Test
    public void testCall(final WireMockRuntimeInfo wmRuntimeInfo) {
        stubFor(get("/test-url").willReturn(ok(
                "<p class=\"resource-title\">\nFirst Title\n.*</p><p class=\"resource-title\">\nOther Title\n.*</p>")));

        final var titles = BEFORE_INSTANCE.getTitlesFromMainPage("http://localhost:"+wmRuntimeInfo.getHttpPort()+"/test-url");
        assertEquals(List.of("First Title", "Other Title"), titles);

    }
}

Many of the average developers would be happy with this test. Especially, after noticing, that this test generates 100% line coverage. And some of them would entirely forget to

•  add @EnableCaching ...
• ... or add @EnableRetry ...
• ... or to create a CacheManager bean

That would not only lead to multiple rounds of deployment and manual testing, but if the developers are not ready to admit (even to themselves) that it is their mistake, such excuses like, "Spring does not work," are going to be made. 

Let’s Make Life Better!

Although my plan is to describe code changes, the point is not only to have nicer code but also to help developers be more reliable and lower the number of bug tickets. Let's not forget, that a couple of unforeseen bug tickets can ruin even the most carefully established sprint plans and can seriously damage the reputation of the developer team. Just think about the experience that businesses have:

• They got something delivered that does not work as expected
• The new (in progress) features are likely not to be delivered on time because the team is busy fixing bugs from previous releases.

Back to the code: you can easily identify a couple of problems like the only method in the class is doing multiple things (a.k.a., has multiple responsibilities), and the test entirely ignores the fact that the class is serving as a Spring bean and actually depending on Spring's features.

1. Rework the class to have more methods with less responsibility. In cases you are depending on something brought by annotations, I would suggest having a method that serves only as a proxy to another method: it will make your life seriously easier when you are writing tests to find out if you used the annotation properly.
2. Step 1 probably led you to have one public method which is going to be called by the actual business callers, and a group of private methods (called by each other and the public method). Let's make them default visibility. This enables you to call them from classes that are in the same package - just like your unit test class.
3. Split your unit test based on what aspect is tested. Although in several cases it is just straightforward to have exactly one test class for each business class, nobody actually restricts you to have multiple test classes for the same test class. 
4. Define what you are expecting: for example, in the test methods, when you want to ensure that retry is working, you do not care about the actual call (as for how the business result is created, you will test that logic in a different test anyway). There you have such expectations as: if I call the method and it throws an exception once, it will be called again. If it fails X times, an exception is thrown. You can also define your expectations against cache: you expect subsequent calls on the public method to lead to only one call of the internal method.

Final Code

After performing Steps 1 and 2, the business class becomes:

@Slf4j
@Component
public class After {

    @Retryable
    @Cacheable("titlesFromMainPage")
    public List<String> getTitlesFromMainPage(final String url) {
        return getTitlesFromMainPageInternal(url);
    }

    List<String> getTitlesFromMainPageInternal(final String url) {
        log.info("Going to fire up a request against {}", url);
        final var content = getContentsOf(url);
        final var titles = extractTitlesFrom(content);
        log.info("Found titles: {}", titles);
        return titles;
    }

    String getContentsOf(final String url) {
        final RestTemplate restTemplate = new RestTemplate();
        final var responseEntity = restTemplate.getForEntity(url, String.class);
        return responseEntity.getBody();
    }

    List<String> extractTitlesFrom(final String content) {
        final Pattern pattern = Pattern.compile("<p class=\"resource-title\">\n(.*)\n.*</p>");
        final Matcher matcher = pattern.matcher(content);

        final List<String> result = new ArrayList<>();
        while (matcher.find()) {
            result.add(matcher.group(1).trim());
        }
        return result;
    }
}

On a side note: you can, of course, spit the original class even to multiple classes. For example:

• One proxy class which only contains @Retryable and @Cacheable (contains only getTitlesFromMainPage method)
• One class that only focuses on REST calls (contains only getContentsOf method)
• One class that is responsible for extracting the titles from HTML content (contains only extractTitlesFrom method)
• One class which orchestrates fetching and processing the HTML content (contains only getTitlesFromMainPageInternal method)

I am convinced that although in that case, the scope of the classes would be even more strict, the overall readability and understandability of the code would suffer from having many classes with 2-3 lines of business code.

Steps 3 and 4 lead you to the following test classes:

@ExtendWith(MockitoExtension.class)
public class AfterTest {

    @Spy private After after = new After();

    @Test
    public void mainFlowFetchesAndExtractsContent() {
        doReturn("contents").when(after).getContentsOf("test-url");
        doReturn(List.of("title1", "title2")).when(after).extractTitlesFrom("contents");

        assertEquals(List.of("title1", "title2"), after.getTitlesFromMainPage("test-url"));
    }

    @Test
    public void extractContent() {
        final String htmlContent = "<p class=\"resource-title\">\nFirst Title\n.*</p><p class=\"resource-title\">\nOther Title\n.*</p>";
        assertEquals(List.of("First Title", "Other Title"), after.extractTitlesFrom(htmlContent));
    }
}

@WireMockTest
public class AfterWireMockTest {

    private final After after = new After();

    @Test
    public void getContents_firesUpGet_andReturnsResultUnmodified(final WireMockRuntimeInfo wmRuntimeInfo) {
        final String testContent = "some totally random string content";
        stubFor(get("/test-url").willReturn(ok(testContent)));
        assertEquals(testContent, after.getContentsOf("http://localhost:" + wmRuntimeInfo.getHttpPort() + "/test-url"));
    }

}

@SpringBootTest
public class AfterSpringTest {

    @Autowired private EmptyAfter after;
    @Autowired private CacheManager cacheManager;

    @BeforeEach public void reset() {
        after.reset();
        cacheManager.getCache("titlesFromMainPage").clear();
    }

    @Test
    public void noException_oneInvocationOfInnerMethod() {
        after.getTitlesFromMainPage("any-test-url");
        assertEquals(1, after.getNumberOfInvocations());
    }

    @Test
    public void oneException_twoInvocationsOfInnerMethod() {
        after.setNumberOfExceptionsToThrow(1);
        after.getTitlesFromMainPage("any-test-url");
        assertEquals(2, after.getNumberOfInvocations());
    }

    @Test
    public void twoExceptions_threeInvocationsOfInnerMethod() {
        after.setNumberOfExceptionsToThrow(2);
        after.getTitlesFromMainPage("any-test-url");
        assertEquals(3, after.getNumberOfInvocations());
    }

    @Test
    public void threeExceptions_threeInvocationsOfInnerMethod_andThrows()  {
        after.setNumberOfExceptionsToThrow(3);
        assertThrows(RuntimeException.class, () -> after.getTitlesFromMainPage("any-test-url"));
        assertEquals(3, after.getNumberOfInvocations());
    }

    @Test
    public void noException_twoPublicCalls_InvocationsOfInnerMethod() {
        assertEquals(0, ((Map)cacheManager.getCache("titlesFromMainPage").getNativeCache()).size());
        after.getTitlesFromMainPage("any-test-url");

        assertEquals(1, after.getNumberOfInvocations());
        assertEquals(1, ((Map)cacheManager.getCache("titlesFromMainPage").getNativeCache()).size());

        after.getTitlesFromMainPage("any-test-url");

        assertEquals(1, after.getNumberOfInvocations());
        assertEquals(1, ((Map)cacheManager.getCache("titlesFromMainPage").getNativeCache()).size());
    }

    @TestConfiguration
    public static class TestConfig {
        @Bean public EmptyAfter getAfter() { return new EmptyAfter(); }
    }

    @Slf4j
    public static class EmptyAfter extends After {
        @Getter private int numberOfInvocations = 0;
        @Setter private int numberOfExceptionsToThrow = 0;

        void reset() {
            numberOfInvocations = 0;
            numberOfExceptionsToThrow = 0;
        }

        @Override
        List<String> getTitlesFromMainPageInternal(String url) {
            numberOfInvocations++;

            if (numberOfExceptionsToThrow > 0) {
                numberOfExceptionsToThrow--;
                log.info("EmptyAfter throws exception now");
                throw new RuntimeException();
            }

            log.info("Empty after returns empty list now");
            return List.of();
        }
    }
}

Note that the usage of various test frameworks is separated: the class that actually tests if Spring features are used correctly has SpringRunner, but is not aware of WireMock and vice-versa. There is no "dangling" extra configuration in the test classes, which is used only by a fraction of the test methods in a given test class.

On a side note to AfterSpringTest: Usage of @DirtiesContext on the class could be an alternative to manually resetting the cache in reset() method, but doing the clean-up manually is a more performant way. My advice on this question is: 

• Do a manual reset if the scope of what to reset is small (this is normally the case in unit tests).
• Reset the beans by annotation if many beans are involved or cleaning the context would require complex logic (this is the case in many integration and system tests).

You can find the complete code on GitHub.

Final Thoughts

After all the reworking and creating extra test classes, what would happen now if you delete @EnableRetry or @EnableCaching from the configuration? What would happen if someone would delete or even modify @Retryable or @Cacheable on the business method? Go ahead and try it out! Or trust me when I say unit tests would fail. And what would happen if a new member joins the team to work on such code? Based on the tests, he would know what is the expected behavior of the class.

Tests are important. Quality tests can help you to produce code that others can better understand, can help you to be more reliable, and identify bugs faster.

Tests can be tricky, and tests can be hard to write. But never forget, that if someone says, "That can not be tested," in the overwhelming majority of cases, it only means "I don't know how to test it and not caring to figure it out."