db-derby-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Kathey Marsden (JIRA)" <j...@apache.org>
Subject [jira] Commented: (DERBY-3842) Convert "org.apache.derbyTesting.functionTests.tests.store.holdCursorExternalSortJDBC30.sql" to junit.
Date Thu, 26 Mar 2009 23:38:50 GMT

    [ https://issues.apache.org/jira/browse/DERBY-3842?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12689737#action_12689737
] 

Kathey Marsden commented on DERBY-3842:
---------------------------------------

Thank you Tiago for picking up this issue. I know it can be tricky picking up a patch that
someone else started.  Here are my comments:

There are methods in BaseJDBCTestCase that can be used instead of calling getConnection().xxx

Instead of getConnection().setAutoCommit(false), you can just call setAutoComit(false0

Instead of getConnection().createStatement() and getConnection.prepareStatement()  you can
just call createStatement() or prepareStatment and the statements  will get closed automatically
so you can take out the closing of the statements. We may not have helper methods for the
cases where you need to specify extra arguments.  In those cases, what you have is fine.


And also just commit()  instead of getConnection.commit();


Let's make a separate table for the test with the added rows.

There are properties in the derby properties for the old test that are not getting set in
the new one:

derby.storage.sortBufferMax=5
derby.debug.true=testSort

There is a SystemPropertiesTestSetup class that you can use for setting the properties.


testOrder_Hold()

Comment in test says: "Cutover to external sort has been set to 4 rows by the test property
file", but should reflect that we are actually now setting it by Sytem property and that it
is actually 5. Similar comments are in the other fixtures. 

I see 10 elements in boolean[] doCommitAfter = {true, false, false, false, true, false, false,
false, true, true};

If I am reading the master, there seems to be some discrepancy between the master and the
number and sequence of next calls and commits and the  doCommitAfter array /for loop in the
new test.  Could you please recount carefully and fix this up.  I don't fully understand the
store code paths being tested but I think that they are farily specific with this order.


testOrder_NoHold()
again the for loop seems to be missing a row and we don't do/check the final rs.next() which
is false.

testOrderWthMultipleLevel()
On this one I didn't carefully double check the commit math. Again, please take a close look
before you submit the next patch.



> Convert "org.apache.derbyTesting.functionTests.tests.store.holdCursorExternalSortJDBC30.sql"
to junit.
> ------------------------------------------------------------------------------------------------------
>
>                 Key: DERBY-3842
>                 URL: https://issues.apache.org/jira/browse/DERBY-3842
>             Project: Derby
>          Issue Type: Test
>          Components: Test
>            Reporter: Junjie Peng
>            Assignee: Tiago R. Espinha
>         Attachments: derby-3842-1.patch, derby-3842-1.stat, derby-3842-tiago.patch
>
>


-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


Mime
View raw message