Saturday, March 29, 2014

Testing progress reporting

I was trying to get some code under test.

Originally it had been written as one long method. It was a slow process, so another programmer had added progress reporting. He wisely created a new, decoupled progress reporting service, with an interface like:

Loading IProgress.cs from Gist 9867358

My first step was Compose Method: Applying Extract Method repeatedly until the code read roughly like English.

Loading Example1.cs from Gist 9867358

With the exception of the progress reporting, there's no point in writing a test for a Composed Method. The test would just repeat the code, violating DRY. Test by inspection.

But what about the progress reporting? How do I test it? Some options:

  • Don't test it.
  • Test manually.
  • Use an integration/end-to-end/system test.
  • Mock out the Do*() methods in a unit test.
  • Change the design.

Each option has pros and cons; each has a context in which it is the right choice.

TDD teaches us that difficult-to-test is a code smell; it's design feedback. All other things being equal, I'd rather choose the last option and improve the design.

There are several ways to think about the problem that could lead us to a new design:


  • The number "4" violates DRY with the (often implicit) parameters to ReportWork. How can I test that it stays correct when editing this code in the future?
  • Most difficult-to-test problems are solved by decoupling. What could I decouple here?
  • This method is responsible for both doing the work, and reporting on the work. Can I write those separately?
  • Think about the difficult-to-test aspect of the code; what if I made a new class that did only that, free of any context?

Suppose there was a class that was responsible for calculating & reporting the work to be done. You could just pass it a list of steps, with costs, and let it calculate the sum. The result might look like:

Loading Example2.cs from Gist 9867358

Since lambdas are capable of capturing locals, it's easy to use the output of one step as the input of another:

Loading Example3.cs from Gist 9867358

Consequences

The code is far easier to test:

  • DoWork, etc. are more testable now as separate methods, since they were extracted.
  • The ActiosnWithCostsList class is trivial to unit test. (You might not bother... up to you.)
  • The ExecuteWithProgress extension is non-trivial, but still easy to unit test.
  • The original code is now very simple, and should be tested by inspection.

I would like a lightweight end-to-end test to hit this code, just to make sure I haven't missed anything. Maybe that's just because I'm still new to TDD and haven't developed the confidence in my TDD skills yet.

If you're not used to coding this way, it may feel surprising. Once you understand it, it may seem clever. Those are both feelings I want to avoid in my code. (Well, I do like looking clever, but it's still a code smell.) I believe that any competent C# programmer can spend a few minutes reading my tests and understand what's going on here.

The 4 rules place "passes tests" above "expresses intent", so I think it's worth keeping until we find a better solution.

Next steps

You wouldn't be reporting progress if the operation was fast. Since it's slow, you may want to free up the current thread while waiting. That means async/await. Extend this solution to make that easy.

If you call a method that could report fine-grained progress, it would be nice to pass it a child progress. Something like:

Loading IProgress2.cs from Gist 9867358



No comments: