Derby
  1. Derby
  2. DERBY-2539

convert lang/timestampArith.java to JUnit

    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

      Description

      convert lang/timestampArith.java to JUnit

      1. DERBY-2539_diff_200407.txt
        30 kB
        Ugo Matrangolo
      2. DERBY-2539_stat_200407.txt
        0.3 kB
        Ugo Matrangolo
      3. DERBY-2539_diff_210407.txt
        30 kB
        Ugo Matrangolo
      4. DERBY-2539_stat_210407.txt
        0.3 kB
        Ugo Matrangolo

        Activity

        Ugo Matrangolo created issue -
        Hide
        Ugo Matrangolo added a comment -

        Conversion of the timestampArith.java test. Please review. Thanx.

        Show
        Ugo Matrangolo added a comment - Conversion of the timestampArith.java test. Please review. Thanx.
        Ugo Matrangolo made changes -
        Field Original Value New Value
        Attachment DERBY-2539_stat_200407.txt [ 12355721 ]
        Attachment DERBY-2539_diff_200407.txt [ 12355720 ]
        Ugo Matrangolo made changes -
        Derby Info [Patch Available]
        Ugo Matrangolo made changes -
        Status Open [ 1 ] In Progress [ 3 ]
        Hide
        Kathey Marsden added a comment -

        Thanks Ugo for the patch. I only have two small comments.

        1) in runTest instead of try/catch blocks and failing on exception, you can just have the method throw the exception.

        2) When you make the patch please do it from the top of the code tree, so your patch starts with java/testing/org/...
        instead of org/...

        Show
        Kathey Marsden added a comment - Thanks Ugo for the patch. I only have two small comments. 1) in runTest instead of try/catch blocks and failing on exception, you can just have the method throw the exception. 2) When you make the patch please do it from the top of the code tree, so your patch starts with java/testing/org/... instead of org/...
        Kathey Marsden made changes -
        Derby Info [Patch Available]
        Hide
        Ugo Matrangolo added a comment -

        Thank you Kathey for getting the time to review my code.

        Reading the old test I think that I fail on exception when no exception is expected at all. IMHO getting an exception where no exception is expected is a condition worth of a fail() call as in the old code.

        I took advantage of your comment and rebuilded the patch from the source code root.

        Thank you,
        Ugo.

        Show
        Ugo Matrangolo added a comment - Thank you Kathey for getting the time to review my code. Reading the old test I think that I fail on exception when no exception is expected at all. IMHO getting an exception where no exception is expected is a condition worth of a fail() call as in the old code. I took advantage of your comment and rebuilded the patch from the source code root. Thank you, Ugo.
        Ugo Matrangolo made changes -
        Attachment DERBY-2539_diff_210407.txt [ 12355966 ]
        Attachment DERBY-2539_stat_210407.txt [ 12355967 ]
        Ugo Matrangolo made changes -
        Resolution Fixed [ 1 ]
        Status In Progress [ 3 ] Resolved [ 5 ]
        Hide
        Kathey Marsden added a comment -

        This issue is marked resolved, but I don't see that the patch has been committed yet.
        Does the patch still need to be committed for this issue? If so it should probably be reopenned until the patch has been committed.

        Show
        Kathey Marsden added a comment - This issue is marked resolved, but I don't see that the patch has been committed yet. Does the patch still need to be committed for this issue? If so it should probably be reopenned until the patch has been committed.
        Hide
        Kathey Marsden added a comment -

        I think I see what you mean about the fail statements. There are
        instances where you catch an exception and then throw it, where just
        letting the exception be thrown would suffice.
        For example:

        + try

        { + rs.close(); + }

        catch (SQLException sqle)

        { + throw sqle; }

        can be written as:
        rs.close();

        since runTests throws an SQLException

        Do you have any objection to my changing these cases before I checkin
        the change?

        Thanks

        Kathey

        Show
        Kathey Marsden added a comment - I think I see what you mean about the fail statements. There are instances where you catch an exception and then throw it, where just letting the exception be thrown would suffice. For example: + try { + rs.close(); + } catch (SQLException sqle) { + throw sqle; } can be written as: rs.close(); since runTests throws an SQLException Do you have any objection to my changing these cases before I checkin the change? Thanks Kathey
        Hide
        Ugo Matrangolo added a comment -

        oops.

        Sorry, I erroneously resolved the issue.

        Show
        Ugo Matrangolo added a comment - oops. Sorry, I erroneously resolved the issue.
        Ugo Matrangolo made changes -
        Status Resolved [ 5 ] Reopened [ 4 ]
        Resolution Fixed [ 1 ]
        Hide
        Ugo Matrangolo added a comment -

        Absolutely no objections.

        The old code ignored the exception with an empty catch() block. I filled it but the simplest solution is to let the code try the exception as you noticed.

        Thank you for your attention,
        Ugo.

        Show
        Ugo Matrangolo added a comment - Absolutely no objections. The old code ignored the exception with an empty catch() block. I filled it but the simplest solution is to let the code try the exception as you noticed. Thank you for your attention, Ugo.
        Hide
        Kathey Marsden added a comment -

        committed patch DERBY-2539_diff_210407.txt

        Date: Mon Apr 23 12:37:23 2007
        New Revision: 531571

        URL: http://svn.apache.org/viewvc?view=rev&rev=531571
        Log:

        Show
        Kathey Marsden added a comment - committed patch DERBY-2539 _diff_210407.txt Date: Mon Apr 23 12:37:23 2007 New Revision: 531571 URL: http://svn.apache.org/viewvc?view=rev&rev=531571 Log:
        Ugo Matrangolo made changes -
        Status Reopened [ 4 ] Closed [ 6 ]
        Resolution Fixed [ 1 ]
        Dag H. Wanvik made changes -
        Issue Type Test [ 6 ] Improvement [ 4 ]
        Kathey Marsden made changes -
        Fix Version/s 10.3.1.4 [ 12312590 ]
        Gavin made changes -
        Workflow jira [ 12401572 ] Default workflow, editable Closed status [ 12800658 ]

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development