Derby
  1. Derby
  2. DERBY-2999

convert lang/lockTable.sql to Junit

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 10.3.1.4
    • Fix Version/s: 10.11.1.1
    • Component/s: Test
    • Labels:
      None
    1. DERBY-2999.diff_v3
      24 kB
      Myrna van Lunteren
    2. DERBY-2999.diff-v1
      26 kB
      Ravinder Reddy
    3. DERBY-2999.diff-v2
      27 kB
      Ravinder Reddy
    4. DERBY-2999.stat_v3
      0.6 kB
      Myrna van Lunteren
    5. STATUS-2999-v1
      0.7 kB
      Ravinder Reddy
    6. STATUS-v2
      0.7 kB
      Ravinder Reddy

      Activity

      Hide
      Ravinder Reddy added a comment -

      It is ready for commit.
      Thank You

      Show
      Ravinder Reddy added a comment - It is ready for commit. Thank You
      Hide
      Kristian Waagan added a comment -

      Hello Ravinder,

      I had a look at the patch, and in my opinion it is not quite ready for commit.

      My initial comments:
      1) The drop table statements in the setUp method fails.
      2) After fixing 1, the test takes over 300 seconds to complete. Is this what you intended?
      3) Does the new test actually test/verify the same things as the old one?
      4) A mix of tabs and spaces are used for indentation.

      A nit, but don't you think it would be nicer with a blank line between methods?
      I can have another look at the test when the initial comments are addressed.

      Thanks for working on the tests

      Show
      Kristian Waagan added a comment - Hello Ravinder, I had a look at the patch, and in my opinion it is not quite ready for commit. My initial comments: 1) The drop table statements in the setUp method fails. 2) After fixing 1, the test takes over 300 seconds to complete. Is this what you intended? 3) Does the new test actually test/verify the same things as the old one? 4) A mix of tabs and spaces are used for indentation. A nit, but don't you think it would be nicer with a blank line between methods? I can have another look at the test when the initial comments are addressed. Thanks for working on the tests
      Hide
      Ravinder Reddy added a comment -

      Thank You Waagan for your Review.
      I addressed your comments in the next patch.

      1) now it's working

      2) It takes bcoz of the following reason.
      The table is locked by a user U1 and the user U2 , trying to lock the same table ,
      get SQLExceptio "40XL1" (A lock could not be obtained within the time requested).
      And hence execution time depends on lock timeout.

      3) yes it is.

      4) resolved.

      Thank You.

      Show
      Ravinder Reddy added a comment - Thank You Waagan for your Review. I addressed your comments in the next patch. 1) now it's working 2) It takes bcoz of the following reason. The table is locked by a user U1 and the user U2 , trying to lock the same table , get SQLExceptio "40XL1" (A lock could not be obtained within the time requested). And hence execution time depends on lock timeout. 3) yes it is. 4) resolved. Thank You.
      Hide
      Kristian Waagan added a comment -

      Thanks for addressing comment 1 and 4.

      Regarding comment 2, I'm wondering if you should use a decorator to set the appropriate Derby properties to make the test run faster. In the old test derby.storage.rowLocking=false was also set. Is this no longer needed?

      For comment 3, I don't see any verification of the locks by querying the lock table. If that is a conscious choice, then consider my comment addressed. However, I don't really understand what is tested in "-- verify that user getting error on lock table doesn't get rolled back". The test case does not seem to verify that the other user still has its locks after the exception is thrown.
      And in the method "testTXvsTXLocks", maybe trying to insert a row with connection c2 would be a nice addition to the test?

      Another small thing I noticed, is that dropping the table in the test methods should not be required since you run with auto commit off and do a rollback in the tearDown-method, and also the tables are dropped in setUp.
      The main reason for removing it from the test method would be to make it smaller and have it only contain code relevant to what's being tested, the secondary reason would be to do things only once (or twice).

      regards,

      Show
      Kristian Waagan added a comment - Thanks for addressing comment 1 and 4. Regarding comment 2, I'm wondering if you should use a decorator to set the appropriate Derby properties to make the test run faster. In the old test derby.storage.rowLocking=false was also set. Is this no longer needed? For comment 3, I don't see any verification of the locks by querying the lock table. If that is a conscious choice, then consider my comment addressed. However, I don't really understand what is tested in "-- verify that user getting error on lock table doesn't get rolled back". The test case does not seem to verify that the other user still has its locks after the exception is thrown. And in the method "testTXvsTXLocks", maybe trying to insert a row with connection c2 would be a nice addition to the test? Another small thing I noticed, is that dropping the table in the test methods should not be required since you run with auto commit off and do a rollback in the tearDown-method, and also the tables are dropped in setUp. The main reason for removing it from the test method would be to make it smaller and have it only contain code relevant to what's being tested, the secondary reason would be to do things only once (or twice). regards,
      Hide
      Myrna van Lunteren added a comment -

      This issue appears stalled. Ravinder, if you're not working on this anymore, can you unassign yourself so someone else can finish it?

      Show
      Myrna van Lunteren added a comment - This issue appears stalled. Ravinder, if you're not working on this anymore, can you unassign yourself so someone else can finish it?
      Hide
      Kathey Marsden added a comment -

      Unassigning due to inactivity.

      Show
      Kathey Marsden added a comment - Unassigning due to inactivity.
      Hide
      Myrna van Lunteren added a comment -

      I tried Ravinder's patch, but it wasn't working for me, for starters, the dropping and closing left or created locks. Having the statements as global variables also caused locks to remain in place, and finally the schemas didn't get created.

      So I basically started from scratch, but added Ravinder's check for the Lock Table command on sys.systables.

      Now the test uses a decorateSQL to create the schemas.
      I did add Kristian's suggestion for an extra insert in the TXvsTX test.
      I also have included the selects from syscs_diag.lock_table.
      I also used the assert which checks on an array of errors instead of just 40XL1. Possibly this assert wasn't available when Ravinder made his version.
      I also used the DatabasePropertyTestSetup, so we only wait as per the original test properties.

      Running tests...

      Show
      Myrna van Lunteren added a comment - I tried Ravinder's patch, but it wasn't working for me, for starters, the dropping and closing left or created locks. Having the statements as global variables also caused locks to remain in place, and finally the schemas didn't get created. So I basically started from scratch, but added Ravinder's check for the Lock Table command on sys.systables. Now the test uses a decorateSQL to create the schemas. I did add Kristian's suggestion for an extra insert in the TXvsTX test. I also have included the selects from syscs_diag.lock_table. I also used the assert which checks on an array of errors instead of just 40XL1. Possibly this assert wasn't available when Ravinder made his version. I also used the DatabasePropertyTestSetup, so we only wait as per the original test properties. Running tests...
      Hide
      ASF subversion and git services added a comment -

      Commit 1515691 from Myrna van Lunteren in branch 'code/trunk'
      [ https://svn.apache.org/r1515691 ]

      DERBY-2999; convert lang/lockTable.sql to Junit
      converted the test lang/lockTable.sql to lang/LockTableTest.java

      Show
      ASF subversion and git services added a comment - Commit 1515691 from Myrna van Lunteren in branch 'code/trunk' [ https://svn.apache.org/r1515691 ] DERBY-2999 ; convert lang/lockTable.sql to Junit converted the test lang/lockTable.sql to lang/LockTableTest.java
      Hide
      Myrna van Lunteren added a comment -

      Test ran cleanly. Committed (rev. 1515691), resolving.

      Show
      Myrna van Lunteren added a comment - Test ran cleanly. Committed (rev. 1515691), resolving.
      Hide
      ASF subversion and git services added a comment -

      Commit 1515737 from Knut Anders Hatlen in branch 'code/trunk'
      [ https://svn.apache.org/r1515737 ]

      DERBY-2999: Fix javadoc warning

      Show
      ASF subversion and git services added a comment - Commit 1515737 from Knut Anders Hatlen in branch 'code/trunk' [ https://svn.apache.org/r1515737 ] DERBY-2999 : Fix javadoc warning
      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:
          Myrna van Lunteren
          Reporter:
          Ravinder Reddy
        • Votes:
          0 Vote for this issue
          Watchers:
          3 Start watching this issue

          Dates

          • Created:
            Updated:
            Resolved:

            Development