[
https://issues.apache.org/jira/browse/CASSANDRA-9160?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14578642#comment-14578642
]
Stefania commented on CASSANDRA-9160:
-------------------------------------
Thank you for your input and the first half of the review. Here is what I've
done so far, next I'll focus on
clearing up the remaining dtests and rearranging the java tests.
bq. 1st half of the review done
I believe I addressed all your comments, I kept them in a single commit that I
applied to the 2.1 patch and then merged it into 2.2
and trunk. Only one of the nits got forgotten because it was not applicable to
the 2.1 branch, and that is the reason for the
additional commit on 2.2 and on trunk.
Few things to notice:
* Some of the sleep calls were necessary unfortunately and so I could only
reduce the value.
* Since we had several sleep calls used for index building, I created a method
in CQLTester to do this, waitForIndex(), perhaps the name could be better.
* I removed CQLtester build() as it's not needed any longer, it was previously
used by testCanQuerySecondaryIndex().
BTW, the 2.2 and trunk patches are identical, so we could drop the latter and
merge the 2.2. patch into trunk during commit.
So far I had no conflicts between 2.2 and trunk.
bq. is there a reason you can't use execute() instead of executeInternal() and
trust the QueryPager to make the determination on whether the query is local or
not? I believe that should work fine on single-node unit tests.
We cannot call execute() because that goes to StorageProxy and so we'd have to
initialize the ring and stuff. However I hadn't noticed that the pager does
support local queries, thanks for pointing it out. Therefore it was rather easy
to add it to SelectStatement.executeInternal(). I kept this in a dedicated
commit. On 2.1 there is a bit of duplicated code, but on 2.2 I refactored it.
bq. I'd say append the tests to the respective tickets so we don't calcify
their interfaces / usage patterns while they're still in progress and leave the
responsibility on the developers to test their code.
I've attached the converted unit tests to CASSANDRA-7396 and CASSANDRA-7281 and
I've removed them from this patch.
bq. I think the latter - the 1 dtest per CQL statement type nested under a
test_storageproxy dtest (for example) would be clean and keep those concerns
separated. I think network-based execution unnecessarily expands/blends the
scope of CQLTester.
Sounds good - I am currently working on this.
bq. A naive initial look at the file indicates some clear lines of separation:
select tests, create tests, delete tests, dense/compact tests, batch tests,
indexes, and collections are the first ones that pop to me. I'd recommend
organizing the tests in terms of the underlying CQL operations they're testing
and then renaming TableTest to something to indicate the "overflow" nature of
those tests.
Thanks - I'll tackle this next.
> 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)
