skizz.biz

What is wrong with this code?

public class FooTest extends TestCase {

	Random random = new Random(1000);

	public void testSomething() {
		Long primaryKey = new Long(Math.abs(random.nextInt()));
		[create some objects]
		...
		[delete the objects]
	}

	public void testSomethingElse() {
		Long primaryKey = new Long(Math.abs(random.nextInt()));
		...
	}
}

First problem, in every random library I have used, when you create a random number sequence with a seed, it gives you not a new sequence, but the same sequence. The numbers are pseudo-random, but the sequence will be identical.

So for a quick demonstration of this:

	public void testRandom() {
		System.out.println("First Sequence");

		Random random = new Random(1000);
		System.out.println(random.nextInt());
		System.out.println(random.nextInt());
		System.out.println(random.nextInt());
		System.out.println(random.nextInt());
		System.out.println(random.nextInt());
		System.out.println(random.nextInt());

		System.out.println("New Sequence");

		random = new Random(1000);
		System.out.println(random.nextInt());
		System.out.println(random.nextInt());
		System.out.println(random.nextInt());
		System.out.println(random.nextInt());
		System.out.println(random.nextInt());
		System.out.println(random.nextInt());
	}

gives

-1244746321
1060493871
-1826063944
1976922248
-230127712
68408698
New Sequence
-1244746321
1060493871
-1826063944
1976922248
-230127712
68408698

Since JUnit enforces isolation by creating a new instance for each test*, each test will get the same number (actually -1244746321) for the primaryKey. So when it uses the primaryKey to create the objects for the test they will all have the same primaryKey. Almost certainly not the intent of the test author.

This is not such a bad thing if the tests clean up properly after themselves, but if they don’t, and the tests fail, from then on you will get primaryKey violation errors when you hit the database.

So, the second problem in these tests: the [delete the objects] is in the test. If the test fails, the objects are never cleaned up. Clean up belongs in tearDown(). That is what it is designed for.

*A subject of recent controversy, but IMHO a Good Thing