cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Sylvain Lebresne (JIRA)" <>
Subject [jira] [Commented] (CASSANDRA-10059) Test Coverage and related bug-fixes for AbstractBTreePartition and hierarchy
Date Wed, 02 Dec 2015 13:29:10 GMT


Sylvain Lebresne commented on CASSANDRA-10059:

Slightly late to the review, sorry. Test is convincing in general and fixes looks good but
a few small remarks on the test:
* In the {{test(Supplier ...)}} method, the lack of both braces and indentation makes this
hard to read.
* In the main {{testIter()}} method, there is a sub-part about {{// filtered}}, but while
this filter on the expected input, it doesn't filter the actual iterator. The test still pass
because the row generated by the test never have (cell) tombstones, so there is nothing filtered,
but it's still kind of wrong. We can randomly generate some tombstone when generating rows
and use {{UnfilteredRowIterators.filter()}} on the iterator from the partition, but that's
more a test of {{UnfilteredRowIterators}} that of the partition implementation itself, so
I'm fine if you prefer just removing that case here.
* Nitpicking here: one assumption of a partition is that it should not contain data that it
deletes itself, which in practice mean that any cell covered by a marker should have a timestamp
greater than marker deletion time. The test mostly respect that thanks to the use of the current
time for row timestamp but a random number <= {{KEY_RANGE}} for range markers, but that
feels a bit like a luck as is.  Maybe using {{KEY_RANGE + 1}} as timestamp for the row cells
with a short comment would be more explicit.

Could you also make sure the branch is rebased and CI are ran on it.

> Test Coverage and related bug-fixes for AbstractBTreePartition and hierarchy
> ----------------------------------------------------------------------------
>                 Key: CASSANDRA-10059
>                 URL:
>             Project: Cassandra
>          Issue Type: Test
>          Components: Local Write-Read Paths
>            Reporter: Benedict
>            Assignee: Branimir Lambov
>             Fix For: 3.0.1, 3.1
> Follow up to CASSANDRA-9932. The test coverage for AbstractBTreePartition and its hierarchy
is entirely indirect. That is not to say it is not covered, but we may have some unexplored
behaviour. Coverage for BTree is also missing around a couple of edges, and the gaps should
be filled in.

This message was sent by Atlassian JIRA

View raw message