8

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.)

  • 3
    I think it's fine to reduce duplication this way, however, in my opinion, reading the test alone should still give clear indication of what the test is doing. Also, most frameworks have a setup and teardown functionality in it, any reason why you cannot leverage those instead? – Matthew Mar 9 '16 at 18:20
  • ... and if you're doing different setups and tear downs for different sets of tests within a class, put them in separate classes. – user40980 Mar 9 '16 at 19:17
  • I am curious to know how are you handling assertion part in your helper. For tests, intended output is fixed for the given input, and can be easily tested with parameterized tests. If you are doing something dramatically complex in the test helper assertion, then I would say it indicates a code smell in your actual method being tested. – wonderbell Mar 9 '16 at 20:48
  • RE setup/teardown: I am not sure why not use the special methods (I did not write this code, found it in the project I'm working on) -- probably it can be refactored in that way RE assertions: both the expected and the actual values are parameters, passed to the assertion helper method (see Remark 3.) – Attilio Mar 10 '16 at 9:22
  • Abstracting functions and setup functions in tests is not a bad thing. However reading a test function should clarify what it is testing. Even reading only the test name should indicate this. Furthermore your example could probably be solved better with a dataProvider – Thijs Riezebeek Mar 10 '16 at 9:23
3

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);
  • My question might have been confusing in one point: actually the tests are not named test1 and test2, I just put those names to imply that names do not really matter. Since this caused confusion, I will adapt the question. Also, I like your point that even by splitting up the helper method, and duplicating code, the tests become more intelligible. In other words, if I understood correctly, having the helper methods is not an "anti-pattern", but rather a trade-off between intelligibility and code duplication. – Attilio Mar 13 '16 at 19:43
  • @Attilio Exactly. And from my point of view intelligibility trumps duplication-avoidance. Also DRY is a much broader concept than simply avoiding code duplication. It is not so much about avoiding code duplication as it is about avoiding knowledge (business rules, algorithms) repetition. – Marjan Venema Mar 14 '16 at 6:58
1

There is nothing wrong with structuring your test code as well as your business code. The point of a test suite is that it expresses clearly how the tested code is supposed to behave, and that it signals a failure if it doesn't. With judicious refactoring and well-chosen names in your test code, you can serve this goal better than with cut-and-paste drudgery.

(However, Matthew is quite right: set-up and tear-down code that is common to all tests belongs into the framework's special methods, not into your custom subroutines.)

0

You should absolutely factor out common code in unit tests. Messy and badly-structured unit test code just leads to tests which are difficult to understand and difficult to maintain, no matter how beautiful the code they're testing is.

That said, in this particular case if you were using the .NET test framework NUnit this wouldn't be necessary at all - you'd turn the helper into a test method and use the TestCase attribute to get the framework to call it multiple times with different parameter sets. I'd be surprised if there wasn't something equivalent available for Java and other languages.

So do structure unit tests well, but also make sure you've learned the capabilities of the test framework because it might be able to help you.

0

You should consider using a parameterized Runner using a DataProvider for all valid input for your particular test cases, instead of your upper aproach. I use helper methods only to increase readability of tests, if they have a complex setup in the test and which would clutter the arrange part of the test.

What it could look like is:

@RunWith( DataProviderRunner.class )
public class YourClassTest {

    @Test
    @DataProvider( value = { "value11|value12|value13","value21|value22|value23" }, 
                   trimValues = true, splitBy = "\\|" )
    public void test_something(String value1, String value2, String value3) {
       // action part (dependent on values)

       // assertion part (dependent on values)
    }


    @Before
    public void independendSetup() {
       // set up part (independent of value1, value2, value3)
    }

    @After
    public void independendtearDown() {
       // tear down part (independent of values)
    }
}

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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