db-derby-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "David W. Van Couvering" <David.Vancouver...@Sun.COM>
Subject Re: [jira] Updated: (DERBY-934) create a set of JUnit tests for Scrollable Updatable Resultsets
Date Fri, 10 Feb 2006 04:52:33 GMT
Added these as a comment, but sending out as separate email until JIRA 
unclags itself

===

My second ( and last) batch of comments. I would be willing to commit 
this patch if it weren't for the generically swallowed SQLExceptions on 
negative tests, the rest are just nits or questions. REALLY GOOD TESTS, 
thanks for all your hard work on this!

- You are testing concurrency issues with READ_COMMITTED and 
REPEATABLE_READ isolation levels (and why you choose one over the other 
for various tests is not clear to me; any comments in the code around 
this I think would be most helpful.) Do we need to test at other 
isolation levels?

- In many of the concurrency tests, you say "test what happens". It 
would be great if you described the overall strategy and expected 
behavior in the comments. For example, what is the difference between 
tests 11 and 12? The javadoc comments are equivalent, and there are 
subtle differences in the two tests, and it's not clear what's 
motivating those differences.

- testConcurrency13 - your comments next to the commented out lines are 
a bit mysterious. More explanation is needed. And again, please look for 
the particular SQL State that should cause a failure, rather than 
accepting any SQLException as a sign of success.

- testCompressDuringScan - why do you print a stack trace on an expected 
exception? Isn't that going to generate spurious output, sort of a 
"no-no" for JUnit, style, assertion-based testing? Is this because you 
want to be sure the "right" exception occurs? If so, couldn't you do 
this by checking SQL State?

- In many of the concurrency tests you have commented out the last 
assertion. Can you explain why? It seems you need to either add a 
comment explaining whey and when they should be uncommented, or remove 
the lines altogether.

- Half way through SURTest.java you stop providing method comments for 
your tests. Those were quite valuable, any reason you stopped?

- testScrollInsensitiveConcurUpdatableWithForUpdate1() - please comment 
why you commented out rs.last(), rs.first(), rs.previous(), or remove 
the lines.

- testScrollInsensistiveConurUpdatable3 - You call System.out.print 
(".") for each row. More noise into the output. Any reason for this, or 
is this a debug statement that you forgot to remove? Also, more code 
commented out with no explanation...

- SURTest.suite() also has these strange statement blocks with locally 
scoped variables. OK, I guess, but not the kind of Java I'm used to 
seeing, and I am not sure I see the value in it...

