Stephen Mallette created CASSANDRA-15718:
--------------------------------------------
Summary: 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
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
