isn't testing any behaviour of the production code, is it?
Also, do you really need to set all those other fields?
The soul is dyed the color of its thoughts. Think only on those things that are in line with your principles and can bear the light of day. The content of your character is your choice. Day by day, what you do is who you become. Your integrity is your destiny - it is the light that guides your way. - Heraclitus
Peter, you might also want to move the cleanup code (deleting the inserted user) into tearDown(). If any of the assertions in the test method fail, the remainder of the method will not be executed but tearDown() will be.
Good advice. Make sure, thought, that teardown doesn't throw any exceptions, otherwise they will mask the true test failures.
Joined: Sep 10, 2004
you might also want to move the cleanup code (deleting the inserted user) into tearDown(). If any of the assertions in the test method fail, the remainder of the method will not be executed but tearDown() will be.
I'm not sure I fully understand your advice. What is the propose of tearDown()and why should I not use the userDAO.deleteByPrimaryKey(id1) in the testEmailDuplication() but have it deleted in a different method?
Originally posted by Peter Primrose: I'm not sure I fully understand your advice. What is the propose of tearDown()and why should I not use the userDAO.deleteByPrimaryKey(id1) in the testEmailDuplication() but have it deleted in a different method?
Think about the flow of the test. If all goes well, the method: 1) Inserts a row 2) Tries to insert another row, but can't due to constraint 3) Deletes the row inserted in step 1.
You can run this test multiple times and all will still be well. Now let's say you aren't prevented from adding the second row. The flow is now: 1) Inserts a row 2) Tries to insert another row, and can - test fails 3) Skips delete because assertion throws an exception
If you run the test again, step 1 will fail because the row is still in the database. Putting the delete in tear down ensures the row will get deleted even if step 2 fails.
Personally, I like to have setUp() call tearDown() for database tests. This protects me against other processes getting the database in a state that the test can't run.
Joined: Jan 23, 2002
Peter, JUnit TestCase classes can have two special methods, setUp() and tearDown(), which can be used for [drum-roll...] setting up and tearing down whatever state or data your test needs.
In other words, the JUnit framework when running a TestCase class, basically does the following for each test method:
1) run setUp() 2) run test*() 3) run tearDown()
And this happens regardless of whether test*() throws an exception or not.
Joined: Sep 10, 2004
is this a good practice of tearDown and setUp? thanks
Joined: Jan 23, 2002
I'd personally move the stuff from constructor inside the setUp(). The reason being that JUnit's API does not guarantee that its behavior regarding TestCase instantiation will stay like it's currently implemented.
Having said that, this only concerns JUnit 3.8 which is no longer developed due to JUnit 4.x and Java 5 being all the rage...
Originally posted by Peter Primrose: is this a good practice of tearDown and setUp? thanks
Also note how a lot ofyour test methods have a new Traveller instance. I would create a private variable for traveller and create a new Traveller in the setUp code :
You now have the option to create all the details for a traveller in your setUp code (this will be the default traveller) and only change the details that you are testing in the test method, or just produce the bare-bones of a traveller (like the above code shows) and fill in the details in each test method that uses a Traveller.
There are still a few other things which I would clean up about this latest test example:
First, I would really discourage use of logging in tests. the sole job of a test case is to make sure stuff works. In my opinion tests should run completely silently unless something fails, in which case they should try their best to explain what was being tested and/or what went wrong.
Second, I can see lots of duplication in the tests. I would immediately refactor the Traveler setup into a single helper method with a bunch of parameters, and I'd also refactor the delete stuff into a helper method which takes a primitive rather than duplicating the "new Long" stuff every time.
Each test should say just what it means as clearly as possible, and assert only things that matter, but all things that matter, so I'm worried that your testInsert and testDelete set a bunch of stuff (implying that it is important) but there are no asserts to check that the right thing happened.
In the testInsert method a lot of effort is put in to creating a Traveler with specific values, but the only thing which is actually tested is the increment of a number. There are no checks that any data (let alone the right data) have actually been inserted into the database.
The testDelete case is even more scary. It has no asserts at all. testDelete is effectively worthless - it will pass even if the deleteAllByUserID method is completely empty and does nothing at all