[
https://issues.apache.org/jira/browse/CASSANDRA-9160?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14577844#comment-14577844
]
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
cql_tests.py? 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:
{noformat}
# Check that we do limit the output to 1 *and* that we respect query
# order of keys (even though 48 is after 2)
{noformat}
- 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 re-iterates
- 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: https://issues.apache.org/jira/browse/CASSANDRA-9160
> 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
(v6.3.4#6332)
