Is Assert-ing on tearDown (@After) method wrong?

Question

I have multiple test cases even and if the logic is different, the output must be equal on all of them. So I was thinking in how to generalize them and place the Assert method only once.

Is there any way better to do it than this one:

static public class Tests() {

    private static String expected = null;
    private String actual = null;

    @BeforeClass
    public static void setUpBeforeClass() throws Exception {
        expected = new String("My Desired Output");
    }

    @Before
    public void setUp() {
        actual = new String();
    }

    @Test
    public void test1() throws Exception {
        actual = ...
    }

    @Test
    public void test2() throws Exception {
        actual = ...
    }

    @After
    public void tearDown() throws Exception {
        assertThat(actual, is(equalTo(expected)));
    }

    @AfterClass
    public static void tearDownAfterClass() {
    }
}

Running method:

@Test
public void runTests() {
    Result result = JUnitCore.runClasses(Tests.class);
    assertThat(result.getRunCount(), is(2));
    assertThat(result.getFailureCount(), is(0));
}

score 8 · Accepted Answer · answered Sep 14 '10 at 09:29

8

Yes, asserting in the tearDown method is a bad idea. This method exists, according to the JUnit documentation, to

Tears down the fixture, for example, close a network connection. This method is called after a test is executed.

I think that storing your expected and actual values in the test class are a bad idea in general. These variables are test-dependent, so store them inside your test case and do your assert in the test case. For example:

public class FooTest {

    @Test
    public void testFoo() {
        Object expected = // ...
        Object actual = // ...

        assertThat(actual, is(equalsTo(expected)));
    }

}

Also, I see in your code that all test have the same expected value. It might be a good idea to vary your tests so returned values are always different. Testing only one expected value all the time make you sure the code works for this expected result. Try with some more, possibly very different, and try to test some corner cases.

answered Sep 14 '10 at 09:29

Vivien Barousse

20,555
2
63
64

The class I'm testing is kind of a builder class, so I can build my output using different methods. Doing these tests I can assure that all my methods are working and the output is generated properly. I already detected a bug in one of my methods. – Alexander Sep 14 '10 at 09:42
In addition, using statics in ur test is a bad idea. – emory Sep 14 '10 at 09:43
@emory, you mean asserting against an static variable or using static methods or both? – Alexander Sep 14 '10 at 09:45
@Alexander: In this case, try other methods with other values, that will generate a different output. You method can work fine for a given output and be broken for another, so testing different values is always a good idea, it broadens your tests cases. – Vivien Barousse Sep 14 '10 at 09:45
@Alexander if u do as Vivien suggests then this won't be an issue at all. if u have static expected variable, then the tests are not really independent of each other. – emory Sep 14 '10 at 09:51
I agreed with the statement that the test cases must be independent from each other. – Alexander Sep 14 '10 at 10:03

score 5 · Answer 2 · edited Mar 14 '13 at 10:30

5

If you must generalize then you could create one method like

private void testIt ( String actual ) {
    assertThat(actual, is(equalTo(expected)));
}

and call it from all your test methods.

If and when a test fails it will be more obvious which test failed.

edited Mar 14 '13 at 10:30

Alexander

23,432
11
63
73

answered Sep 14 '10 at 09:28

emory

10,725
2
30
58

score 1 · Answer 3 · answered Jul 22 '21 at 10:23

My interpretation of the existing answers is that you shouldn't use tearDown for assertions, as a matter of clarity:

I agree with this answer that Assert from a general method called explicitly from each method will make it clearer which test failed.
And I agree with that answer that assert in tear down goes against the original intention of the teardown infrastructure. Using assert this way makes the intent of our code less clear.

But I thought it was worth explicitly mentioning what may have also been implicit in that answer, which is that as well as potentially causing confusion, assertions in tearDown may actually break your test logic. This is more clearly demonstrated in this related Python answer:

Asserting something in your tearDown means that you need to be careful that all the cleaning is done before the actual asserting otherwise the cleaning code may not be called if the assert statement fails and raises.

Is Assert-ing on tearDown (@After) method wrong?

3 Answers3

Linked