0

We have a piece of code that decorates an interface to transparently add retry logic.

Inversion of Control configuration

service.AddOurRestApiClient()
   .AddResilienceHandler("Retry", builder => // decorate our REST API client with retry handler
   {
      // Add retry policy so request is retried when a
      // transient error occurs such as HTTP 5XX.
      builder.AddRetry(new HttpRetryStrategyOptions
      {
         MaxRetryAttempts = resilienceOptions.HttpMaxRetryTimes,
         Delay = resilienceOptions.HttpRetryWait
      });

      // builder.AddCircuitBreaker(...) - might add in the future
   });

Is AddResilienceHandler worth testing?

I don't think that testing behavior makes any sense because of the complexity. Testing behavior will become even harder if we add more resilience logic such as rate limiting and circuit breaker.

I could instead test that AddResilienceHandler was added and configured, but is it worth the effort? I feel that the configure code is so straightforward that it's not worth the effort.

However without any automated test, there is no automated way to ensure that resilience logic was added.

Would you test AddResilienceHandler code? If yes, how?

What if I'm practicing TDD?

2
  • It is pointless to write tests when you do not see purpose to writing them. Whether the better move is to learn why writing tests would be useful, or to decide to not spend time testing it; is a matter of opinion and risk analysis for what that means for your codebase going forward. No one can make you write tests if you don't see the value of doing so.
    – Flater
    Commented Mar 7 at 5:59

3 Answers 3

3

Is AddResilienceHandler worth testing?

Absolutely. But testing can be as simple as reading the code. Were you thinking of automated testing?

I don't think that testing behavior makes any sense because of the complexity.Testing behavior will become even harder if we add more resilience logic such as rate limiting and circuit breaker.

Well sure if you do it wrong.

I could instead test that AddResilienceHandler was added and configured,

No no. Instead test that when AddResilienceHandler is added (and configured by your test) it does what it's supposed to do. Don't use the test to demand that it get used. Just that it works when it is.

Now, if you have a requirement to retry that's different. But that requirement has no business demanding that AddResilienceHandler is involved. Just test that retries happen. However they got configured.

When testing behavior seek out the logic that is making decisions. That code needs thorough boundary checking. When testing configuration just make sure things are plugged in correctly. Don't get these confused and try to do both at the same time. It's extra effort and locks down things that shouldn't be.

TDD is one small part of testing. Just because something doesn't fit TDD doesn't mean it doesn't deserve a test. Heck, getting it to compile is passing a test. So is simply running it and trying it.

Don't make it an automated test without doing the work to keep it from becoming a brittle test no one understands that forces people to drag around bad code because they're too afraid to delete the test.

You're right to be concerned about the circuit breaker code. Your tests should gracefully allow that to come and go without breaking too many of them. TDD tests can do their own configuring and so ignore it. A correct integration test driven by a requirement wont care about it unless you're breaking the requirement.

Change happens. Write the test that people will know when to delete. Or don't write one at all.

How do I determine which code is worth testing?

All code is worth testing. But let's assume you mean automated testing. An automated test can be worthwhile if it shows:

  • That the code under test can be trusted
  • How to use the code under test

If you don't have these needs for this code you may not need this test.

Configuration code, free of behavior logic, is often skipped when it comes to automated tests. This works best when the configuration code is simple, obvious, and boring. When it can be trusted just by reading it. Interesting code is what needs the most testing.


I don't understand your answer.

Alright, lets try going through this the other way around.

So do you consider my code snippet to be configuration code or not?

It's clearly configuration code.

I consider it to be a configuration code which you said can be skipped,

Yes it can be skipped. If you already trust it and people will understand how to use it as is. The more readable it is the less it needs automated testing.

but your first few paragraphs state that I should test (presumably automated test) the behavior. – LostInComputer

Again, testing can be as simple as reading the code.

If you decide that it needs more testing than a code review even then you shouldn't expect to test it the same way you would if it were behavior code. There's no logic here. No boundaries to test. No if branches to cover. But you still could write an automated test. If you do, separate the retry requirement from the AddResilienceHandler. The retry requirement test shouldn't know if AddResilienceHandler was used. And the test of AddResilienceHandler should do it's own configuring.

