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 cursor.java to junit

      1. DERBY_2299.txt
        7 kB
        Kathey Marsden
      2. DERBY_2299.stat
        0.5 kB
        Kathey Marsden
      3. DERBY_2299_2.txt
        22 kB
        Kathey Marsden

        Activity

        Kathey Marsden created issue -
        Kathey Marsden made changes -
        Field Original Value New Value
        Assignee Kathey Marsden [ kmarsden ]
        Hide
        Kathey Marsden added a comment -

        First pass at converting cursor.java to junit

        Show
        Kathey Marsden added a comment - First pass at converting cursor.java to junit
        Kathey Marsden made changes -
        Attachment DERBY_2299.txt [ 12350950 ]
        Kathey Marsden made changes -
        Attachment DERBY_2299.stat [ 12350951 ]
        Hide
        Kathey Marsden added a comment -

        I would appreciate if someone with some junit knowledge would review my conversion of cursor.java to a junit test CursorTest.java. Please let me know of any improvements needed. This is my first attempt so I would appreciate a thorough review and any input,

        Show
        Kathey Marsden added a comment - I would appreciate if someone with some junit knowledge would review my conversion of cursor.java to a junit test CursorTest.java. Please let me know of any improvements needed. This is my first attempt so I would appreciate a thorough review and any input,
        Hide
        Manjula Kutty added a comment -

        I made a very quick review. And everything works fine.

        I'm also not an expert in the junit world. But here is what I think to make this test look better

        most of the try catch block can be replaced by assertStatementError methods from the BaseJDBCTestCase.java. Please see Dan's comments for DERBY-2283

        Show
        Manjula Kutty added a comment - I made a very quick review. And everything works fine. I'm also not an expert in the junit world. But here is what I think to make this test look better most of the try catch block can be replaced by assertStatementError methods from the BaseJDBCTestCase.java. Please see Dan's comments for DERBY-2283
        Hide
        Rajesh Kartha added a comment -

        I applied the patch and was able to run the test succesfully in Embedded.

        Comparing the orginal cursor.java the new test I did not find the following in the new one:
        testCursorName("select * from t for update of i", "myselect");

        Also, the test seem to run only in embedded mode, are there plans to make it run under Network server?.
        I tried running it under both using

        return TestConfiguration.defaultSuite(CursorTest.class);

        in the suite() method.

        With the Network server I noticed the SQLStates returned are different for line 87:

        assertEquals("Wrong SQLState thrown",se.getSQLState(),"24000");

        I am getting XJ121 (Invalid operation at current cursor position) instead of the
        expected 24000 (Invalid cursor state - no current row) so the tests failed for me with Network Server.

        Show
        Rajesh Kartha added a comment - I applied the patch and was able to run the test succesfully in Embedded. Comparing the orginal cursor.java the new test I did not find the following in the new one: testCursorName("select * from t for update of i", "myselect"); Also, the test seem to run only in embedded mode, are there plans to make it run under Network server?. I tried running it under both using return TestConfiguration.defaultSuite(CursorTest.class); in the suite() method. With the Network server I noticed the SQLStates returned are different for line 87: assertEquals("Wrong SQLState thrown",se.getSQLState(),"24000"); I am getting XJ121 (Invalid operation at current cursor position) instead of the expected 24000 (Invalid cursor state - no current row) so the tests failed for me with Network Server.
        Hide
        Kathey Marsden added a comment -

        Changes from first patch.
        Manjula recommended I get rid of the try/catch blocks and use methods from BaseJDBCTestCase. I didn't see what I needed so added assertNextError assertGetIntError to BaseJDBCTestCase and removed the try/catch blocks from CursorTest

        Show
        Kathey Marsden added a comment - Changes from first patch. Manjula recommended I get rid of the try/catch blocks and use methods from BaseJDBCTestCase. I didn't see what I needed so added assertNextError assertGetIntError to BaseJDBCTestCase and removed the try/catch blocks from CursorTest
        Kathey Marsden made changes -
        Attachment DERBY_2299_2.txt [ 12350973 ]
        Hide
        Manjula Kutty added a comment -

        I think the test should run in both modes. There is a difference in the SQL state retuned in the n/w server mode. But the cursor behaviour is same in both. Please see the following discussion.

        http://mail-archives.apache.org/mod_mbox/db-derby-dev/200702.mbox/%3cwjo8r6sz8zku.fsf@sun.com%3e

        Show
        Manjula Kutty added a comment - I think the test should run in both modes. There is a difference in the SQL state retuned in the n/w server mode. But the cursor behaviour is same in both. Please see the following discussion. http://mail-archives.apache.org/mod_mbox/db-derby-dev/200702.mbox/%3cwjo8r6sz8zku.fsf@sun.com%3e
        Hide
        Kathey Marsden added a comment -

        MK>I think the test should run in both modes. There is a difference in the SQL state retuned in the n/w server mode.

        Thanks Manjula
        I wonder whether we should test the different states in the test or change the client SQLState to match embedded.
        If the answer is to change the test to check for different SQLStates for client, how do I branch for a different test for client with a junit test?

        Show
        Kathey Marsden added a comment - MK>I think the test should run in both modes. There is a difference in the SQL state retuned in the n/w server mode. Thanks Manjula I wonder whether we should test the different states in the test or change the client SQLState to match embedded. If the answer is to change the test to check for different SQLStates for client, how do I branch for a different test for client with a junit test?
        Hide
        Manjula Kutty added a comment -

        you can test the different SQL states with usingEmbedded/usingDerbyNet/usingDerbyNetClient methods from the BaseJdBCTestCase.

        example:

        if (usingEmbedded())
        assertStatementError("24000", delete);
        else
        assertStatementError("XCL07", delete);

        where delete is the prepared statement

        Show
        Manjula Kutty added a comment - you can test the different SQL states with usingEmbedded/usingDerbyNet/usingDerbyNetClient methods from the BaseJdBCTestCase. example: if (usingEmbedded()) assertStatementError("24000", delete); else assertStatementError("XCL07", delete); where delete is the prepared statement
        Hide
        Daniel John Debrunner added a comment -

        The use of assertEquals in the converted test has the arguments the wrong way around.

        assertEquals(1956, cursor.getInt(1));
        assertEquals("hello world", cursor.getString(2).trim());

        http://www.junit.org/junit/javadoc/3.8.1/junit/framework/Assert.html

        static void assertEquals(int expected, int actual)

        While it doesn't make a difference when the test passes, it does when it fails. The output provided by the method will give the wrong information to the person trying to resolve the failure, since the expected and actual will be switched in the message. This has the potential to waste tiem and confuse people.

        Show
        Daniel John Debrunner added a comment - The use of assertEquals in the converted test has the arguments the wrong way around. assertEquals(1956, cursor.getInt(1)); assertEquals("hello world", cursor.getString(2).trim()); http://www.junit.org/junit/javadoc/3.8.1/junit/framework/Assert.html static void assertEquals(int expected, int actual) While it doesn't make a difference when the test passes, it does when it fails. The output provided by the method will give the wrong information to the person trying to resolve the failure, since the expected and actual will be switched in the message. This has the potential to waste tiem and confuse people.
        Hide
        Kathey Marsden added a comment -

        Thanks Dan for looking at this. I do see one that I think is the wrong way around.
        assertEquals(cursor.getCursorName(),"ForCoverageSake");

        but the ones you quoted seem right to me with the expected value first.

        assertEquals(1956, cursor.getInt(1));
        assertEquals("hello world", cursor.getString(2).trim());

        I tested changing 1956 to a different value and got an error message "expected 19567 but was 1956"
        I am sure I must be missing something quite obvious here, but could you clarify why these are wrong?

        Show
        Kathey Marsden added a comment - Thanks Dan for looking at this. I do see one that I think is the wrong way around. assertEquals(cursor.getCursorName(),"ForCoverageSake"); but the ones you quoted seem right to me with the expected value first. assertEquals(1956, cursor.getInt(1)); assertEquals("hello world", cursor.getString(2).trim()); I tested changing 1956 to a different value and got an error message "expected 19567 but was 1956" I am sure I must be missing something quite obvious here, but could you clarify why these are wrong?
        Hide
        Daniel John Debrunner added a comment -

        Sorry my fault.
        The first version of the patch had a number of assertEquals() wrong and I made the mistake of thinking it was across the whole class.
        Then I went and quoted the wrong ones, I even know I double checked it, but still messed up.

        Show
        Daniel John Debrunner added a comment - Sorry my fault. The first version of the patch had a number of assertEquals() wrong and I made the mistake of thinking it was across the whole class. Then I went and quoted the wrong ones, I even know I double checked it, but still messed up.
        Kathey Marsden made changes -
        Resolution Fixed [ 1 ]
        Status Open [ 1 ] Resolved [ 5 ]
        Kathey Marsden made changes -
        Status Resolved [ 5 ] Closed [ 6 ]
        Gavin made changes -
        Workflow jira [ 12395941 ] Default workflow, editable Closed status [ 12798281 ]
        Transition Time In Source Status Execution Times Last Executer Last Execution Date
        Open Open Resolved Resolved
        14d 21h 24m 1 Kathey Marsden 21/Feb/07 21:48
        Resolved Resolved Closed Closed
        5s 1 Kathey Marsden 21/Feb/07 21:48

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development