Testing – Is Using Helper Methods in Test Cases a Bad Practice?

anti-patternsintegration-testsjunittesting

Consider a test suite like this:

public class MyTestSuite {
   @Test
   public void test_something() {
      testHelperMethod("value11", "value12", "value13");
   }

   @Test
   public void test_something_else_meaningful_name() {
      testHelperMethod("othervalue21", "othervalue22", "othervalue23");
   }

   // ...

   private void testHelperMethod(String value1, String value2, String value3) {
       // set up part (independent of value1, value2, value3)

       // action part (dependent on values)

       // assertion part (dependent on values)

       // tear down part (independent of values)
   }
}

In other words, all the test cases are executed via a single, parametrized helper method, which is structured according to the arrange-act-assert schema. The test cases just call this method with various parameters, according to what exactly needs to be tested.

Question: what is the disadvantage, if any, of structuring the tests like this, and does this (anti?)-pattern have a common name?

Remarks

  1. I tried to Google for it for a long time, including on SO and
    on the blog, but could not find anything useful till now.

  2. This question is the closest I found, but the answers there address other aspects/problems, namely:

    • assertions/setup code intermixed (not the case here, as all
      assertions are at the end of the helper method)
    • more assertions per test (also the case in my example, but I think this is an unrelated issue)
    • responsibility of helper method is not clear: I think in my
      case it is clear, just it is different from the 'traditional' one

  3. To be more precise, the assertion part in testHelperMethod is also a separate utility method (called with many-many parameters), but I guess that does not matter much for the question. (I.e.: why is it bad practice to delegate testing to helper methods, if at all?)

  4. In case this matters, this is an integration test, but I suspect that the answer would be the same for unit-tests as well.

EDIT: Changed the names of the test cases to avoid confusion (Previously, they were called test1, test2. In fact, the tests in the project in questions do have meaningful names, I had just made this simplification myself.)

Best Answer

As others have already stated: nothing wrong with breaking out a separate method to avoid duplication.

However, as others have also stated: you do want each individual test to clearly indicate what it is setting up and what it is asserting.

Test names will help with that enormously and I assume your actual code has more meaningful names than Test1 and Test2.

To make the code of the individual tests more "intelligible" - ie make it clearer what each test is doing just by reading the code of that test, you can break up your helper method into its obvious parts.

Yes, it will "duplicate" what would otherwise be confined to the single helper as you will now have a number of calls in each test when you could do it with one, but it allows you to give more meaningful names to each helper method.

// Arrange
someHelper = MakeSomeDependency(arg1, arg2, arg3);
sut = MakeSomeObjectUnderTest(someHelper);

// Act
sut.DoSomething();

// Assert
AssertCommonAsserts(sut, expectedvalue1, expectedvalue2);
Assert.AreEqual(sut.SomeProperty, expectedValue);

Note: I am not a fan of multiple asserts in a single test, but there are cases when a single "fact" can only be checked by multiple asserts and I can also think of a number of scenarios in which having some "guard" asserts before the actual "fact" being asserted can help in debugging failures.

If you use a language that supports named parameters, you can make your tests even more intelligible by using that feature so the method call doesn't just convey the values used, but also what those values are used for.

// Arrange
someHelper = MakeSomeDependency(
    knownNames: arg1, 
    unknownNames: arg2, 
    lookingFor: arg3);
sut = MakeSomeObjectUnderTest(someHelper);

// Act
sut.DoSomething();

// Assert
AssertCommonAsserts(
    objectUnderTest: sut, 
    numberOfItemsFound: expectedvalue1, 
    nameOfItemFound: expectedvalue2);
Assert.AreEqual(sut.SomeProperty, expectedValue);
Related Topic