Claudenw commented on code in PR #4087:
URL: https://github.com/apache/cassandra/pull/4087#discussion_r2050220654
##########
test/unit/org/apache/cassandra/db/compaction/unified/ControllerTest.java:
##########
@@ -121,6 +121,18 @@ public void testValidateOptionsIntegers()
testValidateOptions(true);
}
+ @Test
+ public void testValidateOptionsInvalidTargetSSTableSize()
+ {
+ try {
+ Map<String, String> options = new HashMap<>();
+ options.putIfAbsent(Controller.TARGET_SSTABLE_SIZE_OPTION,
"12E899");
+ Controller.validateOptions(options);
+ fail("Passing invalid number should have caused exception");
+ } catch(ConfigurationException e) {
+ }
+ }
+
Review Comment:
Please rename the tests to indicate what they are testing. For example
`testValidateOptionsTargetSSTableSize1()` could be renamed
`testCassandra20398Values()`
and
`testValidateOptionsTargetSSTableSize2()` could be renamed
`testTargetSstableSizeOptionGTLongMax()`
In fact you could combine 1 and 2 into a single parameter driven method that
uses "12E899 B" and "9223372036854775907 B" as values and then in comments
explain that 12E899B comes from CASSANDRA-20398 and 9223372036854775907 is
Long.MAX_VALUE + 100
Question: Why does `Long.MAX_VALUE + 100` fail?
A little experimentation shows that
9223372036854776000 is what Long.MAX_VALUE converts to as a double.
all values from [Long.MAX_VALUE, Long.MAX_VALUE + 1024] convert to that
value so all of them should be considered valid.
There needs to be a test for exactly Long.MAX_VALUE to show that it passes.
You could write a single paramaterized test that takes the configuration
file formatted value (e.g. "9223372036854775907 B") and a boolean flag to
indicate that it should or should not pass the test. Then we can add other
values as it becomes clear that they need to be tested.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
