Derby
  1. Derby
  2. DERBY-6073

Test ordering instability in StatementPoolingTest

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 10.9.1.0, 10.10.1.1
    • Fix Version/s: 10.10.1.1
    • Component/s: Test
    • Labels:
      None
    • Environment:
      Java 7, Java 8
    • Urgency:
      Normal

      Description

      StatementPoolingTest.testPoolingEnabledByCheckingImplementationDetails() assumes that the client-side statement cache will have been primed by a previous test case, testCacheOverflow(). On Java 7 and Java 8 the test order is not deterministic. If testPoolingEnabledByCheckingImplementationDetails() is the first test case to run, then it fails with this error:

      org.apache.derbyTesting.functionTests.tests.jdbcapi.StatementPoolingTest.assertClassName(StatementPoolingTest.java:147)
      at org.apache.derbyTesting.functionTests.tests.jdbcapi.StatementPoolingTest.testPoolingEnabledByCheckingImplementationDetails(StatementPoolingTest.java:89)

      I will attach a patch which forces testPoolingEnabledByCheckingImplementationDetails() to be first in the test order. With this patch, StatementPoolingTest fails for me on Java 7 when run on the 10.9 branch as well as on trunk.

      1. z.diff
        8 kB
        Rick Hillegas
      2. derby-6073-01-aa-fixTestCaseOrder.diff
        7 kB
        Rick Hillegas
      3. derby-6073-02-aa-java8tweak.diff
        0.9 kB
        Rick Hillegas
      4. derby-6073-03-aa-fixConnector.diff
        10 kB
        Knut Anders Hatlen

        Issue Links

          Activity

          Hide
          Knut Anders Hatlen added a comment -

          I don't think there's any more work needed on this bug. Closing.

          Show
          Knut Anders Hatlen added a comment - I don't think there's any more work needed on this bug. Closing.
          Hide
          Rick Hillegas added a comment -

          Can we close this issue now? Thanks.

          Show
          Rick Hillegas added a comment - Can we close this issue now? Thanks.
          Hide
          Knut Anders Hatlen added a comment -

          Committed revision 1447667.

          Show
          Knut Anders Hatlen added a comment - Committed revision 1447667.
          Hide
          Knut Anders Hatlen added a comment -

          All the regression tests passed with the patch.

          Show
          Knut Anders Hatlen added a comment - All the regression tests passed with the patch.
          Hide
          Knut Anders Hatlen added a comment -

          I think the problem is in the test framework. ConnectionPoolDataSourceConnector calls setMaxStatements(2) on the data source before creating a connection. However, if that fails, it creates a new data source on which it calls setCreateDatabase("create"), and returns a connection created with this data source. The data source that creates the database does not have statement pooling enabled, and the test case will therefore not use statement pooling if the wombat database doesn't already exist before the test case is executed.

          The attached patch (derby-6073-03-aa-fixConnector.diff) changes ConnectionPoolDataSourceConnector so that it also enables statement pooling on data sources that have the createDatabase attribute set. This makes StatementPoolingTest pass in my environment also if testPoolingEnabledByCheckingImplementationDetails() runs as the first test case.

          Running the full regression test suite on the patch to see if it causes problems for other tests that use ConnectionPoolDataSourceConnector.

          Show
          Knut Anders Hatlen added a comment - I think the problem is in the test framework. ConnectionPoolDataSourceConnector calls setMaxStatements(2) on the data source before creating a connection. However, if that fails, it creates a new data source on which it calls setCreateDatabase("create"), and returns a connection created with this data source. The data source that creates the database does not have statement pooling enabled, and the test case will therefore not use statement pooling if the wombat database doesn't already exist before the test case is executed. The attached patch (derby-6073-03-aa-fixConnector.diff) changes ConnectionPoolDataSourceConnector so that it also enables statement pooling on data sources that have the createDatabase attribute set. This makes StatementPoolingTest pass in my environment also if testPoolingEnabledByCheckingImplementationDetails() runs as the first test case. Running the full regression test suite on the patch to see if it causes problems for other tests that use ConnectionPoolDataSourceConnector.
          Hide
          Rick Hillegas added a comment -

          Attaching derby-6073-02-aa-java8tweak.diff. This patch adjusts the assertClassName() method to account for the fact that some *40 classes have corresponding *42 subclasses and some don't. Committed at subversion revision 1446698.

          The logic is brittle and will probably break again when we implement the next (Java 9) rev of JDBC. But maybe we can just kick the can down the road for another two years. I will unassign myself from this bug so that others can improve this bug fix if they want to.

          Touches the following file:

          M java/testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/StatementPoolingTest.java

          Show
          Rick Hillegas added a comment - Attaching derby-6073-02-aa-java8tweak.diff. This patch adjusts the assertClassName() method to account for the fact that some *40 classes have corresponding *42 subclasses and some don't. Committed at subversion revision 1446698. The logic is brittle and will probably break again when we implement the next (Java 9) rev of JDBC. But maybe we can just kick the can down the road for another two years. I will unassign myself from this bug so that others can improve this bug fix if they want to. Touches the following file: M java/testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/StatementPoolingTest.java
          Hide
          Rick Hillegas added a comment -

          Hi Kristian,

          You probably know more about this test case than I do. If you want to back out my fix and propose a better one, feel free to! Thanks.

          Show
          Rick Hillegas added a comment - Hi Kristian, You probably know more about this test case than I do. If you want to back out my fix and propose a better one, feel free to! Thanks.
          Hide
          Kristian Waagan added a comment -

          Rick, are you sure this is about test ordering?

          The test fails when getting the connection. The connection is not of the right type. I'm thinking this is a bug in that single test, not an ordering instability.

          Show
          Kristian Waagan added a comment - Rick, are you sure this is about test ordering? The test fails when getting the connection. The connection is not of the right type. I'm thinking this is a bug in that single test, not an ordering instability.
          Hide
          Rick Hillegas added a comment -

          Ported 1446675 from trunk to 10.9 branch at subversion revision 1446676.

          Show
          Rick Hillegas added a comment - Ported 1446675 from trunk to 10.9 branch at subversion revision 1446676.
          Hide
          Rick Hillegas added a comment -

          Attaching derby-6073-01-aa-fixTestCaseOrder.diff. This patch makes the test case order deterministic and fixes the bug on Java 7. Committed at subversion revision 1446675.

          After applying this patch, I see a follow-on problem on Java 8:

          1) testAll(org.apache.derbyTesting.functionTests.tests.jdbcapi.StatementPoolingTest)junit.framework.ComparisonFailure: expected:<...alPreparedStatement4[0]> but was:<...alPreparedStatement4[2]>
          at org.apache.derbyTesting.functionTests.tests.jdbcapi.StatementPoolingTest.assertClassName(StatementPoolingTest.java:145)
          at org.apache.derbyTesting.functionTests.tests.jdbcapi.StatementPoolingTest.t02_testPoolingEnabledByCheckingImplementationDetails(StatementPoolingTest.java:90)

          I will port derby-6073-01-aa-fixTestCaseOrder.diff to 10.9 and prepare another patch for trunk to fix the follow-on problem.

          Touches the following file:

          M java/testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/StatementPoolingTest.java

          Show
          Rick Hillegas added a comment - Attaching derby-6073-01-aa-fixTestCaseOrder.diff. This patch makes the test case order deterministic and fixes the bug on Java 7. Committed at subversion revision 1446675. After applying this patch, I see a follow-on problem on Java 8: 1) testAll(org.apache.derbyTesting.functionTests.tests.jdbcapi.StatementPoolingTest)junit.framework.ComparisonFailure: expected:<...alPreparedStatement4 [0] > but was:<...alPreparedStatement4 [2] > at org.apache.derbyTesting.functionTests.tests.jdbcapi.StatementPoolingTest.assertClassName(StatementPoolingTest.java:145) at org.apache.derbyTesting.functionTests.tests.jdbcapi.StatementPoolingTest.t02_testPoolingEnabledByCheckingImplementationDetails(StatementPoolingTest.java:90) I will port derby-6073-01-aa-fixTestCaseOrder.diff to 10.9 and prepare another patch for trunk to fix the follow-on problem. Touches the following file: M java/testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/StatementPoolingTest.java
          Hide
          Rick Hillegas added a comment -

          Linking to DERBY-6000 because this bug was discovered while testing the new JDBC 4.2 implementation on Java 8.

          Show
          Rick Hillegas added a comment - Linking to DERBY-6000 because this bug was discovered while testing the new JDBC 4.2 implementation on Java 8.
          Hide
          Rick Hillegas added a comment -

          Attaching z.diff. After applying this patch, I see the error on trunk. I also see the error after applying the patch to 10.9 and then running with Java 6 or Java 7. Because the problem can be reproduced on 10.9, I do not believe this is a regression introduced by work on the trunk.

          Show
          Rick Hillegas added a comment - Attaching z.diff. After applying this patch, I see the error on trunk. I also see the error after applying the patch to 10.9 and then running with Java 6 or Java 7. Because the problem can be reproduced on 10.9, I do not believe this is a regression introduced by work on the trunk.

            People

            • Assignee:
              Rick Hillegas
              Reporter:
              Rick Hillegas
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development