[GitHub] [cassandra-sidecar] bbotella commented on a diff in pull request #48: CASSANDRASC-51: Use in-jvm dtest framework for integration tests.

["bbotella (via GitHub)" <gi...@apache.org>]

bbotella commented on code in PR #48:
URL: https://github.com/apache/cassandra-sidecar/pull/48#discussion_r1222138911


##########
build.gradle:
##########
@@ -24,6 +24,8 @@ plugins {
     id 'com.google.cloud.tools.jib' version '2.2.0'
 }
 
+ext.dtestJar = System.getenv("DTEST_JAR") ?: "dtest-5.0.jar" // trunk is currently 5.0.jar - update when trunk moves

Review Comment:
   Should we move this default to gradle.properties?



##########
README.md:
##########
@@ -27,9 +27,88 @@ While setting up cassandra instance, make sure the data directories of cassandra
 Testing
 -------
 
-We rely on docker containers for integration tests.
+We rely on the in-jvm dtest framework for testing. You must manually build the dtest jars before you start integration tests.
+At the moment, the JMX feature is unreleased in Cassandra, so you can use the following to build from the PR branches:
 
-The only requirement is to install and run [Docker](https://www.docker.com/products/docker-desktop/) on your test machine.
+```shell
+./scripts/build-dtest-jars.sh
+```
+
+The build script supports two parameters:
+- `REPO` - the Cassandra git repository to use for the source files. This is helpful if you need to test with a fork of the Cassandra codebase.
+  - default: `git@github.com:apache/cassandra.git`
+- `BRANCHES` - a space-delimited list of branches to build.
+  -default: `"cassandra-3.11 cassandra-4.1"`

Review Comment:
   Just a nit: I think the actual defaults on the script are different?



##########
src/test/integration/org/apache/cassandra/sidecar/common/DelegateTest.java:
##########
@@ -32,31 +36,59 @@
  */
 class DelegateTest
 {
+    private static CassandraAdapterDelegate getCassandraAdapterDelegate(CassandraTestContext context)
+    {
+        CassandraVersionProvider versionProvider = new CassandraVersionProvider.Builder()
+                                                       .add(new Cassandra311Factory(DnsResolver.DEFAULT))
+                                                       .add(new CassandraFactory(DnsResolver.DEFAULT))
+                                                       .build();
+        InstanceMetadata instanceMetadata = context.instancesConfig.instances().get(0);
+        CQLSessionProvider sessionProvider = new CQLSessionProvider(instanceMetadata.host(), instanceMetadata.port(), 1000);
+        CassandraAdapterDelegate delegate = new CassandraAdapterDelegate(versionProvider, sessionProvider, context.jmxClient());
+        return delegate;
+    }
+
     @CassandraIntegrationTest
     void testCorrectVersionIsEnabled(CassandraTestContext context)
     {
-        CassandraVersionProvider provider = new CassandraVersionProvider.Builder().add(new V30()).build();
-        CassandraAdapterDelegate delegate = new CassandraAdapterDelegate(provider, context.session, context.jmxClient);
+        CassandraAdapterDelegate delegate = getCassandraAdapterDelegate(context);
         SimpleCassandraVersion version = delegate.version();
         assertThat(version).isNotNull();
+        assertThat(version.major).isEqualTo(context.version.major);
+        assertThat(version.minor).isEqualTo(context.version.minor);
+        assertThat(version).isGreaterThanOrEqualTo(context.version);
     }
 
     @CassandraIntegrationTest
-    void testHealthCheck(CassandraTestContext context) throws IOException, InterruptedException
+    void testHealthCheck(CassandraTestContext context) throws InterruptedException
     {
-        CassandraVersionProvider provider = new CassandraVersionProvider.Builder().add(new V30()).build();
-        CassandraAdapterDelegate delegate = new CassandraAdapterDelegate(provider, context.session, context.jmxClient);
+        CassandraAdapterDelegate delegate = getCassandraAdapterDelegate(context);
 
         delegate.healthCheck();
 
         assertThat(delegate.isUp()).as("health check succeeds").isTrue();
 
-        context.container.execInContainer("nodetool", "disablebinary");
+        NodeToolResult nodetoolResult = context.cluster.get(1).nodetoolResult("disablebinary");
+        assertThat(nodetoolResult.getRc())
+        .withFailMessage("Failed to disable binary:\nstdout:" + nodetoolResult.getStdout()
+                         + "\nstderr: " + nodetoolResult.getStderr())
+        .isEqualTo(0);
 
-        delegate.healthCheck();
+        for (int i = 0; i < 10; i++)

Review Comment:
   Just curious. Why 10?



-- 
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: pr-unsubscribe@cassandra.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org