Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 10.3.1.4
    • Component/s: Test
    • Labels:
      None
    1. DERBY-2562_stat_290507.txt
      0.2 kB
      Ugo Matrangolo
    2. DERBY-2562_diff_290507.txt
      14 kB
      Ugo Matrangolo
    3. DERBY-2562_stat_200507.txt
      0.2 kB
      Ugo Matrangolo
    4. DERBY-2562_diff_200507.txt
      14 kB
      Ugo Matrangolo

      Activity

      Hide
      Kathey Marsden added a comment -

      Thanks Ugo for the patch. I delted the old test file and canon as they are no longer needed.

      Kathey

      Show
      Kathey Marsden added a comment - Thanks Ugo for the patch. I delted the old test file and canon as they are no longer needed. Kathey
      Hide
      Ugo Matrangolo added a comment -

      Hi Knut,

      thank you for your comments. Attached there is the modified code.

      --Ugo.

      Show
      Ugo Matrangolo added a comment - Hi Knut, thank you for your comments. Attached there is the modified code. --Ugo.
      Hide
      Knut Anders Hatlen added a comment -

      Hi Ugo,

      The patch looks good to me. A couple of tiny comments:

      • I think joinStmt and distinctStmt should have been instance variables, not static variables. With that change, you'll also need a tearDown() method which closes those objects, sets them to null and calls super.tearDown()
      • checkAllCa1() has a finally block with clean-up code. Finally blocks could be dangerous in tests since errors that happen in the finally block will hide errors in the try block. In this case, I think it would be OK just to remove the try/finally.

      Thanks!

      Show
      Knut Anders Hatlen added a comment - Hi Ugo, The patch looks good to me. A couple of tiny comments: I think joinStmt and distinctStmt should have been instance variables, not static variables. With that change, you'll also need a tearDown() method which closes those objects, sets them to null and calls super.tearDown() checkAllCa1() has a finally block with clean-up code. Finally blocks could be dangerous in tests since errors that happen in the finally block will hide errors in the try block. In this case, I think it would be OK just to remove the try/finally. Thanks!
      Hide
      Ugo Matrangolo added a comment -

      The attached diff files are the conversion of the Spillhash harness test to JUnit.

      I split the the test in two distinct parts: the first run is against a light load on the test table where the second run exercise the same table filled with a lot of rows. I used a CleanDatabaseTestSetup to create the initial schema on the db, then two distinct BaseJDBCTestSetup to fill tables for the two distinct phases.

      The rest of the code in only a 1-1 translation using assert/fail statements in place of the old checks.

      Please review,
      --Ugo.

      Show
      Ugo Matrangolo added a comment - The attached diff files are the conversion of the Spillhash harness test to JUnit. I split the the test in two distinct parts: the first run is against a light load on the test table where the second run exercise the same table filled with a lot of rows. I used a CleanDatabaseTestSetup to create the initial schema on the db, then two distinct BaseJDBCTestSetup to fill tables for the two distinct phases. The rest of the code in only a 1-1 translation using assert/fail statements in place of the old checks. Please review, --Ugo.
      Hide
      Ugo Matrangolo added a comment -

      Spillhash.java conversion to JUnit.

      Show
      Ugo Matrangolo added a comment - Spillhash.java conversion to JUnit.

        People

        • Assignee:
          Ugo Matrangolo
          Reporter:
          Ugo Matrangolo
        • Votes:
          0 Vote for this issue
          Watchers:
          0 Start watching this issue

          Dates

          • Created:
            Updated:
            Resolved:

            Development