CASSSIDECAR-204: Improve integration test stability #188

yifan-c · 2025-02-06T21:46:11Z

Patch by Yifan Cai; Reviewed by TBD for CASSSIDECAR-204

yifan-c · 2025-02-06T22:39:42Z

...ork/src/main/java/org/apache/cassandra/sidecar/testing/SharedClusterIntegrationTestBase.java

+        Vertx vertx = sidecarServerInjector.getInstance(Vertx.class);
+        vertx.eventBus()
+             .localConsumer(SidecarServerEvents.ON_SIDECAR_SCHEMA_INITIALIZED.address(), msg -> sidecarSchemaReadyLatch.countDown());


register the event consumer at the earliest. The original setup misses the event occasionally.

yifan-c · 2025-02-06T22:40:38Z

...ts/src/integrationTest/org/apache/cassandra/sidecar/routes/SchemaHandlerIntegrationTest.java

+import static org.apache.cassandra.testing.utils.AssertionUtils.getBlocking;
+import static org.assertj.core.api.Assertions.assertThat;
+
+class SchemaHandlerIntegrationTest extends SharedClusterSidecarIntegrationTestBase


migrated, to speed up and to avoid flakiness.

what a difference! This will help with CI, we should find more opportunities to migrate

Before:

$ ./gradlew --stacktrace :server:integrationTestLightWeight --tests org.apache.cassandra.sidecar.routes.SchemaHandlerIntegrationTest > Task :server:integrationTestLightWeight SchemaHandlerIntegrationTest > schemaHandlerWithReservedKeywordKeyspace(VertxTestContext) > schemaHandlerWithReservedKeywordKeyspace(VertxTestContext): 5.1 STARTED SchemaHandlerIntegrationTest > schemaHandlerWithReservedKeywordKeyspace(VertxTestContext) > schemaHandlerWithReservedKeywordKeyspace(VertxTestContext): 5.1 PASSED SchemaHandlerIntegrationTest > schemaHandlerNoKeyspace(VertxTestContext) > schemaHandlerNoKeyspace(VertxTestContext): 5.1 STARTED SchemaHandlerIntegrationTest > schemaHandlerNoKeyspace(VertxTestContext) > schemaHandlerNoKeyspace(VertxTestContext): 5.1 PASSED SchemaHandlerIntegrationTest > schemaHandlerWithCaseSensitiveKeyspace(VertxTestContext) > schemaHandlerWithCaseSensitiveKeyspace(VertxTestContext): 5.1 STARTED SchemaHandlerIntegrationTest > schemaHandlerWithCaseSensitiveKeyspace(VertxTestContext) > schemaHandlerWithCaseSensitiveKeyspace(VertxTestContext): 5.1 PASSED SchemaHandlerIntegrationTest > schemaHandlerKeyspaceDoesNotExist(VertxTestContext) > schemaHandlerKeyspaceDoesNotExist(VertxTestContext): 5.1 STARTED SchemaHandlerIntegrationTest > schemaHandlerKeyspaceDoesNotExist(VertxTestContext) > schemaHandlerKeyspaceDoesNotExist(VertxTestContext): 5.1 PASSED SchemaHandlerIntegrationTest > schemaHandlerWithKeyspace(VertxTestContext) > schemaHandlerWithKeyspace(VertxTestContext): 5.1 STARTED SchemaHandlerIntegrationTest > schemaHandlerWithKeyspace(VertxTestContext) > schemaHandlerWithKeyspace(VertxTestContext): 5.1 PASSED BUILD SUCCESSFUL in 1m 16s 28 actionable tasks: 1 executed, 27 up-to-date

After:

$ ./gradlew --stacktrace :integration-tests:integrationTestLightWeight --tests org.apache.cassandra.sidecar.routes.SchemaHandlerIntegrationTest > Task :integration-tests:integrationTestLightWeight SchemaHandlerIntegrationTest > testSchemaHandlerWithReservedKeywordKeyspace() STARTED SchemaHandlerIntegrationTest > testSchemaHandlerWithReservedKeywordKeyspace() PASSED SchemaHandlerIntegrationTest > testSchemaHandlerWithCaseSensitiveKeyspace() STARTED SchemaHandlerIntegrationTest > testSchemaHandlerWithCaseSensitiveKeyspace() PASSED SchemaHandlerIntegrationTest > testListKeyspaces() STARTED SchemaHandlerIntegrationTest > testListKeyspaces() PASSED SchemaHandlerIntegrationTest > testSchemaHandlerWithKeyspace() STARTED SchemaHandlerIntegrationTest > testSchemaHandlerWithKeyspace() PASSED SchemaHandlerIntegrationTest > testSchemaHandlerKeyspaceDoesNotExist() STARTED SchemaHandlerIntegrationTest > testSchemaHandlerKeyspaceDoesNotExist() PASSED BUILD SUCCESSFUL in 19s 21 actionable tasks: 10 executed, 11 up-to-date

