07 June 2017

Loops in Tests, BAD!

Elsewhere I've mentioned my distain for loops in tests (along with Parameterized Tests). I wanted to put together a quick note on loops in tests just to get it out there. 

Loops in tests are sloppy!

But lets be clear, I'm not talking about a setup method that populates data into a table or something, those make sense as long as they are well written and clear. I'm talking about tests that loop over an execution to make assertions. 

I think we've all done it at some point. I know when I was learning TDD I didn't think anything of it. But I eventually learned the hard way. My compact little test that had a loop in it was clean, clear, and fast. Reading that test gave you a satisfying feeling that it would prove the code valid quickly and without concern. 

That was true until I broke the code!

The specific test was pretty high level, it had about six lines of setup and then a simple loop that pushed numbers in a long range through a function. I had an verification calculation that ensured given any number in that range what the correct answer was with a nice epsilon value. However the calculation was pretty complicated and its specification came from engineering. It wasn't entirely incomprehensible, however it was a bit weird. Anyway, we got an adjustment to the calculation and we thought we understood it pretty well. So we made the modification to the module and all was well. According to its own tests everything was super-green

Then we plugged our new component in and this test blew up. First thing we did was ensure that we'd adjusted the verification calculation and that all seemed to check out. We still had failing cases. As it turned out, not all of them failed though. The first hour we spent reducing and adjusting the input range trying to see if it was a particular case, or on the tail of some curve. But we couldn't get it to budge. We then called the engineers and they came and did the engineer thing. But they couldn't see the issue either. After another 2 hours we decided to trap the failed assertions and list them rather than pick away one by one like we'd been doing.

What we eventually saw was a small fluctuation in the calculation that we hadn't correctly anticipated in our verification function. The change to the module cause a module twice removed to put a 'tremor in the signal' and we didn't adjust for that correctly. So roughly one in a one-hundred tests passed and the rest failed by varying multiples of our epsilon value. 

Once we saw this issue (there were 5 of us by this point) the engineers were able to confirm that the variance was what they could tolerate and helped us adjust our verification calculation. We then patched our test back up and went back to green. 

This was a terrible choice. The same test broke a few months later and we went through similar (though accelerated) gyrations to fix the issue, and again, it was something we could have avoided. We didn't learn and we put things back into that darned loop. 

What we should have done, what we eventually learned was, first you have to make sure your adjusting the verification correctly (sometimes really hard to do) and second, don't use a loop! The loop was causing us pain. The loop was making harder to see the issue. A bunch of individual tests (even if there were a few hundred) would have lit up the build with a big arrow that screamed (metaphorically) 'Hey Dummy! There is Tremor in the Signal. Check module 2'. And it would have done so explicitly, or at least more explicitly that what we were doing, which was relying on tribal knowledge of our prior experiences. 

Now I've run into tests elsewhere where loops are used that are vastly less complicated that the aforementioned project. Less complicated doesn't mean necessarily easier to grasp in the case of the insidious loop, it means easier to fix once you spot it. Sloppy Loops in your test suggest that you are too lazy to put forth the effort to explain your code in test clearly. Rather you say, 'if all this works, we're good, no need to explain'. That reeks of unprofessionalism to me. 

So I guess my point is, never put a loop in a test case.