Derby
  1. Derby
  2. DERBY-2398

Convert jdbcapi/autoGeneratedJdbc30.java to junit

    Details

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

      Description

      Convert the jdbcapi/autoGeneratedJdbc30.java test from the old harness to junit.

      1. test_file_ignore_me.txt
        0.1 kB
        Jean T. Anderson
      2. derby-2398-pre.diff
        24 kB
        Jean T. Anderson
      3. patch-2398.diff
        44 kB
        Jean T. Anderson

        Activity

        Hide
        Jean T. Anderson added a comment -

        test upload

        Show
        Jean T. Anderson added a comment - test upload
        Hide
        Jean T. Anderson added a comment -

        derby-2398-pre.diff converts about half of jdbcapi/autoGeneratedJdbc30.java test to JUnit.

        1) It adds a prepareStatement(String sql, int autoGeneratedKeys) method to junit/BaseJDBCTestCase.java.

        2) This suite runs only if (JDBC.vmSupportsJDBC3()). Is checking for JDBC3 sufficient to handle J2ME/JSR169? --There's one test that will do a DriverManager call.

        3) I made quite a few changes to the original test that are detailed below. For now, I have left in temporary System.out.println statements such as this:

        System.out.println ("Old Test 1"); // temporary

        along with prints of values to make it easy to doublecheck results against the original master.

        Changes to original program:

        4) The original test did SET INCREMENT BY in ALTER statements because "set by increment not yet supported for create table..."
        The new JUnit test includes the increment in the CREATE TABLE statement.

        5) There are many tests in the old version, roughly 25 * 4 queries each (most insert queries are tested four ways), so I divided them up into separate, standalone fixtures.
        However! This means each fixture creates its own Statement and PreparedStatement. The old master output this note:

        System.out.println("Test 8 - create a new statement and request for generated keys on it after doing an insert into ");

        This led me to think there might have been a difference in behavior between reusing the same Statement and create a new one, but I haven't see any difference in behavior compared to the original master.

        If the standalone fixtures path I'm on is not right, I'm only half way through the tests and it would be easy to redo the test so that a single fixture calls the separate test methods with the same Statements and PreparedStatements. Having separate methods will still make the test easier to read and follow.

        6) I assume that what matters about the key generated is null vs. not-null value (I'm not paying attention to the actual values). That's simple, but if it's wrong I can change the code to test for actual values.

        Show
        Jean T. Anderson added a comment - derby-2398-pre.diff converts about half of jdbcapi/autoGeneratedJdbc30.java test to JUnit. 1) It adds a prepareStatement(String sql, int autoGeneratedKeys) method to junit/BaseJDBCTestCase.java. 2) This suite runs only if (JDBC.vmSupportsJDBC3()). Is checking for JDBC3 sufficient to handle J2ME/JSR169? --There's one test that will do a DriverManager call. 3) I made quite a few changes to the original test that are detailed below. For now, I have left in temporary System.out.println statements such as this: System.out.println ("Old Test 1"); // temporary along with prints of values to make it easy to doublecheck results against the original master. Changes to original program: 4) The original test did SET INCREMENT BY in ALTER statements because "set by increment not yet supported for create table..." The new JUnit test includes the increment in the CREATE TABLE statement. 5) There are many tests in the old version, roughly 25 * 4 queries each (most insert queries are tested four ways), so I divided them up into separate, standalone fixtures. However! This means each fixture creates its own Statement and PreparedStatement. The old master output this note: System.out.println("Test 8 - create a new statement and request for generated keys on it after doing an insert into "); This led me to think there might have been a difference in behavior between reusing the same Statement and create a new one, but I haven't see any difference in behavior compared to the original master. If the standalone fixtures path I'm on is not right, I'm only half way through the tests and it would be easy to redo the test so that a single fixture calls the separate test methods with the same Statements and PreparedStatements. Having separate methods will still make the test easier to read and follow. 6) I assume that what matters about the key generated is null vs. not-null value (I'm not paying attention to the actual values). That's simple, but if it's wrong I can change the code to test for actual values.
        Hide
        Jean T. Anderson added a comment -

        2) I think I can answer my own question: yes (I remembered belatedly that JDBC.vmSupportsJDBC2 was adequate for CallableTest.java)

        comments on the rest would be welcome.

        Show
        Jean T. Anderson added a comment - 2) I think I can answer my own question: yes (I remembered belatedly that JDBC.vmSupportsJDBC2 was adequate for CallableTest.java) comments on the rest would be welcome.
        Hide
        Jean T. Anderson added a comment -

        Incidently, in the old test, tests 11 and 12 output this message:

        "expected to see resultset with one row of NULL value but instead get one row of non-NULL value from getGeneratedKeys"

        Test 11 does a commit, followed by an insert into a table with no generated column.
        Test 12 does a rollback, followed by an insert into a table with no generated column.

        In both cases, the previous transaction had a one-row insert on a table with a auto-generated column.

        I don't know if that indicates a potential problem in Derby handling. I'll be sure to copy those comments into code comments in the JUnit version.

        Show
        Jean T. Anderson added a comment - Incidently, in the old test, tests 11 and 12 output this message: "expected to see resultset with one row of NULL value but instead get one row of non-NULL value from getGeneratedKeys" Test 11 does a commit, followed by an insert into a table with no generated column. Test 12 does a rollback, followed by an insert into a table with no generated column. In both cases, the previous transaction had a one-row insert on a table with a auto-generated column. I don't know if that indicates a potential problem in Derby handling. I'll be sure to copy those comments into code comments in the JUnit version.
        Hide
        Daniel John Debrunner added a comment -

        jta> 6) I assume that what matters about the key generated is null vs. not-null value (I'm not paying attention to the actual values). That's simple, but if it's wrong I can change the code to test for actual values.

        Testing the values is best as the purpose (I assume) of this test is to test the JDBC feature that returns auto genrated keys, so checking it's returning the correct value would be good.

        Show
        Daniel John Debrunner added a comment - jta> 6) I assume that what matters about the key generated is null vs. not-null value (I'm not paying attention to the actual values). That's simple, but if it's wrong I can change the code to test for actual values. Testing the values is best as the purpose (I assume) of this test is to test the JDBC feature that returns auto genrated keys, so checking it's returning the correct value would be good.
        Hide
        Jean T. Anderson added a comment -

        Dan wrote:
        > Testing the values is best as the purpose (I assume) of this test is to test the JDBC feature that returns auto genrated keys, so checking it's returning the correct value would be good.

        ack, the old test 13 needs to verify that the value obtained inside a savepoint is the same after it does a rollback. I had missed that fine point, so I do need to check actual values.

        thanks for the feedback, Dan.

        Show
        Jean T. Anderson added a comment - Dan wrote: > Testing the values is best as the purpose (I assume) of this test is to test the JDBC feature that returns auto genrated keys, so checking it's returning the correct value would be good. ack, the old test 13 needs to verify that the value obtained inside a savepoint is the same after it does a rollback. I had missed that fine point, so I do need to check actual values. thanks for the feedback, Dan.
        Hide
        Jean T. Anderson added a comment -

        Patch derby-2398.diff implements all tests. I'll wait for a day or two to give anyone who has time/inclination to review it (and many thanks in advance).

        Three specific questions/notes are below.

        1) The original autoGeneratedJdbc30_app.properties file has this setting:

        runwithj9=false

        Does returning a empty suite if (!JDBC.vmSupportsJDBC3()) do what's needed for j9? Or should I do something special?

        2) I posted to derby-dev* asking if getGeneratedKeys() should return a NULL key after commit/rollback. Depending on that answer, I'll open a Jira issue if one is needed and update comments for the relevant methods in the code.

        3) The testResultSetGarbageCollection() method is noticeably slow (Test 20 in
        the old code).

        The purpose of the test was to verify a fix for an old issue in which the ResultSet returned by getGenerateKeys() was incorrectly tied to the activation of the PreparedStatement. So, when the ResultSet was garbage collected, the activation was closed, resulting in an "Activation closed, operation execute not permitted" exception on subsequent executes.

        So, the test does this:
        for (int i = 0; i < 100; i++)
        {
        ps.setInt(1, 100+i);
        ps.executeUpdate();

        ResultSet rs = ps.getGeneratedKeys();
        while (rs.next())

        { rs.getInt(1); }

        rs.close();
        conn.commit();

        System.runFinalization();
        System.gc();
        System.runFinalization();
        System.gc();
        }

        I wonder if this is overkill. Could it be nipped back without degrading the effectiveness of the test?

        Show
        Jean T. Anderson added a comment - Patch derby-2398.diff implements all tests. I'll wait for a day or two to give anyone who has time/inclination to review it (and many thanks in advance). Three specific questions/notes are below. 1) The original autoGeneratedJdbc30_app.properties file has this setting: runwithj9=false Does returning a empty suite if (!JDBC.vmSupportsJDBC3()) do what's needed for j9? Or should I do something special? 2) I posted to derby-dev* asking if getGeneratedKeys() should return a NULL key after commit/rollback. Depending on that answer, I'll open a Jira issue if one is needed and update comments for the relevant methods in the code. http://www.nabble.com/Should-getGeneratedKeys%28%29-return-a-NULL-key-after-commit-rollback--p9505841.html 3) The testResultSetGarbageCollection() method is noticeably slow (Test 20 in the old code). The purpose of the test was to verify a fix for an old issue in which the ResultSet returned by getGenerateKeys() was incorrectly tied to the activation of the PreparedStatement. So, when the ResultSet was garbage collected, the activation was closed, resulting in an "Activation closed, operation execute not permitted" exception on subsequent executes. So, the test does this: for (int i = 0; i < 100; i++) { ps.setInt(1, 100+i); ps.executeUpdate(); ResultSet rs = ps.getGeneratedKeys(); while (rs.next()) { rs.getInt(1); } rs.close(); conn.commit(); System.runFinalization(); System.gc(); System.runFinalization(); System.gc(); } I wonder if this is overkill. Could it be nipped back without degrading the effectiveness of the test?
        Hide
        Jean T. Anderson added a comment -

        Regarding the j9 question, Myrna posted this comment to derby-dev [1]:
        > Just jumping in on this part - we don't need to worry about j9 per se.
        > That was created for IBM's jclMax implementation with wctme5.7, but it
        > was based on jdk 1.3, which we no longer support with our 10.3
        > codeline.
        > j9_foundation is (with 10.3) changed to mean weme6.1's implementation,
        > which is J2ME 1.1 / JSR169; that we still worry about and it can be
        > addressed by checking JDBC.vmSupportsJSR169. JDBC.vmSupportsJdbc20()
        > will return false for that JVM, as (I think) will vmSupportsJdbc30().

        I didn't see any response to the question if getGeneratedKeys() should return a NULL key after commit/rollback.[2]

        If there's no comment regarding the observation that testResultSetGarbageCollection() is slow (or any other objections raised), I'll go ahead and commit this work, but will note in the javadoc comment for this method that I think it's expensive. That way if anyone reviews tests later for performance issues she or he can more easily find it.

        [1] http://mail-archives.apache.org/mod_mbox/db-derby-dev/200703.mbox/%3cc25576af0703151747g2399c546hee36afcb5a09381a@mail.gmail.com%3e
        [2] http://mail-archives.apache.org/mod_mbox/db-derby-dev/200703.mbox/%3c45F9D7EF.1080806@bristowhill.com%3e

        Show
        Jean T. Anderson added a comment - Regarding the j9 question, Myrna posted this comment to derby-dev [1] : > Just jumping in on this part - we don't need to worry about j9 per se. > That was created for IBM's jclMax implementation with wctme5.7, but it > was based on jdk 1.3, which we no longer support with our 10.3 > codeline. > j9_foundation is (with 10.3) changed to mean weme6.1's implementation, > which is J2ME 1.1 / JSR169; that we still worry about and it can be > addressed by checking JDBC.vmSupportsJSR169. JDBC.vmSupportsJdbc20() > will return false for that JVM, as (I think) will vmSupportsJdbc30(). I didn't see any response to the question if getGeneratedKeys() should return a NULL key after commit/rollback. [2] If there's no comment regarding the observation that testResultSetGarbageCollection() is slow (or any other objections raised), I'll go ahead and commit this work, but will note in the javadoc comment for this method that I think it's expensive. That way if anyone reviews tests later for performance issues she or he can more easily find it. [1] http://mail-archives.apache.org/mod_mbox/db-derby-dev/200703.mbox/%3cc25576af0703151747g2399c546hee36afcb5a09381a@mail.gmail.com%3e [2] http://mail-archives.apache.org/mod_mbox/db-derby-dev/200703.mbox/%3c45F9D7EF.1080806@bristowhill.com%3e
        Hide
        Kathey Marsden added a comment -

        Looks like this test conversion was committed.

        Show
        Kathey Marsden added a comment - Looks like this test conversion was committed.

          People

          • Assignee:
            Jean T. Anderson
            Reporter:
            Jean T. Anderson
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development