Andreas Korneliussen (JIRA) wrote:
>      [ http://issues.apache.org/jira/browse/DERBY-934?page=all ]
> 
> Andreas Korneliussen updated DERBY-934:
> ---------------------------------------
> 
>     Attachment:     (was: DERBY-934.diff)
> 
> 
>>create a set of JUnit tests for Scrollable Updatable Resultsets
>>---------------------------------------------------------------
>>
>>         Key: DERBY-934
>>         URL: http://issues.apache.org/jira/browse/DERBY-934
>>     Project: Derby
>>        Type: Sub-task
>>  Components: Test
>>    Reporter: Andreas Korneliussen
>>    Assignee: Andreas Korneliussen
>> Attachments: DERBY-934.stat
>>
>>Add a set of JUnit tests which tests the implementation for Scrollable Updatable ResultSets.
>>The following is a description of how the tests will be implemented:
>>Data model in test:
>>We use one table containing three int fields and one varchar(5000)
>>field. 
>>Then we run the tests on a number of variants of this model:
>>1. None of the fields are indexed (no primary key, no secondary key)
>>2. One of the fields is indexed as primary key
>>3. One of the fields is indexed as primary key, another field is
>>   indexed as secondary key
>>4. One field is indexed as secondary key
>>(primary key is unique, secondary key is not unique)
>>By having these variations in the data model, we cover a number of
>>variations where the ScrollInsensitiveResultSet implementation uses
>>different classes of source ResultSets, and the CurrentOfResultSet
>>uses different classes of target and source ResultSet.
>>The table can be created with the following fields:
>>(id int, a int, b int, c varchar(5000))
>>-
>>Queries for testing SUR:
>>Select conditions:
>>* Full table scan
>>SQL: SELECT * FROM T1
>>* Full table scan with criteria on non-indexed field
>>SQL: .. WHERE c like ?
>>SQL: .. WHERE b > ?
>>* Full table scan with criteria on indexed field
>>SQL: .. WHERE id>a
>>* SELECT on primary key conditionals:
>>- Upper and lower bond criteria:
>>SQL: .. WHERE ID>? and ID<?
>>SQL: .. WHERE ID=? -- (Single tuple)
>>* Nested queries:
>>SQL: .. WHERE ID in (1,2,3,4) 
>>SQL: .. WHERE a  in (1,2,3,4) 
>>(Other nested queries containing a table seems to not permit updates)
>>* SELECT on secondary key conditionals:
>>SQL: .. WHERE a>? and a<?
>>SQL: .. WHERE a=?
>>* Projections:
>>SQL: SELECT id,a,b,c
>>SQL: SELECT id,c,b,a
>>SQL: SELECT id,c
>>SQL: SELECT id,a
>>SQL: SELECT a,b,c
>>SQL: SELECT a,b
>>SQL: SELECT a,c
>>The test should generate queries with all combinations of the
>>projection and select conditions, and then run a number of tests:
>>- test navigiation
>>- test updates + navigation
>>- test deletes + navigation
>>- check rowUpdated() and rowDeleted() fields
>>Scrollability: All scroll-insensitive cursors should be checked for
>>scrollability. Scrolling is tested by invoking: next(), previous(),
>>beforeFirst(), afterLast(), absolute(), relative(),  isBeforeFirst(),
>>isAfterLast(), isFirst(), isLast(),
>>Updating a scrollable resultset: a ResultSets current row can be
>>updated either by using updateXXX() + updateRow(), or by using
>>a positioned update query.  All tests which updates row, will come in
>>two variants covering both these cases.
>>-
>>Deleting rows in scrollable resultset also has two variants: one using
>>a positioned update query, and one using deleteRow().
>>-
>>Special testcases:
>>Test that you get a warning when specifying a query which is not
>>updatable and concurrency mode CONCUR_UPDATABLE.
>>Case 1: Query containing order by
>>Case 2: Query containing a join
>>Exceptions:
>>Test that you get an exception when specifying update clause "FOR UPDATE"
>>along with a query which is not updatable.
>>Cases:
>> * Query containing order by
>> * Query containing a join
>>Test that you get an exception when attempting to update a ResultSet 
>>which has been downgraded to a read only ResultSet due to the query 
>>Cases:
>> * Query contained a join
>> * Query contained a read only update clause
>> * Query contained a order by
>>Test that you get an exception when attempting to update a ResultSet 
>>which has concurrency mode CONCUR_READ_ONLY
>> 
>>Concurrency tests:
>>(ConcurrencyTest)
>>Cases: 
>>* Test that update locks are downgraded to shared locks after
>>  repositioning. (fails with derby)
>>* Test that we can aquire a update lock even if the row is locked with
>>  a shared lock.
>>* Test that we can aquire a shared lock even if the row is locked with
>>  an update lock.
>>* Test that we do not get a concurrency problem when opening two
>>  cursors as readonly.
>>* Test what happens if you update a deleted and purged record
>>* Test what happens if you update a deleted and purged record using
>>  positioned update
>>* Test what happens if you update a tuple which is deleted and then
>>  reinserted.
>>* Test what happens if you update a tuple which is deleted and then
>>  reinserted with the exact same values
>>* Test what happens if you update a tuple which has been modified by
>>  another transaction.
>>* Test that you cannot compress the table while the ResultSet is open,
>>  and the transaction is open (for validity of RowLocation)
>>* Test that you cannot purge a row if it is locked
>>* Test that Derby set updatelock on current row when using
>>  read-uncommitted
> 
> 

Mime
View raw message