Derby
  1. Derby
  2. DERBY-2461

Convert lang/procedure.java to junit

    Details

    • Type: Task Task
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 10.3.1.4, 10.10.1.1
    • Component/s: Test
    • Labels:
      None

      Description

      Convert the lang/procedure.java test from the old harness to junit.

      1. DERBY-2461_stat.txt
        1 kB
        Kathey Marsden
      2. DERBY-2461_diff.txt
        524 kB
        Kathey Marsden
      3. derby-2461-pre.diff
        16 kB
        Jean T. Anderson

        Issue Links

          Activity

          Hide
          Jean T. Anderson added a comment -

          I need to focus attention elsewhere for a while, so can't work on converting this test right now and will unassign myself.

          I attached a patch with how far I got so far just in case somebody else is interested in picking it up. You're welcome to use it as a starting point (or toss it and start over). If nobody has picked it up by the time I can focus time on it again, I'll pick it back up.

          The patch sets up the JUnit framework and converts just the first two tests in lang/procedure.java: testNegative and testDelayedClassChecking, and it splits testNegative, which I thought was kind of long, into two methods:

          testCreateRoutineErrors
          testMethodSignatureDerby258

          testCreateRoutineErrors still uses the old statementExceptionExpected() method from procedure.java, but with arguments swapped around to match assertStatementError. It should be able to use the BaseJDBCTestCase assertStatementError method (testMethodSignatureDerby258 uses it just fine, as does the XMLTypeAndOpsTest suite). The cause is probably staring me right in the face and I'm not seeing it.

          Show
          Jean T. Anderson added a comment - I need to focus attention elsewhere for a while, so can't work on converting this test right now and will unassign myself. I attached a patch with how far I got so far just in case somebody else is interested in picking it up. You're welcome to use it as a starting point (or toss it and start over). If nobody has picked it up by the time I can focus time on it again, I'll pick it back up. The patch sets up the JUnit framework and converts just the first two tests in lang/procedure.java: testNegative and testDelayedClassChecking, and it splits testNegative, which I thought was kind of long, into two methods: testCreateRoutineErrors testMethodSignatureDerby258 testCreateRoutineErrors still uses the old statementExceptionExpected() method from procedure.java, but with arguments swapped around to match assertStatementError. It should be able to use the BaseJDBCTestCase assertStatementError method (testMethodSignatureDerby258 uses it just fine, as does the XMLTypeAndOpsTest suite). The cause is probably staring me right in the face and I'm not seeing it.
          Hide
          Kathey Marsden added a comment -

          Instead of making a new lang/ProcedureTest class, I think I'll add the tests to the existing jdbcapi/ProcedureTest and then move util/ProcedureTest to junit/TestingProcedures, so we are left with only one ProcedureTest. Please let me know if this does not sound like a reasonable approach,

          Show
          Kathey Marsden added a comment - Instead of making a new lang/ProcedureTest class, I think I'll add the tests to the existing jdbcapi/ProcedureTest and then move util/ProcedureTest to junit/TestingProcedures, so we are left with only one ProcedureTest. Please let me know if this does not sound like a reasonable approach,
          Hide
          Daniel John Debrunner added a comment -

          I don't think junit is the correct location for procedures used in functional tests. The junit package is for classes associated with running tests,
          utilitly assert methods and decorators.

          A better approach may be to make the methods part of the same class as the actual test that uses them, or even a separate but related class, e.g. for the test XXXTest a class XXXRoutines.

          Show
          Daniel John Debrunner added a comment - I don't think junit is the correct location for procedures used in functional tests. The junit package is for classes associated with running tests, utilitly assert methods and decorators. A better approach may be to make the methods part of the same class as the actual test that uses them, or even a separate but related class, e.g. for the test XXXTest a class XXXRoutines.
          Hide
          Kathey Marsden added a comment -

          The test also makes use of util/Formatters. I had planned to move the methods from this class to junit/Utilities but perhaps junit/Utilities is not where they belong either. Where should shared utility methods go?

          Show
          Kathey Marsden added a comment - The test also makes use of util/Formatters. I had planned to move the methods from this class to junit/Utilities but perhaps junit/Utilities is not where they belong either. Where should shared utility methods go?
          Hide
          Daniel John Debrunner added a comment -

          Just a thought on the single procedure test.

          One existing test (jdbcapi) is testing the JDBC api aspect of calling procedures (e.g. multiple result set handling)

          The other is testing the SQL aspect of procedures, create syntax, sql control (read-only etc.) SQL parameter passing.

          There's probably some overlap but they are two different areas.

          Show
          Daniel John Debrunner added a comment - Just a thought on the single procedure test. One existing test (jdbcapi) is testing the JDBC api aspect of calling procedures (e.g. multiple result set handling) The other is testing the SQL aspect of procedures, create syntax, sql control (read-only etc.) SQL parameter passing. There's probably some overlap but they are two different areas.
          Hide
          Kathey Marsden added a comment -

          In converting this test I found what I think may be a bug, but I am not sure. In testing commit with multiple resultset I see the following lock counts

          Embedded Network Server
          autocommit|noautocommit|statement autocommit|noautocommit|statement
          after execution 1 1 1 8 8 8
          after next on rs 3 3 3 8 8 8
          after values 1 0 8
          after getMoreResults 2 2 0 7 7 7
          after next on 2nd rs 7 7 0 7 7 7
          after getMoreResults 0 7 0 7 7 7

          I think the increase in number of locks held after execution and after the first next is ok because of prefetch, but I think that the locks not being released after the values 1 statement execution for Network Server and after the final getMoreResults is a bug. I am not totally sure however and would appreciate other opinions. The reason that network server holds these locks is because it retrieves and saves all the resultsets and specifies getMoreResults(JDBC30Translation.KEEP_CURRENT_RESULT).

          Show
          Kathey Marsden added a comment - In converting this test I found what I think may be a bug, but I am not sure. In testing commit with multiple resultset I see the following lock counts Embedded Network Server autocommit|noautocommit|statement autocommit|noautocommit|statement after execution 1 1 1 8 8 8 after next on rs 3 3 3 8 8 8 after values 1 0 8 after getMoreResults 2 2 0 7 7 7 after next on 2nd rs 7 7 0 7 7 7 after getMoreResults 0 7 0 7 7 7 I think the increase in number of locks held after execution and after the first next is ok because of prefetch, but I think that the locks not being released after the values 1 statement execution for Network Server and after the final getMoreResults is a bug. I am not totally sure however and would appreciate other opinions. The reason that network server holds these locks is because it retrieves and saves all the resultsets and specifies getMoreResults(JDBC30Translation.KEEP_CURRENT_RESULT).
          Hide
          Kathey Marsden added a comment -

          Attaching a patch for this issue. It was a fairly large test to convert so I would appreciate review. Per Dan's suggestion I moved the stored procedure methods to the new test LangProcedureTest.java

          Show
          Kathey Marsden added a comment - Attaching a patch for this issue. It was a fairly large test to convert so I would appreciate review. Per Dan's suggestion I moved the stored procedure methods to the new test LangProcedureTest.java
          Hide
          Daniel John Debrunner added a comment -

          Several of these fixtures could work in J2ME/JSR 169. Only procedures that need to execute SQL cannot work in J2ME (ie. those that require the jdbc:default:connection). This could be a followup cleanup in a separate issue if required.

          Show
          Daniel John Debrunner added a comment - Several of these fixtures could work in J2ME/JSR 169. Only procedures that need to execute SQL cannot work in J2ME (ie. those that require the jdbc:default:connection). This could be a followup cleanup in a separate issue if required.
          Hide
          Dag H. Wanvik added a comment -

          DERBY-2927 contains a patch which wakes up this test again (for embedded only, cf. Kathey's observation of the lock differences with the client driver).

          Show
          Dag H. Wanvik added a comment - DERBY-2927 contains a patch which wakes up this test again (for embedded only, cf. Kathey's observation of the lock differences with the client driver).
          Hide
          Dag H. Wanvik added a comment - - edited

          Kathey, I see you had closed this issue. Did you decide to not complete this conversion? If so, why is the source still in there? In any case, I resuscicated it, cf DERBY-2927/DERBY-5945. Do you think it would be valuable to retain it now that it works?

          Show
          Dag H. Wanvik added a comment - - edited Kathey, I see you had closed this issue. Did you decide to not complete this conversion? If so, why is the source still in there? In any case, I resuscicated it, cf DERBY-2927 / DERBY-5945 . Do you think it would be valuable to retain it now that it works?
          Hide
          Kathey Marsden added a comment -

          I don't remember much about this but I did check in http://svn.apache.org/viewvc?view=revision&revision=525332 for this test conversion which included removing procedure.java. I would think now the thing to do would be re-resolve this issue and then open up a new one to enable the test on client or is there more work to be done?

          Thanks

          Kathey

          Show
          Kathey Marsden added a comment - I don't remember much about this but I did check in http://svn.apache.org/viewvc?view=revision&revision=525332 for this test conversion which included removing procedure.java. I would think now the thing to do would be re-resolve this issue and then open up a new one to enable the test on client or is there more work to be done? Thanks Kathey
          Hide
          Dag H. Wanvik added a comment - - edited

          Thanks! I'll resolve this as soon as I commit the changes for DERBY-5945, and make a new issue for enabling this test for the client.

          Show
          Dag H. Wanvik added a comment - - edited Thanks! I'll resolve this as soon as I commit the changes for DERBY-5945 , and make a new issue for enabling this test for the client.
          Hide
          Dag H. Wanvik added a comment -

          DERBY-5945 enables LangProceduresTest as part of the lang suite for both embedded and client.

          Show
          Dag H. Wanvik added a comment - DERBY-5945 enables LangProceduresTest as part of the lang suite for both embedded and client.
          Hide
          Knut Anders Hatlen added a comment -

          [bulk update] Close all resolved issues that haven't been updated for more than one year.

          Show
          Knut Anders Hatlen added a comment - [bulk update] Close all resolved issues that haven't been updated for more than one year.

            People

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

              Dates

              • Created:
                Updated:
                Resolved:

                Development