I agree with you.

yifan-c · 2025-02-06T22:41:16Z

server/src/test/integration/org/apache/cassandra/sidecar/common/DelegateIntegrationTest.java

+    @Override
+    protected void beforeServerStart()
+    {
+        vertx.eventBus().localConsumer(ON_ALL_CASSANDRA_CQL_READY.address(), allCassandraCqlReadyMessage::set);


Register the event consumer at the earliest to avoid missing events

yifan-c · 2025-02-06T22:43:27Z

server/src/test/integration/org/apache/cassandra/sidecar/common/DelegateIntegrationTest.java

        else if (upInstanceCount == 3)
        {
-            assertThat(jmxConnectedInstances).containsExactly(1, 2, 3);
+            context.verify(() -> assertThat(jmxConnectedInstances).containsExactly(1, 2, 3));


must wrap in the verify block to correctly fail the test if assertion fails. Otherwise, context/flag are not closed, and test gets stuck until timeout.

yifan-c · 2025-02-06T22:46:10Z

server/src/test/integration/org/apache/cassandra/sidecar/routes/StreamStatsIntegrationTest.java

+        CountDownLatch testStart = new CountDownLatch(1);
+        IUpgradeableInstance node = cluster.get(1);
+        AtomicReference<Throwable> nodetoolError = new AtomicReference<>();
+        startAsync("Repairing node" + node.config().num(),


Changed the stream trigger from decommission to repair. The test also does not need bytebuddy to intercept, hence removing it.
We should avoid bytebuddy at the best, to avoid dependency on Cassandra's implementation details.

yifan-c · 2025-02-06T22:46:39Z

...src/test/integration/org/apache/cassandra/sidecar/routes/tokenrange/BasicMultiDCRf3Test.java

 * Note: Some related test classes are broken down to have a single test case to parallelize test execution and
 * therefore limit the instance size required to run the tests from CircleCI as the in-jvm-dtests tests are memory bound
 */
+@Tag("heavy")


This test is actually quite heavy.

yifan-c · 2025-02-06T22:47:17Z

...r/src/test/integration/org/apache/cassandra/sidecar/testing/CassandraSidecarTestContext.java

+    // array of nodeNums that are 1-based
+    private int[] instancesToManage = null;


it makes the Cassandra instances flexible. Now, one can choose to manage, instance 1 and 3.

yifan-c · 2025-02-06T22:48:32Z

server/src/test/integration/org/apache/cassandra/testing/CassandraTestTemplate.java

+    private Path serverKeystorePath(CertificateBundle ca, Path path, int totalNodes) throws Exception
    {
-        CertificateBundle keystore = new CertificateBuilder()
-                                     .subject("CN=Apache Cassandra, OU=ssl_test, O=Unknown, L=Unknown, ST=Unknown, C=Unknown")
-                                     .addSanDnsName("localhost")
-                                     .addSanIpAddress("127.0.0.1")
-                                     .buildIssuedBy(ca);
+        CertificateBuilder builder = new CertificateBuilder();
+        builder.subject("CN=Apache Cassandra, OU=ssl_test, O=Unknown, L=Unknown, ST=Unknown, C=Unknown")
+               .addSanDnsName("localhost");
+        for (int i = 1; i <= totalNodes; i++)
+        {
+            builder.addSanIpAddress("127.0.0." + i);
+        }
+        CertificateBundle keystore = builder.buildIssuedBy(ca);


Allow contacting other hosts rather than 127.0.0.1 only.

yifan-c · 2025-02-06T22:49:26Z

server/src/test/integration/org/apache/cassandra/sidecar/testing/IntegrationTestModule.java

-    public ClusterLease clusterLease()
-    {
-        return new ClusterLease(ClusterLease.Ownership.CLAIMED);
-    }
-


remove the fake lease. It now runs the actual lease competition in every integration test.

frankgh

+1 Looks good to me. Left a minor comment

frankgh · 2025-02-08T17:48:54Z

...ts/src/integrationTest/org/apache/cassandra/sidecar/routes/SchemaHandlerIntegrationTest.java

+import static org.apache.cassandra.testing.utils.AssertionUtils.getBlocking;
+import static org.assertj.core.api.Assertions.assertThat;
+
+class SchemaHandlerIntegrationTest extends SharedClusterSidecarIntegrationTestBase


what a difference! This will help with CI, we should find more opportunities to migrate

Before:

$ ./gradlew --stacktrace :server:integrationTestLightWeight --tests org.apache.cassandra.sidecar.routes.SchemaHandlerIntegrationTest > Task :server:integrationTestLightWeight SchemaHandlerIntegrationTest > schemaHandlerWithReservedKeywordKeyspace(VertxTestContext) > schemaHandlerWithReservedKeywordKeyspace(VertxTestContext): 5.1 STARTED SchemaHandlerIntegrationTest > schemaHandlerWithReservedKeywordKeyspace(VertxTestContext) > schemaHandlerWithReservedKeywordKeyspace(VertxTestContext): 5.1 PASSED SchemaHandlerIntegrationTest > schemaHandlerNoKeyspace(VertxTestContext) > schemaHandlerNoKeyspace(VertxTestContext): 5.1 STARTED SchemaHandlerIntegrationTest > schemaHandlerNoKeyspace(VertxTestContext) > schemaHandlerNoKeyspace(VertxTestContext): 5.1 PASSED SchemaHandlerIntegrationTest > schemaHandlerWithCaseSensitiveKeyspace(VertxTestContext) > schemaHandlerWithCaseSensitiveKeyspace(VertxTestContext): 5.1 STARTED SchemaHandlerIntegrationTest > schemaHandlerWithCaseSensitiveKeyspace(VertxTestContext) > schemaHandlerWithCaseSensitiveKeyspace(VertxTestContext): 5.1 PASSED SchemaHandlerIntegrationTest > schemaHandlerKeyspaceDoesNotExist(VertxTestContext) > schemaHandlerKeyspaceDoesNotExist(VertxTestContext): 5.1 STARTED SchemaHandlerIntegrationTest > schemaHandlerKeyspaceDoesNotExist(VertxTestContext) > schemaHandlerKeyspaceDoesNotExist(VertxTestContext): 5.1 PASSED SchemaHandlerIntegrationTest > schemaHandlerWithKeyspace(VertxTestContext) > schemaHandlerWithKeyspace(VertxTestContext): 5.1 STARTED SchemaHandlerIntegrationTest > schemaHandlerWithKeyspace(VertxTestContext) > schemaHandlerWithKeyspace(VertxTestContext): 5.1 PASSED BUILD SUCCESSFUL in 1m 16s 28 actionable tasks: 1 executed, 27 up-to-date

After:

$ ./gradlew --stacktrace :integration-tests:integrationTestLightWeight --tests org.apache.cassandra.sidecar.routes.SchemaHandlerIntegrationTest > Task :integration-tests:integrationTestLightWeight SchemaHandlerIntegrationTest > testSchemaHandlerWithReservedKeywordKeyspace() STARTED SchemaHandlerIntegrationTest > testSchemaHandlerWithReservedKeywordKeyspace() PASSED SchemaHandlerIntegrationTest > testSchemaHandlerWithCaseSensitiveKeyspace() STARTED SchemaHandlerIntegrationTest > testSchemaHandlerWithCaseSensitiveKeyspace() PASSED SchemaHandlerIntegrationTest > testListKeyspaces() STARTED SchemaHandlerIntegrationTest > testListKeyspaces() PASSED SchemaHandlerIntegrationTest > testSchemaHandlerWithKeyspace() STARTED SchemaHandlerIntegrationTest > testSchemaHandlerWithKeyspace() PASSED SchemaHandlerIntegrationTest > testSchemaHandlerKeyspaceDoesNotExist() STARTED SchemaHandlerIntegrationTest > testSchemaHandlerKeyspaceDoesNotExist() PASSED BUILD SUCCESSFUL in 19s 21 actionable tasks: 10 executed, 11 up-to-date

frankgh · 2025-02-08T17:51:33Z

server/src/test/integration/org/apache/cassandra/sidecar/testing/IntegrationTestBase.java


-                session.execute("CREATE KEYSPACE " + IF_NOT_EXISTS + " " + TEST_KEYSPACE
-                              + " WITH REPLICATION = { 'class' : 'NetworkTopologyStrategy', " + generateRfString(rf) + " };");
+                ResultSet rs = session.execute("CREATE KEYSPACE " + IF_NOT_EXISTS + " " + keyspaceName


tests should try to use cluster.schemaChange instead, but I think we should make an effort to migrate integration tests.

yifan-c force-pushed the CASSSIDECAR-204/trunk branch from b1a32c6 to 41a6fa9 Compare February 6, 2025 21:50

CASSSIDECAR-204: Improve integration test stability

d8782aa

Patch by Yifan Cai; Reviewed by TBD for CASSSIDECAR-204

yifan-c force-pushed the CASSSIDECAR-204/trunk branch from 41a6fa9 to d8782aa Compare February 6, 2025 21:50

yifan-c commented Feb 6, 2025

View reviewed changes

yifan-c added 6 commits February 6, 2025 15:33

cleanup comments

e4e1728

cleanup and synchronize

f163b38

Update test

c1c79eb

Update test

a8cef26

stage the test result htmls

7cd4c85

store test result in CI artifacts

503a78b

frankgh approved these changes Feb 10, 2025

View reviewed changes

yifan-c merged commit 72e38ea into apache:trunk Feb 10, 2025

		// array of nodeNums that are 1-based
		private int[] instancesToManage = null;

CASSSIDECAR-204: Improve integration test stability #188

CASSSIDECAR-204: Improve integration test stability #188

Uh oh!

Conversation

yifan-c commented Feb 6, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

frankgh left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants