Uploaded image for project: 'Derby'
  1. Derby
  2. DERBY-5734

End transaction if CleanDatabaseTestSetup.decorateSQL fails

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 10.9.1.0
    • Fix Version/s: 10.9.1.0
    • Component/s: Test
    • Labels:
      None

      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.

      1. derby-5734-1b-end_transaction.diff
        1 kB
        Kristian Waagan
      2. derby-5734-1a-end_transaction.diff
        1 kB
        Kristian Waagan

        Activity

        Hide
        kristwaa Kristian Waagan added a comment -

        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.

        Show
        kristwaa Kristian Waagan added a comment - 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.
        Hide
        knutanders Knut Anders Hatlen added a comment -

        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.

        Show
        knutanders Knut Anders Hatlen added a comment - 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.
        Hide
        kristwaa Kristian Waagan added a comment -

        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.

        Show
        kristwaa Kristian Waagan added a comment - 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.
        Hide
        kristwaa Kristian Waagan added a comment -

        Committed to trunk with revision 1333360.
        Closing issue.

        Show
        kristwaa Kristian Waagan added a comment - Committed to trunk with revision 1333360. Closing issue.

          People

          • Assignee:
            kristwaa Kristian Waagan
            Reporter:
            kristwaa Kristian Waagan
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development