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

JDBCMBeanTest#testAttributeDriverLevel uses Java assert in lieu of JUnit assert: no real testing happens

    Details

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

      Description

      Cf these lines:

      assert(driverLevelString.indexOf('?') == -1);
      assert(driverLevelString.matches("^JRE - JDBC: " + JDBCVersion + ".*"));

      The "assert" is a Java built-in rather than the JUnit assertTrue we need.

      The string driverLevelString is also wrong, since it is a mere toString of the bean name. We need to use getAttribute on it to get the driver level
      for asserting.

      1. derby-5521-followup.diff
        1 kB
        Dag H. Wanvik
      2. derby-5521b.diff
        2 kB
        Dag H. Wanvik
      3. derby-5521.diff
        1 kB
        Dag H. Wanvik

        Issue Links

          Activity

          Hide
          dagw Dag H. Wanvik added a comment -

          Tested manually with Java 1.5, Java 1.6 and Java 1.7.

          Show
          dagw Dag H. Wanvik added a comment - Tested manually with Java 1.5, Java 1.6 and Java 1.7.
          Hide
          dagw Dag H. Wanvik added a comment -

          Committed this as svn 1212210, resolving

          Patch derby-5521-followup fixes a small issue with the regexp (JDK
          name varied over time: J2SE -> Java SE) used in the test.

          Show
          dagw Dag H. Wanvik added a comment - Committed this as svn 1212210, resolving Patch derby-5521-followup fixes a small issue with the regexp (JDK name varied over time: J2SE -> Java SE) used in the test.
          Hide
          knutanders Knut Anders Hatlen added a comment -

          Seems to have caused a problem on Java 5:

          http://dbtg.foundry.sun.com/derby/test/Daily/jvm1.5/testing/testlog/sles/1211538-suitesAll_diff.txt

          suitesAll fail *************************************************************
          1) testAttributeDriverLevel(org.apache.derbyTesting.functionTests.tests.management.JDBCMBeanTest)junit.framework.AssertionFailedError: Unexpected driver level string: J2SE 5.0 - JDBC 3.0
          2) testAttributeDriverLevel(org.apache.derbyTesting.functionTests.tests.management.JDBCMBeanTest)junit.framework.AssertionFailedError: Unexpected driver level string: J2SE 5.0 - JDBC 3.0
          suitesAll fail *************************************************************

          Show
          knutanders Knut Anders Hatlen added a comment - Seems to have caused a problem on Java 5: http://dbtg.foundry.sun.com/derby/test/Daily/jvm1.5/testing/testlog/sles/1211538-suitesAll_diff.txt suitesAll fail ************************************************************* 1) testAttributeDriverLevel(org.apache.derbyTesting.functionTests.tests.management.JDBCMBeanTest)junit.framework.AssertionFailedError: Unexpected driver level string: J2SE 5.0 - JDBC 3.0 2) testAttributeDriverLevel(org.apache.derbyTesting.functionTests.tests.management.JDBCMBeanTest)junit.framework.AssertionFailedError: Unexpected driver level string: J2SE 5.0 - JDBC 3.0 suitesAll fail *************************************************************
          Hide
          dagw Dag H. Wanvik added a comment -

          Not suitable for backport, bug introduced post 10.8, closing.

          Show
          dagw Dag H. Wanvik added a comment - Not suitable for backport, bug introduced post 10.8, closing.
          Hide
          dagw Dag H. Wanvik added a comment -

          Thanks, Knut. Committed derby-5521b taking that advice as svn 1211266.

          Show
          dagw Dag H. Wanvik added a comment - Thanks, Knut. Committed derby-5521b taking that advice as svn 1211266.
          Hide
          knutanders Knut Anders Hatlen added a comment -

          The patch looks good. One small nit: assertEquals() should have the expected value as the first argument. Perhaps we should also pass driverLevelString as message argument to the two asserts, so we can see what's wrong if they ever fail?

          Show
          knutanders Knut Anders Hatlen added a comment - The patch looks good. One small nit: assertEquals() should have the expected value as the first argument. Perhaps we should also pass driverLevelString as message argument to the two asserts, so we can see what's wrong if they ever fail?
          Hide
          dagw Dag H. Wanvik added a comment -

          Attaching a patch for this. Cf also DERBY-5519.

          Show
          dagw Dag H. Wanvik added a comment - Attaching a patch for this. Cf also DERBY-5519 .

            People

            • Assignee:
              dagw Dag H. Wanvik
              Reporter:
              dagw Dag H. Wanvik
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development