Derby
  1. Derby
  2. DERBY-2674

Stop running jdbc40 tests in the old test framework

    Details

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

      Description

      Tests in the old jdbc40 test suite are run in the derbyall test suite even if all the tests have been converted to Junit and is run as part of the Junit All test suite.

      1. rmTestPreparedStatementMethods.stat
        0.7 kB
        Øystein Grøvlen
      2. rmTestPreparedStatementMethods.diff
        334 kB
        Øystein Grøvlen
      3. rmTestPreparedStatementMethods_v2.diff
        334 kB
        Øystein Grøvlen

        Issue Links

          Activity

          Hide
          Øystein Grøvlen added a comment -

          A closer look shows that while TestPreparedStatementMethods seem to contain the same tests as PreparedStatementTest, this is not so for TestResultSetMethods and TestConnectionMethods and the corresponding ResultSetTest and ConnectionTest. Hence, there are probably a few test cases that needs to be converted to Junit before the old jdbc40 suite can be removed.

          Show
          Øystein Grøvlen added a comment - A closer look shows that while TestPreparedStatementMethods seem to contain the same tests as PreparedStatementTest, this is not so for TestResultSetMethods and TestConnectionMethods and the corresponding ResultSetTest and ConnectionTest. Hence, there are probably a few test cases that needs to be converted to Junit before the old jdbc40 suite can be removed.
          Hide
          Øystein Grøvlen added a comment -

          The attached patch removes the TestPreparedStatementMethods test and associates files that are not used by other tests.

          An inspection of PreparedStatementTest showed that testing of isPoolable/setPoolable on a closed PreparedStatement was omitted when the test was ported to Junit. The patch adds test cases for this to PreparedStatementTest.

          I have run derbyall and the PreparedStatementTest without errors.

          Show
          Øystein Grøvlen added a comment - The attached patch removes the TestPreparedStatementMethods test and associates files that are not used by other tests. An inspection of PreparedStatementTest showed that testing of isPoolable/setPoolable on a closed PreparedStatement was omitted when the test was ported to Junit. The patch adds test cases for this to PreparedStatementTest. I have run derbyall and the PreparedStatementTest without errors.
          Hide
          Knut Anders Hatlen added a comment -

          I think the patch looks good, Øystein. Some minor comments:

          • testIsPoolableOnClosed has this piece of code:
            + psClosed.close();
            + if (psClosed.isPoolable())
            + fail("Should throw exception on closed statement");

          Shouldn't this fail if psClosed.isPoolable() returns false as well?

          • Couldn't the test*OnClosed methods have used ps instead of creating their own PreparedStatement? ps seems to be initialized in the setUp() method for each fixture.
          • testSetPoolable and testIsPoolable have code like this (not originally written by you, only reindented)
            + if (ps.isPoolable())
            + fail("Expected a non-poolable statement");

          Perhaps it would be better with assertTrue/assertFalse in these cases?

          • Not introduced by you, but I noticed that tearDown() doesn't close ps and s, and it doesn't set any of the statements to null. Perhaps you could fix that as well while you're at it?
          Show
          Knut Anders Hatlen added a comment - I think the patch looks good, Øystein. Some minor comments: testIsPoolableOnClosed has this piece of code: + psClosed.close(); + if (psClosed.isPoolable()) + fail("Should throw exception on closed statement"); Shouldn't this fail if psClosed.isPoolable() returns false as well? Couldn't the test*OnClosed methods have used ps instead of creating their own PreparedStatement? ps seems to be initialized in the setUp() method for each fixture. testSetPoolable and testIsPoolable have code like this (not originally written by you, only reindented) + if (ps.isPoolable()) + fail("Expected a non-poolable statement"); Perhaps it would be better with assertTrue/assertFalse in these cases? Not introduced by you, but I noticed that tearDown() doesn't close ps and s, and it doesn't set any of the statements to null. Perhaps you could fix that as well while you're at it?
          Hide
          Øystein Grøvlen added a comment -

          Knut Anders, thanks for the review and the good advice.

          The attached patch, rmTestPreparedStatementMethods_v2.diff, should address your comments.

          Show
          Øystein Grøvlen added a comment - Knut Anders, thanks for the review and the good advice. The attached patch, rmTestPreparedStatementMethods_v2.diff, should address your comments.
          Hide
          Knut Anders Hatlen added a comment -

          Thanks Øystein. Committed revision 540150.

          Show
          Knut Anders Hatlen added a comment - Thanks Øystein. Committed revision 540150.
          Hide
          Myrna van Lunteren added a comment -

          Dan made a step to remove the last 'junit' test - jdbc4/AutoloadBooing.junit.
          Anyone feel up to converting jdbc4/TestResultSetMethods.java and jdbc4/TestConnectionMethods.java to junit in the next 7 days?
          Then this issue can be closed for 10.3.
          Otherwise, I'll move it off the 10.3 list, even though changes have gone in for 10.3 - this is not something that would affect end-users...

          Show
          Myrna van Lunteren added a comment - Dan made a step to remove the last 'junit' test - jdbc4/AutoloadBooing.junit. Anyone feel up to converting jdbc4/TestResultSetMethods.java and jdbc4/TestConnectionMethods.java to junit in the next 7 days? Then this issue can be closed for 10.3. Otherwise, I'll move it off the 10.3 list, even though changes have gone in for 10.3 - this is not something that would affect end-users...
          Hide
          Ramin Moazeni added a comment -

          Hi Myrna

          DERBY-2854 and DERBY-2855 is created to convert jdbc4/TestResultSetMethods.java and jdbc4/TestConnectionMethods.java to JUnit.

          Ramin

          Show
          Ramin Moazeni added a comment - Hi Myrna DERBY-2854 and DERBY-2855 is created to convert jdbc4/TestResultSetMethods.java and jdbc4/TestConnectionMethods.java to JUnit. Ramin
          Hide
          Myrna van Lunteren added a comment -

          I removed the last forgotten file - jdbc40.properties - with revision 612531, most references had been removed with the checkin for DERBY-2855.
          Not sure whom to assign this to as a number of people helped on different tests.

          Show
          Myrna van Lunteren added a comment - I removed the last forgotten file - jdbc40.properties - with revision 612531, most references had been removed with the checkin for DERBY-2855 . Not sure whom to assign this to as a number of people helped on different tests.
          Hide
          Øystein Grøvlen added a comment -

          All JDBC 4 tests are now part of the Junit All suite.

          Show
          Øystein Grøvlen added a comment - All JDBC 4 tests are now part of the Junit All suite.

            People

            • Assignee:
              Unassigned
              Reporter:
              Øystein Grøvlen
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development