fix: rename 3.x driver-core CCM ITs to *Test.java so Surefire discovers them - #982
Conversation
…rs them driver-core has no Failsafe plugin binding (only bound in driver-tests/osgi/*), and Surefire's default includes never match *IT.java, so these CCM integration tests were silently never executed by `mvn verify -Pshort`/`-Plong`. Renaming to *Test.java matches Surefire's default discovery pattern, mirroring the fix already applied to DriverConfigReportingCcmIT in scylladb#973. Renamed: TabletsIT, ZeroTokenNodesIT, LWTLoadBalancingIT, SchemaBuilderIT. Now that LWTLoadBalancingTest actually runs, it surfaced a real (previously undetected) bug: both test methods constructed a SimpleStatement with bound values and then passed it to session.prepare(), which rejects statements carrying values. Fixed by preparing the value-free statement and binding values only on the resulting PreparedStatement, as the tests already intended. All classes verified live against ScyllaDB 2026.1.0: Tablets (3), ZeroTokenNodes (7), and LWTLoadBalancing (2) tests pass. SchemaBuilderTest's 6 methods remain pre-existing enabled=false, unrelated to this fix. Fixes scylladb#981. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Important Review skippedNo new commits to review since the last review. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthrough
Sequence Diagram(s)sequenceDiagram
participant LWTLoadBalancingTest
participant Cluster
participant Replica
LWTLoadBalancingTest->>Cluster: Execute fresh bound SELECT
Cluster->>Replica: Resolve coordinator
Replica-->>LWTLoadBalancingTest: Return queried host
LWTLoadBalancingTest->>LWTLoadBalancingTest: Check coordinator results
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
CCMBridge.add(int, int) unconditionally included `-t <thriftItf>` in the `ccm add` command, but scylla-ccm's `add` command has no Thrift option at all (Scylla never had a Thrift interface) -- passing it makes the whole command fail with "ccm: error: no such option: -t". Confirmed against scylladb/scylla-ccm's actual ClusterAddCmd parser (ccmlib/cmds/cluster_cmds.py, master). ZeroTokenNodesTest is the only caller of this method, and it never ran before the scylladb#981 rename fix, so this was never caught. All three "Scylla ITs" CI matrix legs on scylladb#982 failed with the identical error once the rename made the test actually execute. Verified locally against a venv with the real `scylla-ccm` (master, same as CI's `make install-scylla-ccm`) installed: all 7 ZeroTokenNodesTest methods pass, plus a full regression of the other 3 renamed classes (12/12). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@driver-core/src/test/java/com/datastax/driver/core/CCMBridge.java`:
- Around line 739-751: Update the isScylla initialization in CCMBridge to derive
from the instance scyllaVersion value, using whether scyllaVersion is non-null
rather than the global Scylla property. Preserve the existing Scylla command
options in the add-node branch and ensure withScylla(true).withVersion(...)
selects that branch.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 72af2586-e8f0-4e42-8b3b-5f39b2a5e6bb
📒 Files selected for processing (5)
driver-core/src/test/java/com/datastax/driver/core/CCMBridge.javadriver-core/src/test/java/com/datastax/driver/core/TabletsTest.javadriver-core/src/test/java/com/datastax/driver/core/ZeroTokenNodesTest.javadriver-core/src/test/java/com/datastax/driver/core/policies/LWTLoadBalancingTest.javadriver-core/src/test/java/com/datastax/driver/core/schemabuilder/SchemaBuilderTest.java
Review follow-up on scylladb#982. Renaming these classes made them execute for the first time, but several of their assertions could not fail. Each fix below was verified live against ScyllaDB 2026.1.0. TabletsTest, two independent false-passes: - `removeTableMappings(KEYSPACE_NAME)` passed the mixed-case "tabletsTest" while TabletMap keys arrive lowercased from the server and are matched with an exact equals(), so the "empty out tablets information" step silently cleared nothing and an iteration could be satisfied by state learned in a previous one. Lowercased, as the three sibling call sites already do. - `executeOnAllHostsAndReturnIfResultHasTabletsInfo` pins the statement via setHost() and never clears it, so checkIfRoutedProperly re-executed a pinned statement and always observed exactly one coordinator -- `nodes.size() <= REPLICATION_FACTOR` could not fail. The pin is now cleared before the routing check. - Additionally, checkIfRoutedProperly now clears Statement.getLastHost() per iteration. PagingOptimizingLoadBalancingPolicy returns that host ahead of the real query plan and PagingOptimizingLatencyTracker sets it after every successful BoundStatement execution, which pinned the loop to its first coordinator for the bound-statement half of the matrix. LWTLoadBalancingTest could not distinguish PRESERVE_REPLICA_ORDER from RANDOM, for two reasons: - The framework's default keyspace is hardcoded to RF=1, so "the first replica" was trivially unique and hasSize(1) held under REGULAR routing too. initTestKeyspace() is now overridden to create an RF=3 keyspace (tablets disabled on Scylla, as elsewhere for replica-placement tests), following SingleTokenIntegrationTest's template. - Both tests re-executed one BoundStatement instance, so the paging optimisation described above pinned the coordinator after the first query -- hasSize(1) was guaranteed by that, not by the LWT path. Coordinator collection now binds a fresh statement per execution. - Added should_spread_non_serial_select_across_replicas as the control: same statement, same table, same policy, non-serial consistency level, asserting the coordinator does vary. Verified that it fails (1 coordinator) if REPLICATION_FACTOR is dropped back to 1, so the two hasSize(1) assertions are now load-bearing. ZeroTokenNodesTest asserted on a HashSet with containsExactly, which is order-sensitive; HashSet iteration order is hash-derived, so this was a latent flake. Switched to containsOnly, already used everywhere else in the file (AssertJ 1.7.1, which this module pins, has no containsExactlyInAnyOrder; for a Set containsOnly is equivalent). CCMBridge derived the instance's `isScylla` from the global scylla.version property instead of the constructor's scyllaVersion, unlike the sibling `isDSE = dseVersion != null` one line above. Now derived from the instance. Behaviour-neutral today -- Builder.scylla already defaults to the global, withScylla() has no callers, and every bridge that is actually built takes build()'s !versionConfigured branch where scyllaVersion *is* the global -- so this removes a footgun rather than fixing a live bug. Verified: TabletsTest 3/3, ZeroTokenNodesTest 7/7, LWTLoadBalancingTest 3/3 against ScyllaDB 2026.1.0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
driver-core/src/test/java/com/datastax/driver/core/policies/LWTLoadBalancingTest.java (1)
109-113: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUse a conditional statement for serial-consistency routing tests.
TokenAwarePolicychecks the normal consistency level orStatement.isLWT(). It does not usegetSerialConsistencyLevel()for routing. Replace theseSELECTstatements with conditionalINSERT,UPDATE, orDELETEstatements. SetConsistencyLevel.ONEas the normal level and setLOCAL_SERIALorSERIALwithsetSerialConsistencyLevel. Assert both consistency properties.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@driver-core/src/test/java/com/datastax/driver/core/policies/LWTLoadBalancingTest.java` around lines 109 - 113, Update the serial-consistency routing tests around simpleSelect and preparedSelect to use a conditional INSERT, UPDATE, or DELETE rather than a SELECT, so the statement is recognized as an LWT. Set the normal consistency to ConsistencyLevel.ONE, set the serial level through setSerialConsistencyLevel using LOCAL_SERIAL or SERIAL as appropriate, and assert both consistency properties before exercising TokenAwarePolicy routing.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@driver-core/src/test/java/com/datastax/driver/core/CCMBridge.java`:
- Line 433: Update CCMBridge cluster creation to use the resolved scyllaVersion
consistently, including buildCreateCommand(), add(), and Scylla-specific
configuration, so withScylla(true).withVersion(...) emits the Scylla command and
does not depend solely on GLOBAL_SCYLLA_VERSION_NUMBER. Add a regression test
covering explicit Scylla mode with a configured version.
---
Outside diff comments:
In
`@driver-core/src/test/java/com/datastax/driver/core/policies/LWTLoadBalancingTest.java`:
- Around line 109-113: Update the serial-consistency routing tests around
simpleSelect and preparedSelect to use a conditional INSERT, UPDATE, or DELETE
rather than a SELECT, so the statement is recognized as an LWT. Set the normal
consistency to ConsistencyLevel.ONE, set the serial level through
setSerialConsistencyLevel using LOCAL_SERIAL or SERIAL as appropriate, and
assert both consistency properties before exercising TokenAwarePolicy routing.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: fa5db620-a44f-4cfe-aa0c-729e4f931435
📒 Files selected for processing (4)
driver-core/src/test/java/com/datastax/driver/core/CCMBridge.javadriver-core/src/test/java/com/datastax/driver/core/TabletsTest.javadriver-core/src/test/java/com/datastax/driver/core/ZeroTokenNodesTest.javadriver-core/src/test/java/com/datastax/driver/core/policies/LWTLoadBalancingTest.java
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@driver-core/src/test/java/com/datastax/driver/core/CCMBridge.java`:
- Around line 1397-1403: Update CCMBridge execution-environment construction to
use ResolvedVersions rather than the immutable global ENVIRONMENT_MAP, setting
SCYLLA_PRODUCT to enterprise when Scylla is enabled with an explicit
year-prefixed version while preserving OSS behavior otherwise. Add a regression
test covering withScylla(true).withVersion(VersionNumber.parse("2026.1.0")) and
assert the execution environment contains the enterprise product setting.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d6baeeff-a1f7-4b60-9336-3b7de1230c34
📒 Files selected for processing (2)
driver-core/src/test/java/com/datastax/driver/core/CCMBridge.javadriver-core/src/test/java/com/datastax/driver/core/CCMBridgeCreateCommandTest.java
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
driver-core/src/test/java/com/datastax/driver/core/CCMBridge.java (1)
1232-1269: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftResolve TLS configuration from
ResolvedVersions.Line 1267 now supports an explicitly configured Scylla cluster when global
scylla.versionis absent. However,withSSL()andwithAuth()select Cassandra JKS properties fromGLOBAL_SCYLLA_VERSION_NUMBERbeforeresolveVersions()runs. The resulting Scylla cluster receiveskeystoreandtruststoresettings instead of Scyllacertificate,keyfile, and PEM truststore settings.Record the SSL and authentication options in the builder. Apply the flavor-specific configuration after resolving
versions. Add coverage forwithScylla(true).withVersion(...).withSSL().🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@driver-core/src/test/java/com/datastax/driver/core/CCMBridge.java` around lines 1232 - 1269, Update the builder’s withSSL() and withAuth() flow to record the requested options rather than selecting properties from GLOBAL_SCYLLA_VERSION_NUMBER before resolution. After resolveVersions() determines the active flavor, apply the corresponding Cassandra/DSE or Scylla keystore, certificate, keyfile, and truststore settings using ResolvedVersions; add coverage for withScylla(true).withVersion(...).withSSL() to verify PEM-based Scylla TLS configuration.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@driver-core/src/test/java/com/datastax/driver/core/CCMBridge.java`:
- Around line 258-260: Remove SCYLLA_PRODUCT from envMap before constructing
BASE_ENVIRONMENT_MAP, ensuring inherited process values cannot reach explicit
OSS environments through buildEnvironmentMap. Preserve withScyllaEnterprise as
the only path that adds SCYLLA_PRODUCT for resolved Enterprise versions.
---
Outside diff comments:
In `@driver-core/src/test/java/com/datastax/driver/core/CCMBridge.java`:
- Around line 1232-1269: Update the builder’s withSSL() and withAuth() flow to
record the requested options rather than selecting properties from
GLOBAL_SCYLLA_VERSION_NUMBER before resolution. After resolveVersions()
determines the active flavor, apply the corresponding Cassandra/DSE or Scylla
keystore, certificate, keyfile, and truststore settings using ResolvedVersions;
add coverage for withScylla(true).withVersion(...).withSSL() to verify PEM-based
Scylla TLS configuration.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a1b5b70b-6582-4c58-9681-b04f09d77f90
📒 Files selected for processing (2)
driver-core/src/test/java/com/datastax/driver/core/CCMBridge.javadriver-core/src/test/java/com/datastax/driver/core/CCMBridgeCreateCommandTest.java
🚧 Files skipped from review as they are similar to previous changes (1)
- driver-core/src/test/java/com/datastax/driver/core/CCMBridgeCreateCommandTest.java
|
The two outside-diff findings can't be replied to inline, so answering both here. Detail is in the PR description under "Review round 4". Client encryption resolved from Conditional statement for the LWT routing tests — not applying this one. The premise is that |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@driver-core/src/test/java/com/datastax/driver/core/CCMBridge.java`:
- Around line 260-269: Update the initialization around ENVIRONMENT_MAP and
BASE_ENVIRONMENT_MAP so inherited SCYLLA_PRODUCT is removed from globalEnvMap
for numeric OSS versions, pure Cassandra runs, and unset versions, while
preserving it only when the raw scylla.version is an unparsable branch
specification; ensure the globalScyllaEnterprise path continues to apply
withScyllaEnterprise as intended.
- Around line 1072-1073: Update the version configuration flow around
CCMBridge’s dse and scylla flavor state so every explicit Cassandra version is
marked with withScylla(false), preventing GLOBAL_SCYLLA_VERSION_NUMBER from
classifying it as Scylla; alternatively, add equivalent explicit flavor handling
in CCMTestsSupport and RecommissionedNodeTest. Ensure resolveVersions() and
buildCreateCommand() receive the correct Cassandra flavor for those versions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 36ef99b3-f691-47af-ae74-84a91a4d79c5
📒 Files selected for processing (2)
driver-core/src/test/java/com/datastax/driver/core/CCMBridge.javadriver-core/src/test/java/com/datastax/driver/core/CCMBridgeCreateCommandTest.java
58bf916 to
5b3588e
Compare
|
Restructured this PR to keep it to the point of #981. Review had grown it to 649/205, with ~60% being What is left here is 245/136 — the rename, the @dkropachev your three threads on |
|
@coderabbitai review |
✅ Action performedReview finished.
|
CCMBridge.add(int, int) unconditionally included `-t <thriftItf>` in the `ccm add` command, but scylla-ccm's `add` command has no Thrift option at all (Scylla never had a Thrift interface) -- passing it makes the whole command fail with "ccm: error: no such option: -t". Confirmed against scylladb/scylla-ccm's actual ClusterAddCmd parser (ccmlib/cmds/cluster_cmds.py, master). ZeroTokenNodesTest is the only caller of this method, and it never ran before the #981 rename fix, so this was never caught. All three "Scylla ITs" CI matrix legs on #982 failed with the identical error once the rename made the test actually execute. Verified locally against a venv with the real `scylla-ccm` (master, same as CI's `make install-scylla-ccm`) installed: all 7 ZeroTokenNodesTest methods pass, plus a full regression of the other 3 renamed classes (12/12). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2 participants
Behavior
driver-core's*IT.javaCCM integration tests were silently never executed by CI:driver-core/pom.xmldoesn't bind themaven-failsafe-pluginin its own<build><plugins>— it's only declared in the rootpluginManagement, and actually bound only insidedriver-tests/osgi/*.<includes>(**/Test*.java,**/*Test.java,**/*Tests.java,**/*TestCase.java) never match*IT.java.-Pshort/-Plongprofiles (used bymake test-integration-scylla/test-integration-cassandra, i.e.mvn verify -Pshort) only set the TestNGtest.groupsfilter, which only narrows tests within files Surefire already selected by name — it can't rescue files Surefire never picked up in the first place.Confirmed via
mvn help:effective-pomand by decompiling the actualSurefireMojodefaults from themaven-surefire-pluginjar — no override exists anywhere in this repo's POMs fordriver-core.Changes
Renamed the 4 affected classes to match Surefire's default discovery pattern, mirroring the identical fix already applied to
DriverConfigReportingCcmIT→DriverConfigReportingCcmTestin #973:TabletsIT→TabletsTestZeroTokenNodesIT→ZeroTokenNodesTestLWTLoadBalancingIT→LWTLoadBalancingTestSchemaBuilderIT→SchemaBuilderTestNo pom.xml changes needed — pure
git mv+ updating eachpublic classdeclaration.Now that
LWTLoadBalancingTestactually runs, it surfaced a real, previously undetected bug: both test methods built aSimpleStatementwith bound values already attached and then passed it tosession.prepare(), which throwsIllegalArgumentException: A statement to prepare should not have values. This file was added in a single commit and had never executed before, so nobody caught it. Fixed by preparing the value-free statement and binding actual values only on the resultingPreparedStatement, matching what the test already did on the next line.Follow-up:
ccm addalso broke on ScyllaOnce the rename made
ZeroTokenNodesTestactually execute, every "Scylla ITs" CI leg failed uniformly withccm: error: no such option: -t. Root cause:CCMBridge.add(int, int)unconditionally passed-t <thriftItf>toccm add, butscylla-ccm'saddcommand has no Thrift option at all — Scylla never had a Thrift interface. Confirmed againstscylla-ccm's actualClusterAddCmdparser (ccmlib/cmds/cluster_cmds.py, master).Fixed by branching on
isScyllaand omitting-t/the thrift interface entirely for Scylla clusters.add(int, int)is also reached viaadd(int n)→add(1, n)fromMetadataTest,SessionLeakTest,StateListenerTest,RefreshConnectedHostTest, andNodeRefreshDebouncerTest— not justZeroTokenNodesTest— so this fix covers all of them whenever they run against a Scylla cluster, not only the newly-enabled test.Review round: making the activated assertions actually assert something
Review raised that renaming these classes made them execute, but their assertions had never been scrutinised — and several could not fail.
The recurring cause is worth calling out, because it bit two of the three classes:
PagingOptimizingLoadBalancingPolicy.newQueryPlanreturnsStatement.getLastHost()ahead of the real query plan, andPagingOptimizingLatencyTracker.updatesets that field after every successfulBoundStatementexecution. Any test that re-executes oneBoundStatementinstance in a loop is therefore pinned to its first coordinator, so "all executions hit the same node" holds no matter how routing actually behaves.TabletsTest— two independent false-passes:removeTableMappings(KEYSPACE_NAME)passed mixed-case"tabletsTest", butTabletMapkeys arrive lowercased from the server and are matched with an exactequals(), so the "empty out tablets information" step silently cleared nothing and an iteration could be satisfied by state learned in a previous one. Lowercased, as the three sibling call sites already did.executeOnAllHostsAndReturnIfResultHasTabletsInfopins the statement viasetHost()and never clears it, socheckIfRoutedProperlyre-executed a pinned statement andnodes.size() <= REPLICATION_FACTORcould not fail. The pin is now cleared before the routing check, andcheckIfRoutedProperlyalso clearssetLastHost(null)per iteration to defeat the paging pin described above.LWTLoadBalancingTest— could not distinguishPRESERVE_REPLICA_ORDERfromRANDOM:CCMTestsSupport.initTestKeyspaceformatsCREATE_KEYSPACE_SIMPLE_FORMATwith a literal1, regardless of@CCMConfig(numberOfNodes = 3)), so "the first replica" was trivially unique. Now overridesinitTestKeyspace()to create an RF=3 keyspace, followingSingleTokenIntegrationTest's template — RF=3 soSERIALreads get a 2-of-3 Paxos quorum rather than 2-of-2. Tablets are disabled on Scylla, as in the other replica-placement tests, because an unlearned tablet map yields an empty replica list and drops the LWT plan back to the child policy.BoundStatement, sohasSize(1)was guaranteed by the paging optimisation. Coordinator collection now binds a fresh statement per execution.should_spread_non_serial_select_across_replicasas a control — same statement, table and policy, only a non-serial CL — asserting the coordinator does vary.ZeroTokenNodesTest— asserted on aHashSetwith the order-sensitivecontainsExactly;HashSetiteration order is hash-derived, so this was a latent flake. Switched tocontainsOnly(AssertJ 1.7.1, which this module pins, has nocontainsExactlyInAnyOrder; for aSetit's equivalent, and it is what the rest of the file already uses).CCMBridge—isScyllawas derived from the globalscylla.versionproperty rather than the constructor'sscyllaVersion, unlike the siblingisDSE = dseVersion != nullone line above. Now instance-derived. Behaviour-neutral today (Builder.scyllaalready defaults to the global,withScylla()has no callers, and every bridge actually built takesbuild()'s!versionConfiguredbranch wherescyllaVersionis the global) — a footgun removal, not a live bug fix.Scope:
CCMBridgeflavor resolution moved to #983Review of this PR went on to find that
CCMBridgeresolves the server flavor from global system properties throughout, not just inisScylla—buildCreateCommandignores the Scylla flag and emits-v 3.0.8,SCYLLA_PRODUCTderives from the globalscylla.version, andwithSSL()/withAuth()pick the JKS-vs-PEM yaml at builder-configuration time.Fixes for all three were written and live-verified on this branch, then pulled back out: they're a refactor of the CCM test harness, not part of unblocking the tests, and none of it is reachable from CI (the IT legs run
mvn verify -Pshort, and no enabledshort-group test configures a version while creating a cluster). Tracked as #983, with the full analysis and the two remaining CodeRabbit findings recorded there, and a follow-up PR carrying the commits verbatim. Sibling to #800, which is the same bug class in 4.x.This PR is now just the rename, the
ccm addfix the rename made necessary, and the assertion fixes above.Not addressed: rewriting the LWT tests around a conditional statement
CodeRabbit also asked for
LWTLoadBalancingTestto use a conditionalINSERT/UPDATE/DELETEwithsetSerialConsistencyLevel, on the grounds thatTokenAwarePolicy"does not usegetSerialConsistencyLevel()for routing". That premise doesn't apply to these tests: they don't set the serial consistency level. They set the normal level toLOCAL_SERIAL/SERIAL, andTokenAwarePolicy.getRequestRouting(TokenAwarePolicy.java:476-488) returns the LWT routing method whenstatement.isLWT()or when the normal levelisSerial(). The second branch is exactly what is under test, which is why the tests assertisLWT()isfalse. The level survivesprepare()→bind():AbstractSession.prepareAsynccopies it onto thePreparedStatement(AbstractSession.java:128-131) and theBoundStatementconstructor copies it back off (BoundStatement.java:89-90).The suggested rewrite would exercise the
isLWT()branch instead and drop coverage of the serial-level branch. CoveringisLWT()as well would be a reasonable addition, but it belongs to a feature this PR didn't enable — happy to open it as a follow-up.Testing
mvn -pl driver-core -am compile test-compile— clean compile.mvn -pl driver-core test -Dtest=<ClassName> -Dtest.groups=shortnow actually attempts these classes (previouslyTests run: 0).TabletsTest: 3/3 pass — now with routing genuinely exercisedZeroTokenNodesTest: 7/7 passLWTLoadBalancingTest: 3/3 pass (2 pre-existing + the new non-serial control)SchemaBuilderTest: 0 tests run — all 6 methods are pre-existingenabled = false, unrelated to this change.BoundStatement, the non-serial control collapses to 1 coordinator — identical to the serial tests, i.e. indistinguishable.REPLICATION_FACTORdropped back to 1, the control fails outright (1 coordinator), confirming the RF bump is what makes the twohasSize(1)assertions meaningful.Scylla ITsLATEST / LTS-LATEST / LTS-PRIOR andCassandra ITs 3-LATEST). An earlier attempt on an intermediate head failed once onShardAwarenessTest.correctShardInTracingTest, a pre-existing flake unrelated to this change.Fixes #981.
🤖 Generated with Claude Code