[ 
https://issues.apache.org/jira/browse/CASSANDRA-15718?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17094135#comment-17094135
 ] 

Dinesh Joshi commented on CASSANDRA-15718:
------------------------------------------

FYI: CI was green 
[here|https://circleci.com/workflow-run/10c17f89-a4f3-4b37-9bf4-7abd6834579f].

> Improve BatchMetricsTest 
> -------------------------
>
>                 Key: CASSANDRA-15718
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-15718
>             Project: Cassandra
>          Issue Type: Improvement
>          Components: Test/unit
>            Reporter: Stephen Mallette
>            Assignee: Stephen Mallette
>            Priority: Normal
>             Fix For: 4.0-alpha
>
>         Attachments: CASSANDRA-15582-test-cleanup.patch
>
>
> As noted in CASSANDRA-15582 {{BatchMetricsTest}} should test 
> {{BatchStatement.Type.COUNTER}} to cover all the {{BatchMetrics}}.  Some 
> changes were introduced to make this improvement at:
> https://github.com/apache/cassandra/compare/trunk...spmallette:CASSANDRA-15582-trunk-batchmetrics
> and the following suggestions were made in review (in addition to the 
> suggestion that a separate JIRA be created for this change) by [~dcapwell]:
> {quote}
> * I like the usage of BatchStatement.Type for the tests
> * honestly feel quick theories is better than random, but glad you added the 
> seed to all asserts =). Would still be better as a quick theories test since 
> you basically wrote a property anyways!
> * 
> https://github.com/apache/cassandra/compare/trunk...spmallette:CASSANDRA-15582-trunk-batchmetrics#diff-8948cec1f9d33f10b15c38de80141548R131
>  feel you should rename to expectedPartitionsPerLoggedBatch 
> {Count,Logged,Unlogged}
> * . pre is what the value is, post is what the value is expected to be 
> (rather than what it is).
> * 
> * 
> https://github.com/apache/cassandra/compare/trunk...spmallette:CASSANDRA-15582-trunk-batchmetrics#diff-8948cec1f9d33f10b15c38de80141548R150
>  this doesn't look correct. the batch has distinctPartitions mutations, so 
> shouldn't max reflect that? I ran the current test in a debugger and see that 
> that is the case (aka current test is wrong).
> most of the comments are nit picks, but the last one looks like a test bug to 
> me
> {quote}



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org

Reply via email to