Test failings, Part II

Posted on October 27, 2005 by Scott Leberknight

A co-worker, when I showed him the ExceptionAssertionTemplate and ExceptionAssertionCallback, took one look and said, "Why don't you just catch the exception you are expecting, have your test throw Exception, and be done with it?" For example,

public void testFindNonExistentVisit() throws Exception {
    try {
        dao.findVisit(-9999L);
    }
    catch (HibernateObjectRetrievalFailureException ex) {}
}

Um, yeah, why wasn't I just doing that? It is simpler and less code. Thanks Andrew!

Ensuring Your Tests fail() When They Should

Posted on October 24, 2005 by Scott Leberknight

A few weeks ago I posted about how I use the fail() method immediately after an Exception should have been thrown, in order to ensure that invalid input or use of the method results in the exception that is expected. Since manually remembering to call fail() in every one of these situations is simply not going to work (at least not for me), I wrote a couple of quick and dirty utility classes to help out. They are based on a Spring-like Template/Callback approach, and embed the code I don't want to forget in the template class so that it is impossible to forget to call fail().

The template class is currently called ExceptionAssertionTemplate and contains two overloaded execute() methods. Here it is, with all my verbose JavaDocs and such.

package com.sleberknight.tests;

import junit.framework.Assert;

/**
 * Template class for use by unit tests that want to perform actions
 * that cause expected exceptions and then  assert the exception
 * was thrown and is of the correct type.
 *
 * @author Scott Leberknight
 */
public class ExceptionAssertionTemplate {

    /**
     * Executes the specified ExceptionAssertionCallback and
     * fails the test if no exception was actually thrown. Does not check
     * the type of exception that was thrown. Any thrown exception is
     * considered a successful test.
     *
     * @param callback
     */
    public void execute(ExceptionAssertionCallback callback) {
        try {
            callback.doActionCausingException();
            Assert.fail("Should have thrown an exception ");
        } catch (Exception e) {
        }
    }

    /**
     * Executes the specified ExceptionAssertionCallback
     * and either fails the test if no exception was actually thrown or
     * asserts that the exception thrown is of the expected type.
     *
     * @param callback
     * @param expectedExceptionType
     */
    public void execute(ExceptionAssertionCallback callback, Class expectedExceptionType) {
        try {
            callback.doActionCausingException();
            Assert.fail("Should have thrown a " + expectedExceptionType.getName());
        } catch (Exception e) {
            Assert.assertEquals(expectedExceptionType, e.getClass());
        }
    }

}

So to use it you need a ExceptionAssertionCallback, which is where you put code that should throw an exception. The no-arg execute() method executes the callback and fails the test if no exception was thrown. The execute() method that takes the expectedExceptionType argument additionally asserts that the thrown exception is of the expected type. So here is the callback class implementation.

package com.sleberknight.tests;

/**
 * Callback class that unit tests can use to perform actions that should throw exceptions.
 *
 * @author Scott Leberknight
 */
public abstract class ExceptionAssertionCallback {

    /**
     * Implement this method to throw the expected exception.
     *
     * @throws Exception
     */
    public abstract void doActionCausingException() throws Exception;

}

That's it. The ExceptionAssertionCallback is an abstract class with one method, doActionCausingException(), which must be implemented. So how do you use these classes? Here is a simple example, in which I have a DAO I am testing a findVisit() method with a non-existent record in the database. This method should throw a HibernateObjectRetrievalFailureException when called with an invalid identifier. So basically you create the template, and then call the execute() method, passing in the callback, defined here as an anonymous inner class. Note that dao is defined as a private instance variable in the test case, and is thus available to use in the callback. The template then executes the callback's doActionCausingException() method and verifies that it throws an exception of type HibernateObjectRetrievalFailureException. Assuming it does, the test passes. Otherwise, the test fails.

public void testFindNonExistentVisit() {
    ExceptionAssertionTemplate template = new ExceptionAssertionTemplate();
    template.execute(new ExceptionAssertionCallback() {
        public void doActionCausingException() throws Exception {
            dao.findVisit(-9999L);
        }
    }, HibernateObjectRetrievalFailureException.class);
}

You could of course create a base test case that creates the ExceptionAssertionTemplate, which I actually did, and then there is even less code in your test as you can then just use the existing template that was created for you. But the main point here is that by using this approach, you cannot forget to fail() the test ever again.

Using Spring with Hibernate3's Lazy-Load Everything Approach

Posted on October 22, 2005 by Scott Leberknight

I am writing this so that I never, ever, ever forget how the new default lazy-load behavior in Hibernate3 can completely mess with your head for hours and hours of debugging when you are expecting Spring's HibernateTemplate's load method to throw a HibernateObjectRetrievalFailureException in a unit test, only to find the reason is quite simple but subtle! Oh, and of course if anyone else is reading, or more importantly, Googling, then hopefully this will help you too.

I have an implementation of a DAO that extends Spring's HibernateDaoSupport class which has a finder method for an entity given the unique identifier, which is of type Long. That finder method basically does this:

public Entity findEntity(Long id) {
    return (Entity)getHibernateTemplate().load(Entity.class, id);
}

Note specifically that I am using the load() method which is defined to "Return the persistent instance of the given entity class with the given identifier, throwing an exception if not found." Specifically, this method should catch any HibernateException subclass thrown by the internal HibernateCallback and convert it into the appropriate class in the Spring DataAccessException hierarchy, e.g. in this case it should be converted to a HibernateObjectRetrievalFailureException if I pass in an identifier for which there is no persistent entity. So far, so good.

