Win a copy of Programmer's Guide to Java SE 8 Oracle Certified Associate (OCA) this week in the OCAJP forum!
  • Post Reply
  • Bookmark Topic Watch Topic
  • New Topic

Test duplication on db with exception...am I on the right track?

 
Peter Primrose
Ranch Hand
Posts: 755
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi guys,
I have a table with 3 columns:
id, email and password.

id = primary key auto inc
email = uinque
password = varchar.

the email is not a primary key but cannot be duplicated.

I would like to test that, I wonder if someone can criticize my test and tell me if I'm on the right track (primarily on the exception hardening).

Thanks!


 
Ilja Preuss
author
Sheriff
Posts: 14112
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Seems to me like the line

assertEquals(id2, new Long(-1));

isn't testing any behaviour of the production code, is it?

Also, do you really need to set all those other fields?
 
Jeanne Boyarsky
author & internet detective
Marshal
Posts: 34837
369
Eclipse IDE Java VI Editor
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Peter,
I would rewrite the try/catch block as.
 
Ilja Preuss
author
Sheriff
Posts: 14112
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Jeanne, good catch! Now I understand what that assertion on id2 was used for...
 
Jeanne Boyarsky
author & internet detective
Marshal
Posts: 34837
369
Eclipse IDE Java VI Editor
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Originally posted by Ilja Preuss:
Jeanne, good catch! Now I understand what that assertion on id2 was used for...

Oh! Me too! I didn't follow what that was for either until I read your comment.
 
Peter Primrose
Ranch Hand
Posts: 755
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Jeanne & Ilja

thank you for your thoughts.

I didn't know you could use 'fail' as in...
try{
id2 = userDAO.insert(user2);
fail("should throw an exception");
...

ok, I fixd it and I learned somthing new.

thanks a lot!
 
Lasse Koskela
author
Sheriff
Posts: 11962
5
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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.
 
Ilja Preuss
author
Sheriff
Posts: 14112
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Good advice. Make sure, thought, that teardown doesn't throw any exceptions, otherwise they will mask the true test failures.
 
Peter Primrose
Ranch Hand
Posts: 755
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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.


Lessa,

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?

Could you kindly elaborate
Thanks
 
Jeanne Boyarsky
author & internet detective
Marshal
Posts: 34837
369
Eclipse IDE Java VI Editor
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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.
 
Lasse Koskela
author
Sheriff
Posts: 11962
5
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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.
 
Peter Primrose
Ranch Hand
Posts: 755
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
is this a good practice of tearDown and setUp?
thanks

 
Lasse Koskela
author
Sheriff
Posts: 11962
5
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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...
 
Fintan Conway
Ranch Hand
Posts: 142
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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.

Regards,

Fintan
 
Frank Carver
Sheriff
Posts: 6920
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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
 
  • Post Reply
  • Bookmark Topic Watch Topic
  • New Topic