tags:

views:

1876

answers:

7

Should I initialize class fields at declaration like this?

public class SomeTest extends TestCase
{
    private final List list = new ArrayList();

    public void testPopulateList()
    {
        // Add stuff to the list
        // Assert the list contains what I expect
    }
}

Or in setUp() like this?

public class SomeTest extends TestCase
{
    private List list;

    @Override
    protected void setUp() throws Exception
    {
        super.setUp();
        this.list = new ArrayList();
    }

    public void testPopulateList()
    {
        // Add stuff to the list
        // Assert the list contains what I expect
    }
}

I tend to use the first form because it's more concise, and allows me to use final fields. If I don't need to use the setUp() method for set-up, should I still use it, and why?

Clarification: JUnit will instantiate the test class once per test method. That means list will be created once per test, regardless of where I declare it. It also means there are no temporal dependencies between the tests. So it seems like there are no advantages to using setUp(). However the JUnit FAQ has many examples that initialize an empty collection in setUp(), so I figure there must be a reason.

A: 

I prefer readability first which most often does not use the setup method. I make an exception when a basic setup operation takes a long time and is repeated within each test.
At that point I move that functionality into a setup method using the @BeforeClass annotation (optimize later).

Example of optimization using the @BeforeClass setup method: I use dbunit for some database functional tests. The setup method is responsible for putting the database in a known state (very slow... 30 seconds - 2 minutes depending on amount of data). I load this data in the setup method annotated with @BeforeClass and then run 10-20 tests against the same set of data as opposed to re-loading/initializing the database inside each test.

Using Junit 3.8 (extending TestCase as shown in your example) requires writing a little more code than just adding an annotation, but the "run once before class setup" is still possible.

Alex B
+1 because I also prefer readability. However, I'm not convinced that the second way is an optimization at all.
Craig P. Motlin
@Motlin I added the dbunit example to clarify how you can optimize with setup.
Alex B
The database is essentially global state. So moving the db setup to setUp() is not an optimization, it's necessary for the tests to complete properly.
Craig P. Motlin
@Alex B: As Motlin said, this is not an optimization. You are just changing where in the code the initialization is done, but not how many times nor how quickly.
Eddie
I intended to imply use of the "@BeforeClass" annotation. Editing the example to clarify.
Alex B
@BeforeClass changes the semantics of the test - it changes the state. I really need an empty list for each test. So this no longer answers the question.
Craig P. Motlin
+7  A: 

In addition to Alex B's answer.

It is even required to use the setUp method to instantiate resources in a certain state. Doing this in the constructor is not only a matter of timings, but because of the way JUnit runs the tests, each test state would be erased after running one.

JUnit first creates instances of the testClass for each test method and starts running the tests after each instance is created. Before running the test method, its setup method is ran, in which some state can be prepared.

If the database state would be created in the constructor, all instances would instantiate the db state right after each other, before running each tests. As of the second test, tests would run with a dirty state.

JUnits lifecycle:

  1. Create a different testclass instance for each test method
  2. Repeat for each testclass instance: call setup + call the testmethod

With some loggings in a test with two test methods you get: (number is the hashcode)

  • Creating new instance: 5718203
  • Creating new instance: 5947506
  • Setup: 5718203
  • TestOne: 5718203
  • Setup: 5947506
  • TestTwo: 5947506
Jurgen H
Correct, but off topic. The database is essentially global state. This is not a problem I face. I'm merely concerned with execution speed of properly independent tests.
Craig P. Motlin
+2  A: 

I started digging myself and I found one potential advantage of using setUp(). If any exceptions are thrown during the execution of setUp(), JUnit will print a very helpful stack trace. On the other hand, if an exception is thrown during object construction, the error message simply says JUnit was unable to instantiate the test case and you don't see the line number where the failure occurred. I'm guessing it's because JUnit uses reflection to instantiate the test cases.

None of this applies to the example of creating an empty collection, since that will never throw, but it is an advantage of the setUp() method.

