Roy Osherove

View Original

Two different ways to create bad logic in unit tests

we’re adding more rules to Test Lint, these days, and one of them is called “Production Logic in Test”. It is quite different, and I think even more dangerous than the existing rule “Logic in Test” – because it talks about reproducing logic from within production code inside your unit tests.

What might that look like? here’s a simple example:

   
1 [TestMethod] 2 public void SomeTest() 3 { 4 string user = " user " ; 5 ComponentUnderTest cut = new ComponentUnderTest(); 6 string result = cut.SayHello(user); 7 8 string expected = " hello " + user; 9 Assert.AreEqual(expected,result); 10 11 }

line 8 contains logic that is potentially reproducing logic under test. 99% certainty that the concatenation of the two strings might appear exactly like it does in the test, inside the production code.

so what’s the problem?

  1. If the logic in production code contains a bug, and we are recreating the same logic in the test, we are essentially reproducing the bug as well – we are in fact expecting that bug to occur, so our test will pass because the bug is there.
  2. even if we are not reproducing the bug from production, we might have our own bug in that piece of logic. Our test might fail, but because there is a bug in the expected test value – which we will not try to look for since we assume tests are correct (this would not be true if we were writing this test first!)

How is this rule different than just regular logic in test?

usual logic in tests usually tries to test several outcomes in the same test, or verify different things based on various conditions. a loop in a test will loop and may assert on several things, while an IF statement may exist so we only check something based on certain conditions.

in fact, logic in unit tests can appear in several places, and create the same problem as stated i #2 above:

  • inside a unit test
    • to avoid this, try to test the smallest thing possible that proves your point. for example, do you really need to loop over 10 rows in a memory table, or maybe just one row will do to prove something works or fails?
  • inside a helper method (which also affects the readability of the test that calls that helper method)
    • usually done for the same of reuse – this anti pattern in tests can both create bugs, and lack of readability or magic in the  calling tests. It’s magic if you have to ask yourself as you read the test questions such as “so where is my assert?” or “why am I sending in this string?”
  • inside manual mocks and stubs (which also affects the readability of the test that calls that helper method)
    • usually done for lack of knowledge that this is an anti pattern, people will create mocks and stubs that contains logic that might either reproduce some of the production code, or might make assumptions about what calling unit tests will send as parameters.
    • this leads to lack of readability in the test
    • magic strings or numbers in a test
    • weird bugs
    • many times that logic exists to accomodat ethe different needs of no more than a couple of unit tests that need different behavior from a stub
    • it could easily be avoided by :
      • creating stubs with hardcoded known behavior that is names in the type name of the stub, per each need of a specific unit test
      • having the stubs take “orders” from the tests on how to behave – never to assume anything. for example.