Next I have a simple unit test that calls my finder method using an invalid identifier, and then it asserts that the appropriate exception was thrown. Basically it looks something like this:

public void testFindEntityUsingInvalidIdentifier() {
    final Class expectedExceptionType = HibernateObjectRetrievalFailureException.class;
    try {
        dao.findEntity(-9999L);
        fail("Should have thrown an " + expectedExceptionType.getName());
    } catch (Exception e) {
        assertEquals(expectedExceptionType, e.getClass());
    }
}

So I ran this test thinking it was a no-brainer. It failed. In fact, it failed with the message "junit.framework.AssertionFailedError: Should have thrown a org.springframework.orm.hibernate3.HibernateObjectRetrievalFailureException." Um, what? I was about 100% sure there was no such row in the database, since I was using Spring's AbstractTransactionalDataSourceSpringContextTests test class which ensures transactions are rolled back after each test, and because I knew I didn't put any data in the database with that identifier. So that meant the findEntity method did not throw any exception. I started adding the old fallback System.out.println() statements all over the place to see what exactly was going on. The finder method actually was returning a non-null object, but when I tried to call any method on it, like toString(), it then threw a raw Hibernate ObjectNotFoundException, which as of Hibernate3 is unchecked not checked. Hmmm. Performed some initial debugging using a real debugger no less and found that proxy objects were being returned, and then looked in the stack traces and saw some CGLIB stuff in there, meaning the entity object was in fact proxied.

Since the object was actually a proxy, that explained why no exception was thrown by HibernateTemplate.load() - since no methods had been called on the proxy yet, Hibernate3 was happily returning a proxy object for an identifier which does not really exist. The second you call any method on that proxy, you get the Hibernate ObjectNotFoundException. Did a bunch of research and finally found that the Hibernate3 reference guide, in Section 11.3 (Loading an object) mentions that "load() will throw an unrecoverable exception if there is no matching database row. If the class is mapped with a proxy, load() just returns an uninitialized proxy and does not actually hit the database until you invoke a method of the proxy." I then did more research and arrived at a resource I should probably have looked at much sooner, as this is my first time using Hibernate3 (I've been using Hibernate2.x for a while now). That resource is the Hibernate3 migration guide which mentions that Hibernate3 now defaults to lazy="true" for everything, both classes and collections. I even remember reading this a while back but it didn't occur to me that the load() method in the Hibernate Session would behave like that.

In any case, with lazy loading set to "true" because of the changed default value in Hibernate3, the Spring HibernateTemplate.load() basically is useless for lazily initialized objects, since it will not catch any Hibernate exceptions and thus won't ever do the conversion into one of the DataAccessException subclasses. There are a few solutions or workarounds or hacks or whatever you want to call them. First, you could set default-lazy="false" in your root hibernate-mapping element, which will switch the behavior back to what it was in Hibernate2.x. That is what I did to verify that my test would work properly when there was no proxying involved. Second, you can use the HibernateTemplate.get() method instead of load() (which delegates to the Hibernate Session.get() method) because get() "hits the database immediately and returns null if there is no matching row" and then if the result is null, throw the appropriate exception yourself. Third, you can leave Hibernate3's default behavior alone and override it for specific classes and/or collections. For collections I almost always use lazy loading but I had never done it for classes. So you could set lazy="true" for specific mapped classes in your Hibernate mappings.

I fully understand why you want to lazily initialize your collections, but I am not sure why I would want the default behavior for normal classes to be lazy load. When I perform a find for a specific object, I pretty much know I want that object since I am going to do something with it, like display it in a web page or something. I suppose one use case for lazy initializing classes would be if you perform a query and get back a list of objects, and you don't want to initialize all of them until the user actually needs them. But even in that case I generally am going to display the search results and I will be using those objects. So I am still somewhat at odds for when I would realistically want this behavior; even for search results I can retrieve only the results I need by using Hibernate's ability to set the start row and number of rows returned for a query.

So, the main point of all this is that the default lazy loading of everything in Hibernate3 may cause some unexpected problems, and if you start seeing CGLIB in stack traces and weird things are happening, chances are you've got an object proxy rather than the actual objct you thought you were getting, and that the proxy is causing some weird behavior. Whew!

Don't Forget To fail() Your Tests

Posted on October 05, 2005 by Scott Leberknight

Often when writing JUnit tests, I use the fail() method immediately after an Exception should have been thrown, in order to ensure that invalid input or use of the method results in the exception you expect. However, earlier this week I was updating a test case and noticed one test method that was expecting an exception to occur did not have a call to fail(). So this is the code that should have been in the test:

public void testSomethingFailsThatYouExpectToFail() {
    try {
        SomeObject someObject = new SomeObject();
        someObject.someMethodThatShouldFail("invalid input");
        fail("Should have thrown IllegalArgumentException");  // need this line!
    }
    catch (Exception ex) {
        assertEquals(IllegalArgumentException.class, ex.getClass());
    }
}

But the code in the test did not have the fail() method call. Without fail(), this test works as expected only when the method actually throws the exception and the assertion validates the type of exception thrown. If, on the other hand, the exception is not thrown then the test passes but is not actually a valid test!

So as a result I did a quick search through all tests and found a significant number where I or one of the other developers had forgotten to call fail() immediately following the method call where we expect an exception to be thrown, which I then had to fix by adding the call to fail() back in. After doing that I found there were actually three tests that then failed. So even though these were exceptional test cases I had to fix the offending code. Maybe I can write a Checkstyle or PMD rule that would detect this type of error in unit tests, because it is so easy to forget to call fail(), have the test pass, and move on to the next task.