End transaction if CleanDatabaseTestSetup.decorateSQL fails

Details

Description

By overriding CleanDatabaseTestSetup.decorateSQL you are allowed to perform tasks as part of the test setup / decorator. The connection obtainable through the passed in statement is configured with auto-commit off. If decorateSQL fails the connection may be left active in the middle of a transaction, which again may cause subsequent tests to fails. I've observed subsequent tests fail due to locks helds by the statement / connection passed to decorateSQL.

CleanDatabaseTestSetup.setUp should ensure the transaction is ended regardless of whether decorateSQL throws an exception or not.

Kristian Waagan
added a comment - 30/Apr/12 10:37 Attaching patch 1a, which makes sure the transaction is either committed or rolled back. It doens't guarantee that the connection is closed.
Patch ready for review.

If you call JDBC.cleanup(conn) or clearConnection(conn) instead, the connection will be closed too. The latter will additionally clear the conn instance variable. But I suppose rollback() would be enough to get the tests going again in most cases.

Knut Anders Hatlen
added a comment - 30/Apr/12 11:21 Looks like a useful fix. +1
If you call JDBC.cleanup(conn) or clearConnection(conn) instead, the connection will be closed too. The latter will additionally clear the conn instance variable. But I suppose rollback() would be enough to get the tests going again in most cases.

Kristian Waagan
added a comment - 03/May/12 09:46 Thanks for looking at the patch, Knut.
I rewrote the logic a little and used clearConnection() (no argument passed) in the finally block.
Regression tests passed.