Craig P. Motlin
+3  A: 

In your case (creating a list) there is no difference in practice. But generally it is better to use setUp(), because that will help Junit to report Exceptions correctly. If an exception occurs in constructor/initializer of a Test, that is a test failure. However, if an exception occurs during setup, it is natural to think of it as some issue in setting up the test, and junit reports it appropriately.

well said. Just get used to always instantiate in setUp() and you have one question less to worry about - e.g. where should I instantiate my fooBar, where my collection. It's a kind of coding standard that you just need to adhere to. Benefits you not with lists, but with other instantiations.
Olaf
@Olaf Thanks for the info about the coding standard, I hadn't thought about that. I tend to agree with Moss Collum's idea of a coding standard more though.
Craig P. Motlin
+1  A: 

Since each test is executed independently, with a fresh instance of the object, there's not much point to the Test object having any internal state except that shared between setUp() and an individual test and tearDown(). This is one reason (in addition to the reasons others gave) that it's good to use the setUp() method.

Note: It's a bad idea for a JUnit test object to maintain static state! If you make use of static variable in your tests for anything other than tracking or diagnostic purposes, you are invalidating part of the purpose of JUnit, which is that the tests can (an may) be run in any order, each test running with a fresh, clean state.

The advantages to using setUp() is that you don't have to cut-and-paste initialization code in every test method and that you don't have test setup code in the constructor. In your case, there is little difference. Just creating an empty list can be done safely as you show it or in the constructor as it's a trivial initialization. However, as you and others have pointed out, anything that can possibly throw an Exception should be done in setUp() so you get the diagnostic stack dump if it fails.

In your case, where you are just creating an empty list, I would do the same way you are suggesting: Assign the new list at the point of declaration. Especially because this way you have the option of marking it final if this makes sense for your test class.

Eddie
+1 because you're the first person to actually support initializing the list during object construction for marking it final. The stuff about static variables is off-topic to the question though.
Craig P. Motlin
@Motlin: true, the stuff about static variables is a little off-topic. I'm not sure why I added that, but it seemed appropriate at the time, an extension of what I was saying in the first paragraph.
Eddie
+4  A: 

If you're wondering specifically about the examples in the JUnit FAQ, such as the basic test template, I think the best practice being shown off there is that the class under test should be instantiated in your setUp method (or in a test method).

When the JUnit examples create an ArrayList in the setUp method, they all go on to test the behavior of that ArrayList, with cases like testIndexOutOfBoundException, testEmptyCollection, and the like. The perspective there is of someone writing a class and making sure it works right.

You should probably do the same when testing your own classes: create your object in setUp or in a test method, so that you'll be able to get reasonable output if you break it later.

On the other hand, if you use a Java collection class (or other library class, for that matter) in your test code, it's probably not because you want to test it--it's just part of the test fixture. In this case, you can safely assume it works as intended, so initializing it in the declaration won't be a problem.

For what it's worth, I work on a reasonably large, several-year-old, TDD-developed code base. We habitually initialize things in their declarations in test code, and in the year and a half that I've been on this project, it has never caused a problem. So there's at least some anecdotal evidence that it's a reasonable thing to do.

Moss Collum
+1 Good complete answer. It's also the only answer without incorrect or irrelevant information. So far it looks like the bounty will be yours.
Craig P. Motlin
A: 

The simple best case is this: your test case class tests a single class. Obviously each test method will need an instance of that class. But no test should have to handle the mingled object from previous tests. So one can either 'new' it up in each test, or use 'setUp()' (or a @Before annotated method) to keep it DRY. I see no reason to give with the former.

Edit: Even though there might not be any shared state - it's still DRYer! No reason to 'new' it up again and again...

abyx
There's no shared state, please see the comments to the other answers.
Craig P. Motlin
@Motlin - See edit
abyx
@abyx Similar to Bill the Lizard's answer. But the list is a field, not a local variable. So there's no repetition.
Craig P. Motlin