adelapena commented on code in PR #2369:
URL: https://github.com/apache/cassandra/pull/2369#discussion_r1210553787
##########
test/distributed/org/apache/cassandra/distributed/test/guardrails/GuardrailPartitionSizeTest.java:
##########
@@ -75,12 +75,11 @@ protected Cluster getCluster()
@Test
public void testPartitionSize()
{
+ // test yaml-loaded config
testPartitionSize(WARN_THRESHOLD, FAIL_THRESHOLD);
- }
+ schemaChange("DROP TABLE %s");
Review Comment:
The tests extending `o.a.c.distributed.test.guardrails.GuardrailTester` use
a single cluster for the entire suite. The regular test case uses the yaml
configuration set at startup, the one that is set at `setupCluster`. In
contrast, the test for dynamic update changes that config on the running
instance by simulating the JMX methods.
That works if the test for static config runs before the test for dynamic
update, but not the other way around. If the test for dynamic config runs
first, the test for static config will find the dynamically updated config,
breaking the test. We could simply restart the node before each test, but I
think the test will save resources if we simply combine both tests. I think the
tests are brief enough to remain readable.
So `GuardrailPartitionSizeTest` only worked incidentally because the tests
were run in the fortunate order. The new `GuardrailPartitionTombstonesTest`
however was not so fortunate, exposing the bug. So I have changed both to make
them independent of whatever order the test runner decides for us.
--
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]
