You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by "David Capwell (Jira)" <ji...@apache.org> on 2020/04/13 16:41:00 UTC
[jira] [Updated] (CASSANDRA-15718) Improve BatchMetricsTest
[ https://issues.apache.org/jira/browse/CASSANDRA-15718?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
David Capwell updated CASSANDRA-15718:
--------------------------------------
Change Category: Quality Assurance
Complexity: Low Hanging Fruit
Status: Open (was: Triage Needed)
> 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
>
> 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-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org