What that gives you is a way to know if the current configuration works (as far as the requirement is concerned) and a regression test that tells you if something broke the old way of configuring it. All this without locking you into any need to keep using AddResilienceHandler if you change your mind. If you drop it you delete the one test that knew about it. You keep the requirement test that didn't.

I don't point that out to make you feel you must do these two tests. I point that out to show you what you could get out of creating automated tests for this. How flexible they could be. How much work that will be. And how different those kinds of tests should be from the more common functional core behavior tests.

Only once you understand all that will it be clear if this is worth it. If it's not, skip it.

10
  • AddResilienceHandler and everything in it comes from a Microsoft library. Are you suggesting that if I configure it to have rate limiter, that I should write an automated test that sends a lot of requests to test that there the REST API client has been configured to have a rate limiter? Commented Mar 7 at 7:26
  • I guess my point is I don't see a good reason to write a complicated automated "behavior" test to ensure that I have configured my REST API client with the correct AddResilienceHandler settings Commented Mar 7 at 7:42
  • @LostInComputer better now? Commented Mar 7 at 8:14
  • 1
    "reading the code" is not testing under any definition I've ever seen.
    – pjc50
    Commented Mar 7 at 11:09
  • 1
    @candied_orange source: you made it up? That's far too broad to be useful and covers all sorts of things that are not normally considered testing
    – pjc50
    Commented Mar 7 at 14:47
1

Sure it is worth testing. Maybe it is not worth to be tested automatically, but at the latest when the system is running productively, the code will be "tested" by the end user. Also, I guess you will also run a few manual tests with that code before you set it life into production.

You have to think what will happen in the worst when that code breaks at production stage:

  • are there are just a few pennies at stake, because the system runs a little slower?

  • are there some users annoyed because when the connection gets down, they have to retry a reconnect manually?

  • or: are lifes at stake or some huge costs when the retry mechanics is configured wrongly?

  • are their organizational fail-safe mechanism installed (like an admin or devop who can downgrade the system to the latest working version in case developers are not available to fix a bug in this configuration)?

This is called a risk analysis, and that should be the criterion on which you decide whether it is worth to invest time more implementing a more-or-less complex automated test, or not.

In a typical real-world case, this is not just an "all-or-nothing" decision between "no automated tests at all" or testing specifically AddResilienceHandler. Maybe there are some end-to-end tests in place, or some run-time checks, or some other mechanism which prevents the shutdown of the system even when a bug slips into the decoration code which makes the REST API unusable. These things can mitigate the risks of missing unit tests, or missing integration tests a lot.

So in short: you do not "determine which code is worth testing" just by looking at the code and how hard it is to test. You "determine which code is worth testing" by looking at the context and surroundings, and not just the technical ones, but also the organizational measures.

0

I am really surprised by the given answers. In my opinion, there is absolutely no point in testing builder.AddRetry() method, since it is not developed at this point. This method is part of a library, and the guys who developed the library should have tested it. This is like testing if a foreach loop goes through all the items in a list. The rest variables are constants, and on top of that, there is Zero calculation in this method. Sure you can test it, if you don't have any personal life ..

4
  • 2
    I am also really surprised - because you cannot imagine there is room for errors in the posted code. Or room for regressions when the code is refactored and evolved. The questions was not if it is worth to test AddRetry, but if it is worth to test the whole AddResilienceHandler snippet.
    – Doc Brown
    Commented Mar 7 at 21:41
  • 1
    Indeed. What could go wrong? The only scenario is if someone changes the HttpMaxRetryTimes and HttpRetryWait and this someone should definitely know what he/she is doing before changing any of these configurations. I just can't see any point in testing any code that has no calculation logic in it. Commented Mar 7 at 22:04
  • 2
    @DimitarBoychev I'm really surprised you insist on taking reasonable advise like focus on interesting code to an absurd extreme like only test calculation code. No, sometimes it's also nice to know if the DB server was powered on. Commented Mar 8 at 1:12
  • 1
    Well, investing effort in testing is good thing, I agree on that. But spending too much effort when it is not needed, that's bad. Commented Mar 9 at 18:39

Not the answer you're looking for? Browse other questions tagged or ask your own question.