cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Joshua McKenzie (JIRA)" <>
Subject [jira] [Commented] (CASSANDRA-9160) Migrate CQL dtests to unit tests
Date Mon, 08 Jun 2015 21:02:01 GMT


Joshua McKenzie commented on CASSANDRA-9160:

1st half of the review done - I'm using the spreadsheet to track which I've reviewed. (Note:
I'd normally just tidy up a lot of the following and push a branch but will just note here
as you're building patches for multiple branches.) 

h6. General feedback:
- Do we still need the Thread.sleep calls that were ported directly from If
so, do they need to remain at their original values (500ms, 1500ms, etc)?
- Make fields in CQL3CasRequest package private rather than public
- Rename CQLTester.rebuild to CQLTester.rebuildSecondaryIndexes
- Add comment back into LimitTest.testColumnRange:
# Check that we do limit the output to 1 *and* that we respect query
# order of keys (even though 48 is after 2)
- SecondaryIndexTest.testSelectCountOnIndexedColumn: it's unclear why we have the commented
out index creation. Rather than carry it over from the dtest, maybe we should delete it.
- Header on SelectTest.testSelectBounds incorrectly references source dtest TestCQL.select_key_in_test
rather than TestCQL.exclusive_slice_test which is the actual source
- StaticColumnsTest.testStaticColumnsWithDistinct has four lines from the dtest copied over
and commented out that can be removed (default_fetch_size, a print statement, fetch_size lines)

- nit: Don't need //cursor.default_fetch_size = 10000 in ManyRowsTest.testLargeCount as comment
- nit: Remove //if v >= "2.1.1" or v < "2.1" and v >= "2.0.11": from testConditionalDelete
- nit: Fix spelling of "generally" in header comment for OrderedTest.testOrderByMultikey

h6. formatting / spelling nits:
* Whitespace problems in SecondaryIndexTest.testIndexOnCollections
* CollectionsTest.testSetWithUnsetValues: didn't fix the indentation on all assertRows(...)
calls, just 2 of 4. May as well do them all :)
* CollectionsTest.testSelectMapKeySingleRow: inconsistent spacing on cast

h6. ultra-neurotic-nit(s):
* // "Should not apply" should read "Shouldn't apply" in testCondionalUpdate comments as we
already have 7 instances of the contraction form and that one just sticks out like a very
painful sore thumb... (also, can remove unnecessary space in the top "Shouldn 't apply" comment
while you're at it. I blame [~slebresne] for these since you just copied his code straight
from the dtests :)). Saw a couple other "Shouldn 't" instances throughout the code-base so
a simple search-and-replace could be in order.

Should be able to get to the 2nd half tomorrow and hopefully dtest and 2.1 backports.

> Migrate CQL dtests to unit tests
> --------------------------------
>                 Key: CASSANDRA-9160
>                 URL:
>             Project: Cassandra
>          Issue Type: Test
>            Reporter: Sylvain Lebresne
>            Assignee: Stefania
> We have CQL tests in 2 places: dtests and unit tests. The unit tests are actually somewhat
better in the sense that they have the ability to test both prepared and unprepared statements
at the flip of a switch. It's also better to have all those tests in the same place so we
can improve the test framework in only one place (CASSANDRA-7959, CASSANDRA-9159, etc...).
So we should move the CQL dtests to the unit tests (which will be a good occasion to organize
them better).

This message was sent by Atlassian JIRA

View raw message