Derby
  1. Derby
  2. DERBY-2411

convert scrollCursors2.java to junit

    Details

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

      Description

      convert scrollCursors2.java to junit

      1. derby-2411.diff
        85 kB
        Kathey Marsden

        Activity

        Hide
        Kathey Marsden added a comment -

        Attached is a patch to make a new junit test ScrollCursors2Test.java to replace scrollCursors2.java. The test had an assertion style so this is just a line by line translation to junit.

        Show
        Kathey Marsden added a comment - Attached is a patch to make a new junit test ScrollCursors2Test.java to replace scrollCursors2.java. The test had an assertion style so this is just a line by line translation to junit.
        Hide
        Knut Anders Hatlen added a comment -

        I have read through the patch. It generally looks good. Please see my comments below.

        What about assertEquals() in these two cases?
        + rs.setFetchSize(5);
        + if (rs.getFetchSize() != 5)

        { + fail("getFetchSize() expected to return 5"); + }

        +
        + if (rs.getFetchDirection() != ResultSet.FETCH_FORWARD)

        { + fail("getFetchDirection() expected to return FETCH_FORWARD, not " + + rs.getFetchDirection()); + }

        assertNull(conn.getWarnings()) instead of the println loop?
        + // We should have gotten no warnings and a read only forward only cursor
        + warning = conn.getWarnings();
        + while (warning != null)

        { + System.out.println("warning = " + warning); + warning = warning.getNextWarning(); + }

        assert instead of println here?
        + // Verify that result set from statement is
        + // scroll insensitive and read only
        + rs = ps_f_r.executeQuery();
        + if (rs.getType() != ResultSet.TYPE_FORWARD_ONLY)

        { + System.out.println("cursor type = " + rs.getType() + ", not " + + ResultSet.TYPE_FORWARD_ONLY); + }

        + if (rs.getConcurrency() != ResultSet.CONCUR_READ_ONLY)

        { + System.out.println("concurrency = " + rs.getConcurrency() + + ", not " + ResultSet.CONCUR_READ_ONLY); + }

        assertNotNull? Actually, testing that the result set is not null could probably be removed since the subsequent use of it would fail anyway.
        + rs = s_f_r.executeQuery("values 1, 2, 3, 4, 5, 6");
        + if (rs == null)

        { + fail("rs expected to be non-null."); + }

        This could be replaced with a single call to JDBC.assertDrainResults(rs, 5):
        + if (rs == null) { + fail("rs expected to be non-null."); + }

        + // Iterate straight thru RS, expect only 5 rows.
        + for (int index = 1; index < 6; index++) {
        + if (!rs.next())

        { + fail("rs.next() failed, index = " + index); + break; + }

        + }
        + // We should not see another row (only 5, not 6)
        + if (rs.next())

        { + fail("rs.next() failed, should not have seen 6th row."); + passed = false; + }

        + rs.close();

        scrollInsensitivePositive() and scrollInsensitiveNegative() should start with "test".

        The last ResultSets in scrollInsensitiveNegative() and testScrollInsensitiveNegative() are not closed.

        scrollVerifyMaxRowWithFetchSize() could be private to clearly indicate that it's not a top-level test method.

        Class lacks copyright header.

        Show
        Knut Anders Hatlen added a comment - I have read through the patch. It generally looks good. Please see my comments below. What about assertEquals() in these two cases? + rs.setFetchSize(5); + if (rs.getFetchSize() != 5) { + fail("getFetchSize() expected to return 5"); + } + + if (rs.getFetchDirection() != ResultSet.FETCH_FORWARD) { + fail("getFetchDirection() expected to return FETCH_FORWARD, not " + + rs.getFetchDirection()); + } assertNull(conn.getWarnings()) instead of the println loop? + // We should have gotten no warnings and a read only forward only cursor + warning = conn.getWarnings(); + while (warning != null) { + System.out.println("warning = " + warning); + warning = warning.getNextWarning(); + } assert instead of println here? + // Verify that result set from statement is + // scroll insensitive and read only + rs = ps_f_r.executeQuery(); + if (rs.getType() != ResultSet.TYPE_FORWARD_ONLY) { + System.out.println("cursor type = " + rs.getType() + ", not " + + ResultSet.TYPE_FORWARD_ONLY); + } + if (rs.getConcurrency() != ResultSet.CONCUR_READ_ONLY) { + System.out.println("concurrency = " + rs.getConcurrency() + + ", not " + ResultSet.CONCUR_READ_ONLY); + } assertNotNull? Actually, testing that the result set is not null could probably be removed since the subsequent use of it would fail anyway. + rs = s_f_r.executeQuery("values 1, 2, 3, 4, 5, 6"); + if (rs == null) { + fail("rs expected to be non-null."); + } This could be replaced with a single call to JDBC.assertDrainResults(rs, 5): + if (rs == null) { + fail("rs expected to be non-null."); + } + // Iterate straight thru RS, expect only 5 rows. + for (int index = 1; index < 6; index++) { + if (!rs.next()) { + fail("rs.next() failed, index = " + index); + break; + } + } + // We should not see another row (only 5, not 6) + if (rs.next()) { + fail("rs.next() failed, should not have seen 6th row."); + passed = false; + } + rs.close(); scrollInsensitivePositive() and scrollInsensitiveNegative() should start with "test". The last ResultSets in scrollInsensitiveNegative() and testScrollInsensitiveNegative() are not closed. scrollVerifyMaxRowWithFetchSize() could be private to clearly indicate that it's not a top-level test method. Class lacks copyright header.
        Hide
        Kathey Marsden added a comment -

        Commited test and made changes from Knut's review

        Show
        Kathey Marsden added a comment - Commited test and made changes from Knut's review
        Hide
        Knut Anders Hatlen added a comment -

        Thank you for addressing my comments, Kathey. I noticed that ScrollCursors2Test.java didn't have the svn:eol-style property. Please check that you have configured subversion to set it to native automatically for java files. I fixed the eol-style in revision 516322. I also removed the javadoc for the deleted method in revision 516323.

        Show
        Knut Anders Hatlen added a comment - Thank you for addressing my comments, Kathey. I noticed that ScrollCursors2Test.java didn't have the svn:eol-style property. Please check that you have configured subversion to set it to native automatically for java files. I fixed the eol-style in revision 516322. I also removed the javadoc for the deleted method in revision 516323.

          People

          • Assignee:
            Kathey Marsden
            Reporter:
            Kathey Marsden
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development