From adbdd2494a570110341570529a600c8661949d44 Mon Sep 17 00:00:00 2001 From: janehe Date: Mon, 5 May 2025 19:36:30 -0700 Subject: [PATCH 01/16] java 8 --- CONTRIBUTING.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 53857383cf2..c082dea90b3 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -19,6 +19,10 @@ under the License. # Contributing guidelines +## Set up development environment + +Make sure you are using Java 8. + ## Code formatting ### Java From 9adec6c8919c899e01a0bb6d5ce24d593414af4f Mon Sep 17 00:00:00 2001 From: janehe Date: Tue, 6 May 2025 08:42:00 -0700 Subject: [PATCH 02/16] WIP --- CONTRIBUTING.md | 31 +++++++++++++++++++------------ 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index c082dea90b3..d13362a3e01 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -19,9 +19,23 @@ under the License. # Contributing guidelines -## Set up development environment +Thank you for your interest in contributing to Apache Cassandra Java Driver! -Make sure you are using Java 8. +# Bugs, features, and questions +If you have a bug to report or a feature request, please file a ticket on our [JIRA](https://issues.apache.org/jira/projects/CASSJAVA). +If you have a question, please ask on our [user mailing list](https://groups.google.com/a/lists.datastax.com/g/java-driver-user). + +# Development environment setup + +## Build and IDE configuration +Make sure you have maven installed and you are using exactly Java 8. Run `mvn clean package -DskipTests` to build the project. + +If you open the project with an IDE like IntelliJ, you may find out that the guava-shaded classes are not recognized. +You can fix this by running `mvn clean install -DskipTests`. +IntelliJ may use a built-in mvn version, which is not the same in your shell. +In that case, you need to click the buttons in the IntelliJ maven window. Under `Lifecycle`, click `clean` and then `install`. + +## Run tests ## Code formatting @@ -387,22 +401,15 @@ In that case, you don't need to use a rule chain. ## Running the tests ### Unit tests +Make sure you are using Java 8, and run the following command: - mvn clean test - -This currently takes about 30 seconds. The goal is to keep it within a couple of minutes (it runs -for each commit if you enable the pre-commit hook -- see below). + mvn clean install -DskipTests + mvn test ### Integration tests mvn clean verify -This currently takes about 9 minutes. We don't have a hard limit, but ideally it should stay within -30 minutes to 1 hour. - -You can skip test categories individually with `-DskipParallelizableITs`, `-DskipSerialITs` and -`-DskipIsolatedITs` (`-DskipITs` still works to skip them all at once). - ### Configuring MacOS for Simulacron Simulacron (used in integration tests) relies on loopback aliases to simulate multiple nodes. On From 5c1caa5195e3b99739074a2ad0679330409b159a Mon Sep 17 00:00:00 2001 From: janehe Date: Thu, 15 May 2025 17:21:02 -0700 Subject: [PATCH 03/16] loopback --- CONTRIBUTING.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index d13362a3e01..2dd30b10306 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -408,6 +408,16 @@ Make sure you are using Java 8, and run the following command: ### Integration tests +1. Install Cassandra Cluster Manager (CCM) following their (README)[https://github.com/apache/cassandra-ccm]. +2. If on MacOS, turn on loopback aliases. + ```shell + for i in {2..255}; do sudo ifconfig lo0 alias 127.0.0.$i up; done + ``` + Note this will cause netowkring to become slow. When you finish testing, run the following to tear those down + ```shell + for i in {2..255}; do sudo ifconfig lo0 -alias 127.0.0.$i up; done + ``` + mvn clean verify ### Configuring MacOS for Simulacron From 692c64b55f7f2004cc2b2b0d45f6d8c656156a31 Mon Sep 17 00:00:00 2001 From: janehe Date: Thu, 15 May 2025 19:01:36 -0700 Subject: [PATCH 04/16] fmt --- CONTRIBUTING.md | 39 +++++++++++++++------------------------ 1 file changed, 15 insertions(+), 24 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 2dd30b10306..0a752ec36f2 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -35,8 +35,6 @@ You can fix this by running `mvn clean install -DskipTests`. IntelliJ may use a built-in mvn version, which is not the same in your shell. In that case, you need to click the buttons in the IntelliJ maven window. Under `Lifecycle`, click `clean` and then `install`. -## Run tests - ## Code formatting ### Java @@ -417,33 +415,26 @@ Make sure you are using Java 8, and run the following command: ```shell for i in {2..255}; do sudo ifconfig lo0 -alias 127.0.0.$i up; done ``` - +3. Run the integration tests + ```shell mvn clean verify + ``` + Use the flag `-Dccm.version=` to run integration tests against a specific version of Apache Cassandra. + ```shell + mvn verify -Dccm.version=3.11.0 + ``` -### Configuring MacOS for Simulacron - -Simulacron (used in integration tests) relies on loopback aliases to simulate multiple nodes. On -Linux or Windows, you shouldn't have anything to do. On MacOS, run this script: - -``` -#!/bin/bash -for sub in {0..4}; do - echo "Opening for 127.0.$sub" - for i in {0..255}; do sudo ifconfig lo0 alias 127.0.$sub.$i up; done -done -``` - -Note that this is known to cause temporary increased CPU usage in OS X initially while mDNSResponder -acclimates itself to the presence of added IP addresses. This lasts several minutes. Also, this does -not survive reboots. - - -## License headers +## Code formatting and license headers -The build will fail if some license headers are missing. To update all files from the command line, -run: +We follow the [Google Java Style Guide](https://google.github.io/styleguide/javaguide.html). See +https://github.com/google/google-java-format for IDE plugins. ``` +# Run the following for java files +mvn fmt:format +# For XML files +mvn xml-format:xml-format +# For license headers mvn license:format ``` From ab1233ed74b9fb3a2697d1d0ee596e1aee4da0bb Mon Sep 17 00:00:00 2001 From: janehe Date: Thu, 15 May 2025 19:01:54 -0700 Subject: [PATCH 05/16] hook --- CONTRIBUTING.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 0a752ec36f2..171feb0d2b1 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -438,6 +438,7 @@ mvn xml-format:xml-format mvn license:format ``` + ## Pre-commit hook (highly recommended) Ensure `pre-commit.sh` is executable, then run: From 4a24e9758964b2751d2d6f5a52901b9e275fd048 Mon Sep 17 00:00:00 2001 From: janehe Date: Thu, 15 May 2025 19:08:17 -0700 Subject: [PATCH 06/16] hook --- CONTRIBUTING.md | 1 - .../driver/internal/core/cql/CqlRequestHandler.java | 10 ++++++++++ .../core/type/codec/registry/DefaultCodecRegistry.java | 2 +- pre-commit.sh | 6 +++++- 4 files changed, 16 insertions(+), 3 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 171feb0d2b1..0a752ec36f2 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -438,7 +438,6 @@ mvn xml-format:xml-format mvn license:format ``` - ## Pre-commit hook (highly recommended) Ensure `pre-commit.sh` is executable, then run: diff --git a/core/src/main/java/com/datastax/oss/driver/internal/core/cql/CqlRequestHandler.java b/core/src/main/java/com/datastax/oss/driver/internal/core/cql/CqlRequestHandler.java index 0808bdce63f..fd88f8401b5 100644 --- a/core/src/main/java/com/datastax/oss/driver/internal/core/cql/CqlRequestHandler.java +++ b/core/src/main/java/com/datastax/oss/driver/internal/core/cql/CqlRequestHandler.java @@ -15,6 +15,16 @@ * See the License for the specific language governing permissions and * limitations under the License. */ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or moreicenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ package com.datastax.oss.driver.internal.core.cql; import com.datastax.oss.driver.api.core.AllNodesFailedException; diff --git a/core/src/main/java/com/datastax/oss/driver/internal/core/type/codec/registry/DefaultCodecRegistry.java b/core/src/main/java/com/datastax/oss/driver/internal/core/type/codec/registry/DefaultCodecRegistry.java index 4334f22b63d..cc14740e180 100644 --- a/core/src/main/java/com/datastax/oss/driver/internal/core/type/codec/registry/DefaultCodecRegistry.java +++ b/core/src/main/java/com/datastax/oss/driver/internal/core/type/codec/registry/DefaultCodecRegistry.java @@ -162,7 +162,7 @@ public int hashCode() { // NOTE: inlined Objects.hash for performance reasons (avoid Object[] allocation // seen in profiler allocation traces) return ((31 + Objects.hashCode(cqlType)) * 31 + Objects.hashCode(javaType)) * 31 - + Boolean.hashCode(isJavaCovariant); + + Boolean.hashCode(isJavaCovariant); } } } diff --git a/pre-commit.sh b/pre-commit.sh index 912564ae81e..9be78ab3caf 100755 --- a/pre-commit.sh +++ b/pre-commit.sh @@ -19,7 +19,11 @@ # STASH_NAME="pre-commit-$(date +%s)" # git stash save --keep-index $STASH_NAME -mvn clean test +mvn fmt:format +mvn xml-format:xml-format +mvn license:format +mvn clean install -DskipTests +mvn test RESULT=$? # STASHES=$(git stash list) From afc1d382a26ccf96f1ed047865e5200e0d8a8d84 Mon Sep 17 00:00:00 2001 From: janehe Date: Mon, 19 May 2025 13:09:16 -0700 Subject: [PATCH 07/16] CONTRIBUTING.md and pre commit hook --- CONTRIBUTING.md | 584 +++++++----------------------------------------- pre-commit.sh | 8 - 2 files changed, 81 insertions(+), 511 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 0a752ec36f2..72831ef39df 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -17,530 +17,108 @@ specific language governing permissions and limitations under the License. --> -# Contributing guidelines +# Contributing Guidelines -Thank you for your interest in contributing to Apache Cassandra Java Driver! +Thank you for your interest in contributing to the Apache Cassandra Java Driver! Please review the following guidelines to help you get started. -# Bugs, features, and questions -If you have a bug to report or a feature request, please file a ticket on our [JIRA](https://issues.apache.org/jira/projects/CASSJAVA). -If you have a question, please ask on our [user mailing list](https://groups.google.com/a/lists.datastax.com/g/java-driver-user). +## Table of Contents -# Development environment setup +- [Bugs, Features, and Questions](#bugs-features-and-questions) +- [Development Environment Setup](#development-environment-setup) +- [Before You Submit a Pull Request](#before-you-submit-a-pull-request) -## Build and IDE configuration -Make sure you have maven installed and you are using exactly Java 8. Run `mvn clean package -DskipTests` to build the project. +## Bugs, Features, and Questions -If you open the project with an IDE like IntelliJ, you may find out that the guava-shaded classes are not recognized. -You can fix this by running `mvn clean install -DskipTests`. -IntelliJ may use a built-in mvn version, which is not the same in your shell. -In that case, you need to click the buttons in the IntelliJ maven window. Under `Lifecycle`, click `clean` and then `install`. +- To report bugs or request features, please file a ticket on our [JIRA](https://issues.apache.org/jira/projects/CASSJAVA). +- For questions, use our [user mailing list](https://groups.google.com/a/lists.datastax.com/g/java-driver-user). -## Code formatting +## Development Environment Setup -### Java +### Build and IDE Configuration -We follow the [Google Java Style Guide](https://google.github.io/styleguide/javaguide.html). See -https://github.com/google/google-java-format for IDE plugins. The rules are not configurable. - -The build will fail if the code is not formatted. To format all files from the command line, run: - -``` -mvn fmt:format -``` - -Some aspects are not covered by the formatter: braces must be used with `if`, `else`, `for`, `do` -and `while` statements, even when the body is empty or contains only a single statement. - -### XML - -The build will fail if XML files are not formatted correctly. Run the following command before you -commit: - -```java -mvn xml-format:xml-format -``` - -The formatter does not enforce a maximum line length, but please try to keep it below 100 characters -to keep files readable across all mediums (IDE, terminal, Github...). - -### Other text files (markdown, etc) - -Similarly, enforce a right margin of 100 characters in those files. Editors and IDEs generally have -a way to configure this (for IDEA, install the "Wrap to column" plugin). - -## Coding style -- production code - -Do not use static imports. They make things harder to understand when you look at the code -someplace where you don't have IDE support, like Github's code view. - -Avoid abbreviations in class and variable names. A good rule of thumb is that you should only use -them if you would also do so verbally, for example "id" and "config" are probably reasonable. -Single-letter variables are permissible if the variable scope is only a few lines, or for commonly -understood cases (like `i` for a loop index). - -Keep source files short. Short files are easy to understand and test. The average should probably -be around 200-300 lines. - -### Javadoc - -All types in "API" packages must be documented. For "internal" packages, documentation is optional, -but in no way discouraged: it's generally a good idea to have a class-level comment that explains -where the component fits in the architecture, and anything else that you feel is important. - -You don't need to document every parameter or return type, or even every method. Don't document -something if it is completely obvious, we don't want to end up with this: - -```java -/** - * Returns the name. - * - * @return the name - */ -String getName(); -``` - -On the other hand, there is often something useful to say about a method, so most should have at -least a one-line comment. Use common sense. - -Driver users coding in their IDE should find the right documentation at the right time. Try to -think of how they will come into contact with the class. For example, if a type is constructed with -a builder, each builder method should probably explain what the default is when you don't call it. - -Avoid using too many links, they can make comments harder to read, especially in the IDE. Link to a -type the first time it's mentioned, then use a text description ("this registry"...) or an `@code` -block. Don't link to a class in its own documentation. Don't link to types that appear right below -in the documented item's signature. - -```java -/** -* @return this {@link Builder} <-- completely unnecessary -*/ -Builder withLimit(int limit) { -``` - -### Logs - -We use SLF4J; loggers are declared like this: - -```java -private static final Logger LOG = LoggerFactory.getLogger(TheEnclosingClass.class); -``` - -Logs are intended for two personae: - -* Ops who manage the application in production. -* Developers (maybe you) who debug a particular issue. - -The first 3 log levels are for ops: - -* `ERROR`: something that renders the driver -- or a part of it -- completely unusable. An action is - required to fix it: bouncing the client, applying a patch, etc. -* `WARN`: something that the driver can recover from automatically, but indicates a configuration or - programming error that should be addressed. For example: the driver connected successfully, but - one of the contact points in the configuration was malformed; the same prepared statement is being - prepared multiple time by the application code. -* `INFO`: something that is part of the normal operation of the driver, but might be useful to know - for an operator. For example: the driver has initialized successfully and is ready to process - queries; an optional dependency was detected in the classpath and activated an enhanced feature. - -Do not log errors that are rethrown to the client (such as the error that you're going to complete a -request with). This is annoying for ops because they see a lot of stack traces that require no -actual action on their part, because they're already handled by application code. - -Similarly, do not log stack traces for non-critical errors. If you still want the option to get the -trace for debugging, see the `Loggers.warnWithException` utility. - -The last 2 levels are for developers, to help follow what the driver is doing from a "black box" -perspective (think about debugging an issue remotely, and all you have are the logs). - -* `TRACE`: anything that happens **for every user request**. Not only request handling, but all - related components (e.g. timestamp generators, policies, etc). -* `DEBUG`: everything else. For example, node state changes, control connection activity, etc. - -Note that `DEBUG` and `TRACE` can coexist within the same component, for example the LBP -initializing is a one-time event, but returning a query plan is a per-request event. - -Logs statements start with a prefix that identifies its origin, for example: - -* for components that are unique to the cluster instance, just the cluster name: `[c0]`. -* for sessions, the cluster name + a generated unique identifier: `[c0|s0]`. -* for channel pools, the session identifier + the address of the node: `[c0|s0|/127.0.0.2:9042]`. -* for channels, the identifier of the owner (session or control connection) + the Netty identifier, - which indicates the local and remote ports: - `[c0|s0|id: 0xf9ef0b15, L:/127.0.0.1:51482 - R:/127.0.0.1:9042]`. -* for request handlers, the session identifier, a unique identifier, and the index of the - speculative execution: `[c0|s0|1077199500|0]`. - -Tests run with the configuration defined in `src/test/resources/logback-test.xml`. The default level -for driver classes is `WARN`, but you can override it with a system property: `-DdriverLevel=DEBUG`. -A nice setup is to use `DEBUG` when you run from your IDE, and keep the default for the command -line. - -When you add or review new code, take a moment to run the tests in `DEBUG` mode and check if the -output looks good. - -### Don't abuse the stream API - -The `java.util.stream` API is often used (abused?) as a "functional API for collections": - -```java -List sizes = words.stream().map(String::length).collect(Collectors.toList()); -``` - -The perceived advantages of this approach over traditional for-loops are debatable: - -* readability: this is highly subjective. But consider the following: - * everyone can read for-loops, whether they are familiar with the Stream API or not. The opposite - is not true. - * the stream API does not spell out all the details: what kind of list does `Collectors.toList()` - return? Is it pre-sized? Mutable? Thread-safe? - * the stream API looks pretty on simple examples, but things can get ugly fast. Try rewriting - `NetworkTopologyReplicationStrategy` with streams. -* concision: this is irrelevant. When we look at code we care about maintainability, not how many - keystrokes the author saved. The for-loop version of the above example is just 5 lines long, and - your brain doesn't take longer to parse it. - -The bottom line: don't try to "be functional" at all cost. Plain old for-loops are often just as -simple. - -### Never assume a specific format for `toString()` - -Only use `toString()` for debug logs or exception messages, and always assume that its format is -unspecified and can change at any time. - -If you need a specific string representation for a class, make it a dedicated method with a -documented format, for example `toCqlLiteral`. Otherwise it's too easy to lose track of the intended -usage and break things: for example, someone modifies your `toString()` method to make their logs -prettier, but unintentionally breaks the script export feature that expected it to produce CQL -literals. - -`toString()` can delegate to `toCqlLiteral()` if that is appropriate for logs. - - -### Concurrency annotations - -We use the [JCIP annotations](http://jcip.net/annotations/doc/index.html) to document thread-safety -policies. - -Add them for all new code, with the exception of: - -* enums and interfaces; -* utility classes (only static methods); -* test code. - -Make sure you import the types from `net.jcip`, there are homonyms in the classpath. - - -### Nullability annotations - -We use the [Spotbugs annotations](https://spotbugs.github.io) to document nullability of parameters, -method return types and class members. - -Please annotate any new class or interface with the appropriate annotations: `@NonNull`, `@Nullable`. Make sure you import -the types from `edu.umd.cs.findbugs.annotations`, there are homonyms in the classpath. - - -## Coding style -- test code - -Static imports are permitted in a couple of places: -* All AssertJ methods, e.g.: - ```java - assertThat(node.getDatacenter()).isNotNull(); - fail("Expecting IllegalStateException to be thrown"); - ``` -* All Mockito methods, e.g.: - ```java - when(codecRegistry.codecFor(DataTypes.INT)).thenReturn(codec); - verify(codec).decodePrimitive(any(ByteBuffer.class), eq(ProtocolVersion.DEFAULT)); +- Ensure Maven is installed and you are using Java 8. +- Build the project with: ``` -* All Awaitility methods, e.g.: - ```java - await().until(() -> somethingBecomesTrue()); + mvn clean package -DskipTests ``` +- If using an IDE like IntelliJ and encountering issues with guava-shaded classes: + - Run: + ``` + mvn clean install -DskipTests + ``` + - If IntelliJ uses a different Maven version, use the Maven window in IntelliJ: under `Lifecycle`, click `clean` and then `install`. -Test methods names use lower snake case, generally start with `should`, and clearly indicate the -purpose of the test, for example: `should_fail_if_key_already_exists`. If you have trouble coming -up with a simple name, it might be a sign that your test does too much, and should be split. - -We use AssertJ (`assertThat`) for assertions. Don't use JUnit assertions (`assertEquals`, -`assertNull`, etc). - -Don't try to generify at all cost: a bit of duplication is acceptable, if that helps keep the tests -simple to understand (a newcomer should be able to understand how to fix a failing test without -having to read too much code). - -Test classes can be a bit longer, since they often enumerate similar test cases. You can also -factor some common code in a parent abstract class named with "XxxTestBase", and then split -different families of tests into separate child classes. For example, `CqlRequestHandlerTestBase`, -`CqlRequestHandlerRetryTest`, `CqlRequestHandlerSpeculativeExecutionTest`... - -### Unit tests - -They live in the same module as the code they are testing. They should be fast and not start any -external process. They usually target one specific component and mock the rest of the driver -context. - -### Integration tests - -They live in the `integration-tests` module, and exercise the whole driver stack against an external -process, which can be either one of: -* [Simulacron](https://github.com/datastax/simulacron): simulates Cassandra nodes on loopback - addresses; your test must "prime" data, i.e. tell the nodes what results to return for - pre-determined queries. - - For an example of a Simulacron-based test, see `NodeTargetingIT`. -* [CCM](https://github.com/pcmanus/ccm): launches actual Cassandra nodes locally. The `ccm` - executable must be in the path. - - You can pass a `-Dccm.version` system property to the build to target a particular Cassandra - version (it defaults to 3.11.0). `-Dccm.directory` allows you to point to a local installation - -- this can be a checkout of the Cassandra codebase, as long as it's built. See `CcmBridge` in - the driver codebase for more details. - - For an example of a CCM-based test, see `PlainTextAuthProviderIT`. - -#### Categories - -Integration tests are divided into three categories: - -##### Parallelizable tests - -These tests can be run in parallel, to speed up the build. They either use: -* dedicated Simulacron instances. These are lightweight, and Simulacron will manage the ports to - make sure that there are no collisions. -* a shared, one-node CCM cluster. Each test works in its own keyspace. - -The build runs them with a configurable degree of parallelism (currently 8). The shared CCM cluster -is initialized the first time it's used, and stopped before moving on to serial tests. Note that we -run with `parallel=classes`, which means methods within the same class never run concurrent to each -other. - -To make an integration test parallelizable, annotate it with `@Category(ParallelizableTests.class)`. -If you use CCM, it **must** be with `CcmRule`. - -For an example of a Simulacron-based parallelizable test, see `NodeTargetingIT`. For a CCM-based -test, see `DirectCompressionIT`. - -##### Serial tests - -These tests cannot run in parallel, in general because they require CCM clusters of different sizes, -or with a specific configuration (we never run more than one CCM cluster simultaneously: it would be -too resource-intensive, and too complicated to manage all the ports). - -The build runs them one by one, after the parallelizable tests. - -To make an integration test serial, do not annotate it with `@Category`. The CCM rule **must** be -`CustomCcmRule`. - -For an example, see `DefaultLoadBalancingPolicyIT`. - -Note: if multiple serial tests have a common "base" class, do not pull up `CustomCcmRule`, each -child class must have its own instance. Otherwise they share the same CCM instance, and the first -one destroys it on teardown. See `TokenITBase` for how to organize code in those cases. - -##### Isolated tests - -Not only can those tests not run in parallel, they also require specific environment tweaks, -typically system properties that need to be set before initialization. - -The build runs them one by one, *each in its own JVM fork*, after the serial tests. - -To isolate an integration test, annotate it with `@Category(IsolatedTests.class)`. The CCM rule -**must** be `CustomCcmRule`. - -For an example, see `HeapCompressionIT`. - -#### About test rules - -Do not mix `CcmRule` and `SimulacronRule` in the same test. It makes things harder to follow, and -can be inefficient (if the `SimulacronRule` is method-level, it will create a Simulacron cluster for -every test method, even those that only need CCM). - -##### Class-level rules - -Rules annotated with `@ClassRule` wrap the whole test class, and are reused across methods. Try to -use this as much as possible, as it's more efficient. The fields need to be static; also make them -final and use constant naming conventions, like `CCM_RULE`. - -When you use a server rule (`CcmRule` or `SimulacronRule`) and a `SessionRule` at the same level, -wrap them into a rule chain to ensure proper initialization order: - -```java -private static final CcmRule CCM_RULE = CcmRule.getInstance(); -private static final SessionRule SESSION_RULE = SessionRule.builder(CCM_RULE).build(); - -@ClassRule -public static final TestRule CHAIN = RuleChain.outerRule(CCM_RULE).around(SESSION_RULE); -``` - -##### Method-level rules - -Rules annotated with `@Rule` wrap each test method. Use lower-camel case for field names: - -```java -private CcmRule ccmRule = CcmRule.getInstance(); -private SessionRule sessionRule = SessionRule.builder(ccmRule).build(); - -@ClassRule -public TestRule chain = RuleChain.outerRule(ccmRule).around(sessionRule); -``` - -Only use this for: - -* CCM tests that use `@CassandraRequirement` or `@DseRequirement` restrictions at the method level - (ex: `BatchStatementIT`). -* tests where you *really* need to restart from a clean state for every method. - -##### Mixed - -It's also possible to use a `@ClassRule` for CCM / Simulacron, and a `@Rule` for the session rule. -In that case, you don't need to use a rule chain. +### Running the Tests -## Running the tests +#### Unit Tests -### Unit tests -Make sure you are using Java 8, and run the following command: +- Ensure you are using Java 8. +- Run: + ``` + mvn clean install -DskipTests + mvn test + ``` - mvn clean install -DskipTests - mvn test +#### Integration Tests + +1. Install Cassandra Cluster Manager (CCM) following its [README](https://github.com/apache/cassandra-ccm). +2. On macOS, enable loopback aliases: + ```shell + for i in {2..255}; do sudo ifconfig lo0 alias 127.0.0.$i up; done + ``` + Note: This may slow down networking. To remove the aliases after testing: + ```shell + for i in {2..255}; do sudo ifconfig lo0 -alias 127.0.0.$i up; done + ``` +3. Run integration tests: + ``` + mvn clean verify + ``` + To target a specific Cassandra version: + ``` + mvn verify -Dccm.version=3.11.0 + ``` + +### Code Formatting and License Headers + +- We follow the [Google Java Style Guide](https://google.github.io/styleguide/javaguide.html). See [google-java-format](https://github.com/google/google-java-format) for IDE plugins. +- To format code: + ``` + # Java files + mvn fmt:format + # XML files + mvn xml-format:xml-format + # License headers + mvn license:format + ``` -### Integration tests +### Pre-commit Hook (Highly Recommended) -1. Install Cassandra Cluster Manager (CCM) following their (README)[https://github.com/apache/cassandra-ccm]. -2. If on MacOS, turn on loopback aliases. - ```shell - for i in {2..255}; do sudo ifconfig lo0 alias 127.0.0.$i up; done +- Make `pre-commit.sh` executable: ``` - Note this will cause netowkring to become slow. When you finish testing, run the following to tear those down - ```shell - for i in {2..255}; do sudo ifconfig lo0 -alias 127.0.0.$i up; done + chmod +x pre-commit.sh ``` -3. Run the integration tests - ```shell - mvn clean verify +- Set up the pre-commit hook: ``` - Use the flag `-Dccm.version=` to run integration tests against a specific version of Apache Cassandra. - ```shell - mvn verify -Dccm.version=3.11.0 - ``` - -## Code formatting and license headers - -We follow the [Google Java Style Guide](https://google.github.io/styleguide/javaguide.html). See -https://github.com/google/google-java-format for IDE plugins. - -``` -# Run the following for java files -mvn fmt:format -# For XML files -mvn xml-format:xml-format -# For license headers -mvn license:format -``` - -## Pre-commit hook (highly recommended) - -Ensure `pre-commit.sh` is executable, then run: - -``` -ln -s ../../pre-commit.sh .git/hooks/pre-commit -``` - -This will only allow commits if the tests pass. It is also a good reminder to keep the test suite -short. - -Note: the tests run on the current state of the working directory. I tried to add a `git stash` in -the script to only test what's actually being committed, but I couldn't get it to run reliably -(it's still in there but commented). Keep this in mind when you commit, and don't forget to re-add -the changes if the first attempt failed and you fixed the tests. - -## Speeding up the build for local tests - -If you need to install something in your local repository quickly, you can use the `fast` profile to -skip all "non-essential" checks (licenses, formatting, tests, etc): - -``` -mvn clean install -Pfast -``` - -You can speed things up even more by targeting specific modules with the `-pl` option: - -``` -mvn clean install -Pfast -pl core,query-builder,mapper-runtime,mapper-processor,bom -``` - -Please run the normal build at least once before you push your changes. - -## Commits - -Keep your changes **focused**. Each commit should have a single, clear purpose expressed in its -message. - -Resist the urge to "fix" cosmetic issues (add/remove blank lines, move methods, etc.) in existing -code. This adds cognitive load for reviewers, who have to figure out which changes are relevant to -the actual issue. If you see legitimate issues, like typos, address them in a separate commit (it's -fine to group multiple typo fixes in a single commit). - -Isolate trivial refactorings into separate commits. For example, a method rename that affects dozens -of call sites can be reviewed in a few seconds, but if it's part of a larger diff it gets mixed up -with more complex changes (that might affect the same lines), and reviewers have to check every -line. - -Commit message subjects start with a capital letter, use the imperative form and do **not** end -with a period: - -* correct: "Add test for CQL request handler" -* incorrect: "~~Added test for CQL request handler~~" -* incorrect: "~~New test for CQL request handler~~" - -Avoid catch-all messages like "Minor cleanup", "Various fixes", etc. They don't provide any useful -information to reviewers, and might be a sign that your commit contains unrelated changes. - -We don't enforce a particular subject line length limit, but try to keep it short. - -You can add more details after the subject line, separated by a blank line. The following pattern -(inspired by [Netty](http://netty.io/wiki/writing-a-commit-message.html)) is not mandatory, but -welcome for complex changes: - -``` -One line description of your change - -Motivation: - -Explain here the context, and why you're making that change. -What is the problem you're trying to solve. - -Modifications: - -Describe the modifications you've done. - -Result: - -After your change, what will change. -``` - -## Pull requests - -Like commits, pull requests should be focused on a single, clearly stated goal. + ln -s ../../pre-commit.sh .git/hooks/pre-commit + ``` +- This script will format files and run unit tests before each commit. -Don't base a pull request onto another one, it's too complicated to follow two branches that evolve -at the same time. If a ticket depends on another, wait for the first one to be merged. +## Before You Submit a Pull Request -If you have to address feedback, avoid rewriting the history (e.g. squashing or amending commits): -this makes the reviewers' job harder, because they have to re-read the full diff and figure out -where your new changes are. Instead, push a new commit on top of the existing history; it will be -squashed later when the PR gets merged. If the history is complex, it's a good idea to indicate in -the message where the changes should be squashed: +### Pull Request Titles and Commit Messages -``` -* 20c88f4 - Address feedback (to squash with "Add metadata parsing logic") (36 minutes ago) -* 7044739 - Fix various typos in Javadocs (2 days ago) -* 574dd08 - Add metadata parsing logic (2 days ago) -``` +- Pull request titles must follow the format: `{JIRA-ticket-number}: {JIRA-ticket-title}` + Example: `CASSJAVA-40: Driver testing against Java 21` +- Before merging, squash your commits into a single commit with a message formatted as: + ``` + {JIRA-ticket-number}: {JIRA-ticket-title} + patch by {your-name}; reviewed by {approver-1} and {approver-2} for {JIRA-ticket-number} + ``` -(Note that the message refers to the other commit's subject line, not the SHA-1. This way it's still -relevant if there are intermediary rebases.) +### Coding Styles -If you need new stuff from the base branch, it's fine to rebase and force-push, as long as you don't -rewrite the history. Just give a heads up to the reviewers beforehand. Don't push a merge commit to -a pull request. +1. Javadoc: All types in API packages must be documented. For internal packages, documentation is optional. +2. Nullability: Please use the [Spotbugs annotations](https://spotbugs.github.io) to document nullability of new class or interface using the `@Nullable` and `@NonNull` annotations from the package `edu.umd.cs.findbugs.annotations`. +3. Static imports: Please only use static imports for AssertJ, Mockito, and Awaitility. +4. Concurrency annotations: Please use the [JCIP annotations](http://jcip.net/annotations/doc/index.html) to document thread-safety. \ No newline at end of file diff --git a/pre-commit.sh b/pre-commit.sh index 9be78ab3caf..60b115dedd4 100755 --- a/pre-commit.sh +++ b/pre-commit.sh @@ -16,9 +16,6 @@ # specific language governing permissions and limitations # under the License. -# STASH_NAME="pre-commit-$(date +%s)" -# git stash save --keep-index $STASH_NAME - mvn fmt:format mvn xml-format:xml-format mvn license:format @@ -26,10 +23,5 @@ mvn clean install -DskipTests mvn test RESULT=$? -# STASHES=$(git stash list) -# if [[ $STASHES == *$STASH_NAME* ]]; then -# git stash pop -# fi - [ $RESULT -ne 0 ] && exit 1 exit 0 From efc2fd7501134fef74ed3a75eb39817c3e1ee735 Mon Sep 17 00:00:00 2001 From: janehe Date: Mon, 19 May 2025 13:16:17 -0700 Subject: [PATCH 08/16] reset cqlrequesthandler --- .../driver/internal/core/cql/CqlRequestHandler.java | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/core/src/main/java/com/datastax/oss/driver/internal/core/cql/CqlRequestHandler.java b/core/src/main/java/com/datastax/oss/driver/internal/core/cql/CqlRequestHandler.java index fd88f8401b5..0808bdce63f 100644 --- a/core/src/main/java/com/datastax/oss/driver/internal/core/cql/CqlRequestHandler.java +++ b/core/src/main/java/com/datastax/oss/driver/internal/core/cql/CqlRequestHandler.java @@ -15,16 +15,6 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or moreicenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ package com.datastax.oss.driver.internal.core.cql; import com.datastax.oss.driver.api.core.AllNodesFailedException; From fca6351a9e3b677b258f6f70d6ecc5fb2fcfb80b Mon Sep 17 00:00:00 2001 From: janehe Date: Wed, 15 Oct 2025 13:04:18 -0700 Subject: [PATCH 09/16] add coding guide --- CONTRIBUTING.md | 112 ++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 98 insertions(+), 14 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 72831ef39df..123e4995743 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -25,7 +25,8 @@ Thank you for your interest in contributing to the Apache Cassandra Java Driver! - [Bugs, Features, and Questions](#bugs-features-and-questions) - [Development Environment Setup](#development-environment-setup) -- [Before You Submit a Pull Request](#before-you-submit-a-pull-request) +- [To Submit a Pull Request](#to-submit-a-pull-request) +- [Coding Guide](#coding-guide) ## Bugs, Features, and Questions @@ -42,11 +43,11 @@ Thank you for your interest in contributing to the Apache Cassandra Java Driver! mvn clean package -DskipTests ``` - If using an IDE like IntelliJ and encountering issues with guava-shaded classes: - - Run: - ``` - mvn clean install -DskipTests - ``` - - If IntelliJ uses a different Maven version, use the Maven window in IntelliJ: under `Lifecycle`, click `clean` and then `install`. + - Run: + ``` + mvn clean install -DskipTests + ``` + - If IntelliJ uses a different Maven version, use the Maven window in IntelliJ: under `Lifecycle`, click `clean` and then `install`. ### Running the Tests @@ -104,9 +105,7 @@ Thank you for your interest in contributing to the Apache Cassandra Java Driver! ``` - This script will format files and run unit tests before each commit. -## Before You Submit a Pull Request - -### Pull Request Titles and Commit Messages +## To Submit a Pull Request - Pull request titles must follow the format: `{JIRA-ticket-number}: {JIRA-ticket-title}` Example: `CASSJAVA-40: Driver testing against Java 21` @@ -116,9 +115,94 @@ Thank you for your interest in contributing to the Apache Cassandra Java Driver! patch by {your-name}; reviewed by {approver-1} and {approver-2} for {JIRA-ticket-number} ``` -### Coding Styles - -1. Javadoc: All types in API packages must be documented. For internal packages, documentation is optional. -2. Nullability: Please use the [Spotbugs annotations](https://spotbugs.github.io) to document nullability of new class or interface using the `@Nullable` and `@NonNull` annotations from the package `edu.umd.cs.findbugs.annotations`. +## Coding Guide +### Javadoc + +- **API packages:** must be documented. +- **Internal packages:** optional but encouraged—add a short class-level comment explaining the component’s role. + +You don’t need to document every method or parameter. Skip obvious stuff like: + +``` +/** + * @return the name + */ +String getName(); +``` +### Logs +We use **SLF4J**: + +``` +private static final Logger LOG = LoggerFactory.getLogger(TheEnclosingClass.class); +``` + +Logs help two groups: +1. **Ops** – running the app in production. +2. **Developers** – debugging issues. + +**Log levels:** + +- **ERROR** – critical, needs action, not the ones re-thrown to the client +- **WARN** – recoverable but should be fixed. +- **INFO** – normal but useful events. +- **DEBUG** – detailed activity (e.g., state changes). +- **TRACE** – per-request details. + +To change the log level when running tests, modify the `src/test/resources/logback-test.xml`. + +**Log prefix** shows origin, e.g.: +`[s0|90232530|0]` (session name | hash code of the CqlRequestHandler instance | number of request attempts) + +### Integration Tests +Testing against an external process, using either of the tools: +- [Simulacron](https://github.com/datastax/simulacron): simulates Cassandra nodes on loopback. You “prime” it with expected query results. + *Example*: `NodeTargetingIT`. +- [CCM](https://github.com/pcmanus/ccm): launches actual Cassandra nodes locally. The `ccm` executable must be in the path. Pass variables like `-Dccm.version=5.0.0 -Dccm.dse=false` to specify the Cassandra version or distribution. + _Example:_ `PlainTextAuthProviderIT`. + +#### Test Categories + +##### Parallelizable +Run concurrently to speed up builds. They use either: +- Their own Simulacron instances (Simulacron manages ports to prevent conflicts.) +- A shared 1-node CCM cluster with separate keyspaces. Annotate with `@Category(ParallelizableTests.class)` and specify `CcmRule`. + +Classes are ran parallel, but methods in a class will never run concurrently. + +_Examples:_ `NodeTargetingIT` (Simulacron), `DirectCompressionIT` (CCM). +##### Serial +Run one by one (e.g., tests that need different CCM cluster sizes/configs). +Do **not** annotate with a category. Use `CustomCcmRule`. +_Example:_ `DefaultLoadBalancingPolicyIT`, `TokenITBase` +##### Isolated +Need special environment setup (e.g., system properties) and run in separate JVM forks. +Annotate with `@Category(IsolatedTests.class)`. Use `CustomCcmRule`. +_Example:_ `HeapCompressionIT`. + +#### Test Rules +**Class-level rules** (`@ClassRule`): +Apply to the whole class and are reused between methods. Preferred for efficiency. + +```java +private static final CcmRule CCM_RULE = CcmRule.getInstance(); +private static final SessionRule SESSION_RULE = SessionRule.builder(CCM_RULE).build(); + +@ClassRule +public static final TestRule CHAIN = RuleChain.outerRule(CCM_RULE).around(SESSION_RULE); +``` + +**Method-level rules** (`@Rule`): +Re-initialize for each test method. Use for method-specific requirements or clean states. + +```java +private CcmRule ccmRule = CcmRule.getInstance(); +private SessionRule sessionRule = SessionRule.builder(ccmRule).build(); + +@Rule +public TestRule chain = RuleChain.outerRule(ccmRule).around(sessionRule); +``` +### Miscellaneous +1. Nullability: Please use the [Spotbugs annotations](https://spotbugs.github.io) to document nullability of new class or interface using the `@Nullable` and `@NonNull` annotations from the package `edu.umd.cs.findbugs.annotations`. +2. Concurrency annotations: Please use the [JCIP annotations](http://jcip.net/annotations/doc/index.html) to document thread-safety. 3. Static imports: Please only use static imports for AssertJ, Mockito, and Awaitility. -4. Concurrency annotations: Please use the [JCIP annotations](http://jcip.net/annotations/doc/index.html) to document thread-safety. \ No newline at end of file +4. `toString()`: only for debug in development. Use `toCqlLiteral()` is applicable for logs. From b00e40ef42377f47017ede4a3d9b240e0895ac69 Mon Sep 17 00:00:00 2001 From: janehe Date: Wed, 15 Oct 2025 13:11:43 -0700 Subject: [PATCH 10/16] git stash back --- pre-commit.sh | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/pre-commit.sh b/pre-commit.sh index 60b115dedd4..9be78ab3caf 100755 --- a/pre-commit.sh +++ b/pre-commit.sh @@ -16,6 +16,9 @@ # specific language governing permissions and limitations # under the License. +# STASH_NAME="pre-commit-$(date +%s)" +# git stash save --keep-index $STASH_NAME + mvn fmt:format mvn xml-format:xml-format mvn license:format @@ -23,5 +26,10 @@ mvn clean install -DskipTests mvn test RESULT=$? +# STASHES=$(git stash list) +# if [[ $STASHES == *$STASH_NAME* ]]; then +# git stash pop +# fi + [ $RESULT -ne 0 ] && exit 1 exit 0 From 22c8a4ba61f5012abb48653e6fa55d6415596478 Mon Sep 17 00:00:00 2001 From: janehe Date: Thu, 30 Oct 2025 15:55:44 -0700 Subject: [PATCH 11/16] add braces --- CONTRIBUTING.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 123e4995743..9838d783056 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -92,6 +92,7 @@ Thank you for your interest in contributing to the Apache Cassandra Java Driver! # License headers mvn license:format ``` + - Braces must be used with `if`, `else`, `for`, `do` and `while` statements. ### Pre-commit Hook (Highly Recommended) From d091ae68f095e12aa0897df3bdb4ce56867daf09 Mon Sep 17 00:00:00 2001 From: janehe Date: Tue, 9 Dec 2025 21:43:33 -0800 Subject: [PATCH 12/16] make it long again --- CONTRIBUTING.md | 489 ++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 385 insertions(+), 104 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 9838d783056..4eb6347aba8 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -17,25 +17,104 @@ specific language governing permissions and limitations under the License. --> -# Contributing Guidelines +# Contributing to the Cassandra Java Driver -Thank you for your interest in contributing to the Apache Cassandra Java Driver! Please review the following guidelines to help you get started. +Thank you for your interest in contributing! ## Table of Contents +- [Ways to Contribute](#ways-to-contribute) +- [Contribution Process](#contribution-process) + - [Reporting Issues](#reporting-issues) + - [Submitting Changes (Pull Requests)](#submitting-changes-pull-requests) +- [Development Setup](#development-setup) + - [Prerequisites](#prerequisites) + - [Building the Project](#building-the-project) + - [Running Tests](#running-tests) +- [Coding Guidelines](#coding-guidelines) + - [General](#general) + - [Code Formatting and License Headers](#code-formatting-and-license-headers) + - [Javadoc](#javadoc) + - [Logging](#logging) + - [Don't abuse the stream API](#dont-abuse-the-stream-api) + - [Never assume a specific format for toString()](#never-assume-a-specific-format-for-tostring) + - [Concurrency annotations](#concurrency-annotations) + - [Nullability annotations](#nullability-annotations) +- [Coding Guidelines for Tests](#coding-guidelines-for-tests) + - [Coding style -- test code](#coding-style----test-code) + - [Unit tests](#unit-tests) + - [Integration tests](#integration-tests) -- [Bugs, Features, and Questions](#bugs-features-and-questions) -- [Development Environment Setup](#development-environment-setup) -- [To Submit a Pull Request](#to-submit-a-pull-request) -- [Coding Guide](#coding-guide) +--- -## Bugs, Features, and Questions +## Ways to Contribute -- To report bugs or request features, please file a ticket on our [JIRA](https://issues.apache.org/jira/projects/CASSJAVA). -- For questions, use our [user mailing list](https://groups.google.com/a/lists.datastax.com/g/java-driver-user). +There are many ways to contribute, including: -## Development Environment Setup +- **Bug Reports**: Identify incorrect behavior, inconsistencies, or regressions in the driver. Provide reproduction steps when possible. +- **Feature Requests**: Propose improvements or new functionality. Please describe the use case (not just a proposed API). +- **Documentation Improvements**: Enhance guides, examples, javadocs, or configuration explanations. +- **Pull Requests**: Submit fixes, enhancements, performance improvements, or refactorings. +- **Testing Contributions**: Add missing tests, improve coverage, or enhance test infrastructure. +- **Support & Triage**: Help evaluate reported issues or contribute to discussions. +- **Verify Releases**: Verify the release artifacts work correctly in your environment, when a release is proposed in the mailing list. -### Build and IDE Configuration +All contributions go through JIRA + GitHub + mailing list, as described below. + +--- + +## Contribution Process + +### Reporting Issues + +All issues must be tracked in **Apache JIRA**: + + +When filing an issue: + +- Clearly describe the problem, expected behavior, and actual behavior. +- Include driver version, Java version, and Cassandra cluster details. +- Add reproduction steps or a minimal test case if possible. +- Use the appropriate issue type (Bug, Improvement, New Feature, etc.). +- Set the correct components (e.g., `core`, `mapper-runtime`, `quarkus`). + +Committers will help refine the ticket if needed. + +### Submitting Changes (Pull Requests) + +All code changes require: + +1. **A corresponding JIRA ticket** + Include the JIRA key in the PR title, e.g.: + `CASSJAVA-40: Driver testing against Java 21` + +2. **A pull request on GitHub** + Repository: + +3. **Tests** + Every fix or feature should include or update tests. PRs without tests are rarely accepted. + +4. **Documentation updates** + Update manual, javadocs, examples, or reference docs when applicable. + +5. **Passing CI** + PRs must pass all CI jobs unless reviewers explicitly allow exceptions. + +**Do not** mix unrelated changes in one PR—keep contributions focused. + +**Do not** base a PR on another one. + +**Do not** squash commits before the PR is ready to merge. + +--- + +## Development Setup + +### Prerequisites + +- **Java 8+** +- **Maven** + +### Building the Project - Ensure Maven is installed and you are using Java 8. - Build the project with: @@ -43,29 +122,28 @@ Thank you for your interest in contributing to the Apache Cassandra Java Driver! mvn clean package -DskipTests ``` - If using an IDE like IntelliJ and encountering issues with guava-shaded classes: - - Run: - ``` - mvn clean install -DskipTests - ``` - - If IntelliJ uses a different Maven version, use the Maven window in IntelliJ: under `Lifecycle`, click `clean` and then `install`. + - Run: + ``` + mvn clean install -DskipTests + ``` + - If IntelliJ uses a different Maven version, use the Maven window in IntelliJ: under `Lifecycle`, click `clean` and then `install`. -### Running the Tests +### Running Tests #### Unit Tests -- Ensure you are using Java 8. -- Run: - ``` - mvn clean install -DskipTests - mvn test - ``` +```bash +mvn clean install -DskipTests +mvn test +``` #### Integration Tests 1. Install Cassandra Cluster Manager (CCM) following its [README](https://github.com/apache/cassandra-ccm). 2. On macOS, enable loopback aliases: - ```shell - for i in {2..255}; do sudo ifconfig lo0 alias 127.0.0.$i up; done + +```shell +for i in {2..255}; do sudo ifconfig lo0 alias 127.0.0.$i up; done ``` Note: This may slow down networking. To remove the aliases after testing: ```shell @@ -80,6 +158,18 @@ Thank you for your interest in contributing to the Apache Cassandra Java Driver! mvn verify -Dccm.version=3.11.0 ``` +--- + +## Coding Guidelines + +### General + +Do not use static imports. They make things harder to understand when you look at the code someplace where you don't have IDE support, like Github's code view. + +Avoid abbreviations in class and variable names. A good rule of thumb is that you should only use them if you would also do so verbally, for example "id" and "config" are probably reasonable. Single-letter variables are permissible if the variable scope is only a few lines, or for commonly understood cases (like i for a loop index). + +Keep source files short. Short files are easy to understand and test. The average should probably be around 200-300 lines. + ### Code Formatting and License Headers - We follow the [Google Java Style Guide](https://google.github.io/styleguide/javaguide.html). See [google-java-format](https://github.com/google/google-java-format) for IDE plugins. @@ -92,118 +182,309 @@ Thank you for your interest in contributing to the Apache Cassandra Java Driver! # License headers mvn license:format ``` - - Braces must be used with `if`, `else`, `for`, `do` and `while` statements. - -### Pre-commit Hook (Highly Recommended) -- Make `pre-commit.sh` executable: - ``` - chmod +x pre-commit.sh - ``` -- Set up the pre-commit hook: - ``` - ln -s ../../pre-commit.sh .git/hooks/pre-commit - ``` -- This script will format files and run unit tests before each commit. +### Javadoc +All types in "API" packages must be documented. For "internal" packages, documentation is optional, but in no way discouraged: it's generally a good idea to have a class-level comment that explains where the component fits in the architecture, and anything else that you feel is important. -## To Submit a Pull Request +You don't need to document every parameter or return type, or even every method. Don't document something if it is completely obvious, we don't want to end up with this: -- Pull request titles must follow the format: `{JIRA-ticket-number}: {JIRA-ticket-title}` - Example: `CASSJAVA-40: Driver testing against Java 21` -- Before merging, squash your commits into a single commit with a message formatted as: - ``` - {JIRA-ticket-number}: {JIRA-ticket-title} - patch by {your-name}; reviewed by {approver-1} and {approver-2} for {JIRA-ticket-number} - ``` +```java +/** +* Returns the name. +* +* @return the name + */ + String getName(); +``` -## Coding Guide -### Javadoc +On the other hand, there is often something useful to say about a method, so most should have at least a one-line comment. Use common sense. -- **API packages:** must be documented. -- **Internal packages:** optional but encouraged—add a short class-level comment explaining the component’s role. +Driver users coding in their IDE should find the right documentation at the right time. Try to think of how they will come into contact with the class. For example, if a type is constructed with a builder, each builder method should probably explain what the default is when you don't call it. -You don’t need to document every method or parameter. Skip obvious stuff like: +Avoid using too many links, they can make comments harder to read, especially in the IDE. Link to a type the first time it's mentioned, then use a text description ("this registry"...) or an @code block. Don't link to a class in its own documentation. Don't link to types that appear right below in the documented item's signature. -``` +```java /** - * @return the name - */ -String getName(); +* @return this {@link Builder} <-- completely unnecessary + */ + Builder withLimit(int limit) { ``` -### Logs -We use **SLF4J**: -``` +### Logging + +We use SLF4J; loggers are declared like this: + +```java private static final Logger LOG = LoggerFactory.getLogger(TheEnclosingClass.class); ``` -Logs help two groups: -1. **Ops** – running the app in production. -2. **Developers** – debugging issues. +Logs are intended for two personae: + +* Ops who manage the application in production. +* Developers (maybe you) who debug a particular issue. + +#### Log Levels + +* `ERROR`: something that renders the driver -- or a part of it -- completely unusable. An action is + required to fix it: bouncing the client, applying a patch, etc. +* `WARN`: something that the driver can recover from automatically, but indicates a configuration or + programming error that should be addressed. For example: the driver connected successfully, but + one of the contact points in the configuration was malformed; the same prepared statement is being + prepared multiple time by the application code. +* `INFO`: something that is part of the normal operation of the driver, but might be useful to know + for an operator. For example: the driver has initialized successfully and is ready to process + queries; an optional dependency was detected in the classpath and activated an enhanced feature. + +Do not log errors that are rethrown to the client (such as the error that you're going to complete a +request with). This is annoying for ops because they see a lot of stack traces that require no +actual action on their part, because they're already handled by application code. + +Similarly, do not log stack traces for non-critical errors. If you still want the option to get the +trace for debugging, see the `Loggers.warnWithException` utility. -**Log levels:** +The last 2 levels are for developers, to help follow what the driver is doing from a "black box" +perspective (think about debugging an issue remotely, and all you have are the logs). -- **ERROR** – critical, needs action, not the ones re-thrown to the client -- **WARN** – recoverable but should be fixed. -- **INFO** – normal but useful events. -- **DEBUG** – detailed activity (e.g., state changes). -- **TRACE** – per-request details. +* `TRACE`: anything that happens **for every user request**. Not only request handling, but all + related components (e.g. timestamp generators, policies, etc). +* `DEBUG`: everything else. For example, node state changes, control connection activity, etc. + +Note that `DEBUG` and `TRACE` can coexist within the same component, for example the LBP +initializing is a one-time event, but returning a query plan is a per-request event. -To change the log level when running tests, modify the `src/test/resources/logback-test.xml`. **Log prefix** shows origin, e.g.: `[s0|90232530|0]` (session name | hash code of the CqlRequestHandler instance | number of request attempts) -### Integration Tests -Testing against an external process, using either of the tools: -- [Simulacron](https://github.com/datastax/simulacron): simulates Cassandra nodes on loopback. You “prime” it with expected query results. - *Example*: `NodeTargetingIT`. -- [CCM](https://github.com/pcmanus/ccm): launches actual Cassandra nodes locally. The `ccm` executable must be in the path. Pass variables like `-Dccm.version=5.0.0 -Dccm.dse=false` to specify the Cassandra version or distribution. - _Example:_ `PlainTextAuthProviderIT`. -#### Test Categories +Tests run with the configuration defined in `src/test/resources/logback-test.xml`. The default level +for driver classes is `WARN`, but you can override it with a system property: `-DdriverLevel=DEBUG`. +A nice setup is to use `DEBUG` when you run from your IDE, and keep the default for the command +line. + +When you add or review new code, take a moment to run the tests in `DEBUG` mode and check if the +output looks good. -##### Parallelizable -Run concurrently to speed up builds. They use either: -- Their own Simulacron instances (Simulacron manages ports to prevent conflicts.) -- A shared 1-node CCM cluster with separate keyspaces. Annotate with `@Category(ParallelizableTests.class)` and specify `CcmRule`. -Classes are ran parallel, but methods in a class will never run concurrently. -_Examples:_ `NodeTargetingIT` (Simulacron), `DirectCompressionIT` (CCM). -##### Serial -Run one by one (e.g., tests that need different CCM cluster sizes/configs). -Do **not** annotate with a category. Use `CustomCcmRule`. -_Example:_ `DefaultLoadBalancingPolicyIT`, `TokenITBase` -##### Isolated -Need special environment setup (e.g., system properties) and run in separate JVM forks. -Annotate with `@Category(IsolatedTests.class)`. Use `CustomCcmRule`. -_Example:_ `HeapCompressionIT`. +### Don't abuse the stream API -#### Test Rules -**Class-level rules** (`@ClassRule`): -Apply to the whole class and are reused between methods. Preferred for efficiency. +The `java.util.stream` API is often used (abused?) as a "functional API for collections": ```java -private static final CcmRule CCM_RULE = CcmRule.getInstance(); -private static final SessionRule SESSION_RULE = SessionRule.builder(CCM_RULE).build(); +List sizes = words.stream().map(String::length).collect(Collectors.toList()); +``` + +The perceived advantages of this approach over traditional for-loops are debatable: + +* readability: this is highly subjective. But consider the following: + * everyone can read for-loops, whether they are familiar with the Stream API or not. The opposite + is not true. + * the stream API does not spell out all the details: what kind of list does `Collectors.toList()` + return? Is it pre-sized? Mutable? Thread-safe? + * the stream API looks pretty on simple examples, but things can get ugly fast. Try rewriting + `NetworkTopologyReplicationStrategy` with streams. +* concision: this is irrelevant. When we look at code we care about maintainability, not how many + keystrokes the author saved. The for-loop version of the above example is just 5 lines long, and + your brain doesn't take longer to parse it. + +The bottom line: don't try to "be functional" at all cost. Plain old for-loops are often just as +simple. + +### Never assume a specific format for `toString()` -@ClassRule +Only use `toString()` for debug logs or exception messages, and always assume that its format is +unspecified and can change at any time. + +If you need a specific string representation for a class, make it a dedicated method with a +documented format, for example `toCqlLiteral`. Otherwise it's too easy to lose track of the intended +usage and break things: for example, someone modifies your `toString()` method to make their logs +prettier, but unintentionally breaks the script export feature that expected it to produce CQL +literals. + +`toString()` can delegate to `toCqlLiteral()` if that is appropriate for logs. + + +### Concurrency annotations + +We use the [JCIP annotations](http://jcip.net/annotations/doc/index.html) to document thread-safety +policies. + +Add them for all new code, with the exception of: + +* enums and interfaces; +* utility classes (only static methods); +* test code. + +Make sure you import the types from `net.jcip`, there are homonyms in the classpath. + + +### Nullability annotations + +We use the [Spotbugs annotations](https://spotbugs.github.io) to document nullability of parameters, +method return types and class members. + +Please annotate any new class or interface with the appropriate annotations: `@NonNull`, `@Nullable`. Make sure you import +the types from `edu.umd.cs.findbugs.annotations`, there are homonyms in the classpath. + +## Coding Guidelines for Tests + +## Coding style -- test code + +Static imports are permitted in a couple of places: +* All AssertJ methods, e.g.: + ```java + assertThat(node.getDatacenter()).isNotNull(); + fail("Expecting IllegalStateException to be thrown"); + ``` +* All Mockito methods, e.g.: + ```java + when(codecRegistry.codecFor(DataTypes.INT)).thenReturn(codec); + verify(codec).decodePrimitive(any(ByteBuffer.class), eq(ProtocolVersion.DEFAULT)); + ``` +* All Awaitility methods, e.g.: + ```java + await().until(() -> somethingBecomesTrue()); + ``` + +Test methods names use lower snake case, generally start with `should`, and clearly indicate the +purpose of the test, for example: `should_fail_if_key_already_exists`. If you have trouble coming +up with a simple name, it might be a sign that your test does too much, and should be split. + +We use AssertJ (`assertThat`) for assertions. Don't use JUnit assertions (`assertEquals`, +`assertNull`, etc). + +Don't try to generify at all cost: a bit of duplication is acceptable, if that helps keep the tests +simple to understand (a newcomer should be able to understand how to fix a failing test without +having to read too much code). + +Test classes can be a bit longer, since they often enumerate similar test cases. You can also +factor some common code in a parent abstract class named with "XxxTestBase", and then split +different families of tests into separate child classes. For example, `CqlRequestHandlerTestBase`, +`CqlRequestHandlerRetryTest`, `CqlRequestHandlerSpeculativeExecutionTest`... + +### Unit tests + +They live in the same module as the code they are testing. They should be fast and not start any +external process. They usually target one specific component and mock the rest of the driver +context. + +### Integration tests + +They live in the `integration-tests` module, and exercise the whole driver stack against an external +process, which can be either one of: +* [Simulacron](https://github.com/datastax/simulacron): simulates Cassandra nodes on loopback + addresses; your test must "prime" data, i.e. tell the nodes what results to return for + pre-determined queries. + + For an example of a Simulacron-based test, see `NodeTargetingIT`. +* [CCM](https://github.com/pcmanus/ccm): launches actual Cassandra nodes locally. The `ccm` + executable must be in the path. + + You can pass a `-Dccm.version` system property to the build to target a particular Cassandra + version (it defaults to 3.11.0). `-Dccm.directory` allows you to point to a local installation + -- this can be a checkout of the Cassandra codebase, as long as it's built. See `CcmBridge` in + the driver codebase for more details. + + For an example of a CCM-based test, see `PlainTextAuthProviderIT`. + +#### Categories + +Integration tests are divided into three categories: + +##### Parallelizable tests + +These tests can be run in parallel, to speed up the build. They either use: +* dedicated Simulacron instances. These are lightweight, and Simulacron will manage the ports to + make sure that there are no collisions. +* a shared, one-node CCM cluster. Each test works in its own keyspace. + +The build runs them with a configurable degree of parallelism (currently 8). The shared CCM cluster +is initialized the first time it's used, and stopped before moving on to serial tests. Note that we +run with `parallel=classes`, which means methods within the same class never run concurrent to each +other. + +To make an integration test parallelizable, annotate it with `@Category(ParallelizableTests.class)`. +If you use CCM, it **must** be with `CcmRule`. + +For an example of a Simulacron-based parallelizable test, see `NodeTargetingIT`. For a CCM-based +test, see `DirectCompressionIT`. + +##### Serial tests + +These tests cannot run in parallel, in general because they require CCM clusters of different sizes, +or with a specific configuration (we never run more than one CCM cluster simultaneously: it would be +too resource-intensive, and too complicated to manage all the ports). + +The build runs them one by one, after the parallelizable tests. + +To make an integration test serial, do not annotate it with `@Category`. The CCM rule **must** be +`CustomCcmRule`. + +For an example, see `DefaultLoadBalancingPolicyIT`. + +Note: if multiple serial tests have a common "base" class, do not pull up `CustomCcmRule`, each +child class must have its own instance. Otherwise they share the same CCM instance, and the first +one destroys it on teardown. See `TokenITBase` for how to organize code in those cases. + +##### Isolated tests + +Not only can those tests not run in parallel, they also require specific environment tweaks, +typically system properties that need to be set before initialization. + +The build runs them one by one, *each in its own JVM fork*, after the serial tests. + +To isolate an integration test, annotate it with `@Category(IsolatedTests.class)`. The CCM rule +**must** be `CustomCcmRule`. + +For an example, see `HeapCompressionIT`. + +#### About test rules + +Do not mix `CcmRule` and `SimulacronRule` in the same test. It makes things harder to follow, and +can be inefficient (if the `SimulacronRule` is method-level, it will create a Simulacron cluster for +every test method, even those that only need CCM). + +Use the `@BackendRequirement` annotation to restrict the backend type and version. +Specify the Cassandra Distribution, CASSANDRA/DSE/HCD, and the version requirement. +For example, `@BackendRequirement(type = BackendType.CASSANDRA, minInclusive = "2.2")` + +##### Class-level rules + +Rules annotated with `@ClassRule` wrap the whole test class, and are reused across methods. Try to +use this as much as possible, as it's more efficient. The fields need to be static; also make them +final and use constant naming conventions, like `CCM_RULE`. + +When you use a server rule (`CcmRule` or `SimulacronRule`) and a `SessionRule` at the same level, +wrap them into a rule chain to ensure proper initialization order: + +```java +private static final CcmRule CCM_RULE = CcmRule.getInstance(); +private static final SessionRule SESSION_RULE = SessionRule.builder(CCM_RULE).build(); + +@ClassRule public static final TestRule CHAIN = RuleChain.outerRule(CCM_RULE).around(SESSION_RULE); ``` -**Method-level rules** (`@Rule`): -Re-initialize for each test method. Use for method-specific requirements or clean states. +##### Method-level rules + +Rules annotated with `@Rule` wrap each test method. Use lower-camel case for field names: ```java -private CcmRule ccmRule = CcmRule.getInstance(); -private SessionRule sessionRule = SessionRule.builder(ccmRule).build(); +private CcmRule ccmRule = CcmRule.getInstance(); +private SessionRule sessionRule = SessionRule.builder(ccmRule).build(); -@Rule +@ClassRule public TestRule chain = RuleChain.outerRule(ccmRule).around(sessionRule); ``` -### Miscellaneous -1. Nullability: Please use the [Spotbugs annotations](https://spotbugs.github.io) to document nullability of new class or interface using the `@Nullable` and `@NonNull` annotations from the package `edu.umd.cs.findbugs.annotations`. -2. Concurrency annotations: Please use the [JCIP annotations](http://jcip.net/annotations/doc/index.html) to document thread-safety. -3. Static imports: Please only use static imports for AssertJ, Mockito, and Awaitility. -4. `toString()`: only for debug in development. Use `toCqlLiteral()` is applicable for logs. + +Only use this for: + +* CCM tests that use `@BackendRequirement` restrictions at the method level + (ex: `BatchStatementIT`). +* tests where you *really* need to restart from a clean state for every method. + +##### Mixed + +It's also possible to use a `@ClassRule` for CCM / Simulacron, and a `@Rule` for the session rule. +In that case, you don't need to use a rule chain. From 2d55aba5609a73701b3e51a263c2831579abb687 Mon Sep 17 00:00:00 2001 From: janehe Date: Tue, 9 Dec 2025 21:47:56 -0800 Subject: [PATCH 13/16] add -Dccm.distribution --- CONTRIBUTING.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 4eb6347aba8..342a6a91c59 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -153,9 +153,10 @@ for i in {2..255}; do sudo ifconfig lo0 alias 127.0.0.$i up; done ``` mvn clean verify ``` - To target a specific Cassandra version: + To target a specific Cassandra version or distribution: ``` mvn verify -Dccm.version=3.11.0 + mvn verify -Dccm.distribution=dse -Dccm.version=6.8.0 ``` --- From 462e7e340b063cd6b94901fdc1796473195932cc Mon Sep 17 00:00:00 2001 From: janehe Date: Tue, 9 Dec 2025 22:00:35 -0800 Subject: [PATCH 14/16] revert formatting that IntelliJ did --- CONTRIBUTING.md | 61 +++++++++++++++++++++++-------------------------- 1 file changed, 28 insertions(+), 33 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 342a6a91c59..c86e2f5a97c 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -224,12 +224,12 @@ Logs are intended for two personae: * Ops who manage the application in production. * Developers (maybe you) who debug a particular issue. -#### Log Levels +#### Log levels * `ERROR`: something that renders the driver -- or a part of it -- completely unusable. An action is required to fix it: bouncing the client, applying a patch, etc. * `WARN`: something that the driver can recover from automatically, but indicates a configuration or - programming error that should be addressed. For example: the driver connected successfully, but + programming error that should be addressed. For example: the driver connected successfully, but one of the contact points in the configuration was malformed; the same prepared statement is being prepared multiple time by the application code. * `INFO`: something that is part of the normal operation of the driver, but might be useful to know @@ -241,23 +241,21 @@ request with). This is annoying for ops because they see a lot of stack traces t actual action on their part, because they're already handled by application code. Similarly, do not log stack traces for non-critical errors. If you still want the option to get the -trace for debugging, see the `Loggers.warnWithException` utility. +trace for debugging, see the `Loggers.warnWithException` utility. The last 2 levels are for developers, to help follow what the driver is doing from a "black box" perspective (think about debugging an issue remotely, and all you have are the logs). * `TRACE`: anything that happens **for every user request**. Not only request handling, but all related components (e.g. timestamp generators, policies, etc). -* `DEBUG`: everything else. For example, node state changes, control connection activity, etc. +* `DEBUG`: everything else. For example, node state changes, control connection activity, etc. Note that `DEBUG` and `TRACE` can coexist within the same component, for example the LBP initializing is a one-time event, but returning a query plan is a per-request event. - **Log prefix** shows origin, e.g.: `[s0|90232530|0]` (session name | hash code of the CqlRequestHandler instance | number of request attempts) - Tests run with the configuration defined in `src/test/resources/logback-test.xml`. The default level for driver classes is `WARN`, but you can override it with a system property: `-DdriverLevel=DEBUG`. A nice setup is to use `DEBUG` when you run from your IDE, and keep the default for the command @@ -266,8 +264,6 @@ line. When you add or review new code, take a moment to run the tests in `DEBUG` mode and check if the output looks good. - - ### Don't abuse the stream API The `java.util.stream` API is often used (abused?) as a "functional API for collections": @@ -279,12 +275,12 @@ List sizes = words.stream().map(String::length).collect(Collectors.toLi The perceived advantages of this approach over traditional for-loops are debatable: * readability: this is highly subjective. But consider the following: - * everyone can read for-loops, whether they are familiar with the Stream API or not. The opposite - is not true. - * the stream API does not spell out all the details: what kind of list does `Collectors.toList()` - return? Is it pre-sized? Mutable? Thread-safe? - * the stream API looks pretty on simple examples, but things can get ugly fast. Try rewriting - `NetworkTopologyReplicationStrategy` with streams. + * everyone can read for-loops, whether they are familiar with the Stream API or not. The opposite + is not true. + * the stream API does not spell out all the details: what kind of list does `Collectors.toList()` + return? Is it pre-sized? Mutable? Thread-safe? + * the stream API looks pretty on simple examples, but things can get ugly fast. Try rewriting + `NetworkTopologyReplicationStrategy` with streams. * concision: this is irrelevant. When we look at code we care about maintainability, not how many keystrokes the author saved. The for-loop version of the above example is just 5 lines long, and your brain doesn't take longer to parse it. @@ -296,14 +292,14 @@ simple. Only use `toString()` for debug logs or exception messages, and always assume that its format is unspecified and can change at any time. - + If you need a specific string representation for a class, make it a dedicated method with a documented format, for example `toCqlLiteral`. Otherwise it's too easy to lose track of the intended usage and break things: for example, someone modifies your `toString()` method to make their logs prettier, but unintentionally breaks the script export feature that expected it to produce CQL literals. - -`toString()` can delegate to `toCqlLiteral()` if that is appropriate for logs. + +`toString()` can delegate to `toCqlLiteral()` if that is appropriate for logs. ### Concurrency annotations @@ -325,12 +321,11 @@ Make sure you import the types from `net.jcip`, there are homonyms in the classp We use the [Spotbugs annotations](https://spotbugs.github.io) to document nullability of parameters, method return types and class members. -Please annotate any new class or interface with the appropriate annotations: `@NonNull`, `@Nullable`. Make sure you import +Please annotate any new class or interface with the appropriate annotations: `@NonNull`, `@Nullable`. Make sure you import the types from `edu.umd.cs.findbugs.annotations`, there are homonyms in the classpath. -## Coding Guidelines for Tests -## Coding style -- test code +## Coding Guidelines for Tests Static imports are permitted in a couple of places: * All AssertJ methods, e.g.: @@ -349,10 +344,10 @@ Static imports are permitted in a couple of places: ``` Test methods names use lower snake case, generally start with `should`, and clearly indicate the -purpose of the test, for example: `should_fail_if_key_already_exists`. If you have trouble coming +purpose of the test, for example: `should_fail_if_key_already_exists`. If you have trouble coming up with a simple name, it might be a sign that your test does too much, and should be split. -We use AssertJ (`assertThat`) for assertions. Don't use JUnit assertions (`assertEquals`, +We use AssertJ (`assertThat`) for assertions. Don't use JUnit assertions (`assertEquals`, `assertNull`, etc). Don't try to generify at all cost: a bit of duplication is acceptable, if that helps keep the tests @@ -377,17 +372,17 @@ process, which can be either one of: * [Simulacron](https://github.com/datastax/simulacron): simulates Cassandra nodes on loopback addresses; your test must "prime" data, i.e. tell the nodes what results to return for pre-determined queries. - - For an example of a Simulacron-based test, see `NodeTargetingIT`. + + For an example of a Simulacron-based test, see `NodeTargetingIT`. * [CCM](https://github.com/pcmanus/ccm): launches actual Cassandra nodes locally. The `ccm` executable must be in the path. - - You can pass a `-Dccm.version` system property to the build to target a particular Cassandra - version (it defaults to 3.11.0). `-Dccm.directory` allows you to point to a local installation - -- this can be a checkout of the Cassandra codebase, as long as it's built. See `CcmBridge` in - the driver codebase for more details. - - For an example of a CCM-based test, see `PlainTextAuthProviderIT`. + + You can pass a `-Dccm.version` system property to the build to target a particular Cassandra + version (it defaults to 3.11.0). `-Dccm.directory` allows you to point to a local installation + -- this can be a checkout of the Cassandra codebase, as long as it's built. See `CcmBridge` in + the driver codebase for more details. + + For an example of a CCM-based test, see `PlainTextAuthProviderIT`. #### Categories @@ -423,7 +418,7 @@ To make an integration test serial, do not annotate it with `@Category`. The CCM `CustomCcmRule`. For an example, see `DefaultLoadBalancingPolicyIT`. - + Note: if multiple serial tests have a common "base" class, do not pull up `CustomCcmRule`, each child class must have its own instance. Otherwise they share the same CCM instance, and the first one destroys it on teardown. See `TokenITBase` for how to organize code in those cases. @@ -436,7 +431,7 @@ typically system properties that need to be set before initialization. The build runs them one by one, *each in its own JVM fork*, after the serial tests. To isolate an integration test, annotate it with `@Category(IsolatedTests.class)`. The CCM rule -**must** be `CustomCcmRule`. +**must** be `CustomCcmRule`. For an example, see `HeapCompressionIT`. From 06f58eea06997d6c4fff4425116681adb7eb5cd3 Mon Sep 17 00:00:00 2001 From: janehe Date: Tue, 9 Dec 2025 22:04:18 -0800 Subject: [PATCH 15/16] more formatting --- CONTRIBUTING.md | 60 +++++++++++++++++++++++++++++++------------------ 1 file changed, 38 insertions(+), 22 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index c86e2f5a97c..8813f5d9598 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -140,7 +140,7 @@ mvn test #### Integration Tests 1. Install Cassandra Cluster Manager (CCM) following its [README](https://github.com/apache/cassandra-ccm). -2. On macOS, enable loopback aliases: +2. On MacOS only, enable loopback aliases: ```shell for i in {2..255}; do sudo ifconfig lo0 alias 127.0.0.$i up; done @@ -165,11 +165,16 @@ for i in {2..255}; do sudo ifconfig lo0 alias 127.0.0.$i up; done ### General -Do not use static imports. They make things harder to understand when you look at the code someplace where you don't have IDE support, like Github's code view. +Do not use static imports. They make things harder to understand when you look at the code +someplace where you don't have IDE support, like Github's code view. -Avoid abbreviations in class and variable names. A good rule of thumb is that you should only use them if you would also do so verbally, for example "id" and "config" are probably reasonable. Single-letter variables are permissible if the variable scope is only a few lines, or for commonly understood cases (like i for a loop index). +Avoid abbreviations in class and variable names. A good rule of thumb is that you should only use +them if you would also do so verbally, for example "id" and "config" are probably reasonable. +Single-letter variables are permissible if the variable scope is only a few lines, or for commonly +understood cases (like `i` for a loop index). -Keep source files short. Short files are easy to understand and test. The average should probably be around 200-300 lines. +Keep source files short. Short files are easy to understand and test. The average should probably +be around 200-300 lines. ### Code Formatting and License Headers @@ -185,30 +190,41 @@ Keep source files short. Short files are easy to understand and test. The averag ``` ### Javadoc -All types in "API" packages must be documented. For "internal" packages, documentation is optional, but in no way discouraged: it's generally a good idea to have a class-level comment that explains where the component fits in the architecture, and anything else that you feel is important. -You don't need to document every parameter or return type, or even every method. Don't document something if it is completely obvious, we don't want to end up with this: +All types in "API" packages must be documented. For "internal" packages, documentation is optional, +but in no way discouraged: it's generally a good idea to have a class-level comment that explains +where the component fits in the architecture, and anything else that you feel is important. + +You don't need to document every parameter or return type, or even every method. Don't document +something if it is completely obvious, we don't want to end up with this: + ```java /** -* Returns the name. -* -* @return the name - */ - String getName(); + * Returns the name. + * + * @return the name + */ +String getName(); ``` -On the other hand, there is often something useful to say about a method, so most should have at least a one-line comment. Use common sense. +On the other hand, there is often something useful to say about a method, so most should have at +least a one-line comment. Use common sense. -Driver users coding in their IDE should find the right documentation at the right time. Try to think of how they will come into contact with the class. For example, if a type is constructed with a builder, each builder method should probably explain what the default is when you don't call it. +Driver users coding in their IDE should find the right documentation at the right time. Try to +think of how they will come into contact with the class. For example, if a type is constructed with +a builder, each builder method should probably explain what the default is when you don't call it. -Avoid using too many links, they can make comments harder to read, especially in the IDE. Link to a type the first time it's mentioned, then use a text description ("this registry"...) or an @code block. Don't link to a class in its own documentation. Don't link to types that appear right below in the documented item's signature. +Avoid using too many links, they can make comments harder to read, especially in the IDE. Link to a +type the first time it's mentioned, then use a text description ("this registry"...) or an `@code` +block. Don't link to a class in its own documentation. Don't link to types that appear right below +in the documented item's signature. ```java /** * @return this {@link Builder} <-- completely unnecessary - */ - Builder withLimit(int limit) { +*/ +Builder withLimit(int limit) { ``` ### Logging @@ -275,12 +291,12 @@ List sizes = words.stream().map(String::length).collect(Collectors.toLi The perceived advantages of this approach over traditional for-loops are debatable: * readability: this is highly subjective. But consider the following: - * everyone can read for-loops, whether they are familiar with the Stream API or not. The opposite - is not true. - * the stream API does not spell out all the details: what kind of list does `Collectors.toList()` - return? Is it pre-sized? Mutable? Thread-safe? - * the stream API looks pretty on simple examples, but things can get ugly fast. Try rewriting - `NetworkTopologyReplicationStrategy` with streams. + * everyone can read for-loops, whether they are familiar with the Stream API or not. The opposite + is not true. + * the stream API does not spell out all the details: what kind of list does `Collectors.toList()` + return? Is it pre-sized? Mutable? Thread-safe? + * the stream API looks pretty on simple examples, but things can get ugly fast. Try rewriting + `NetworkTopologyReplicationStrategy` with streams. * concision: this is irrelevant. When we look at code we care about maintainability, not how many keystrokes the author saved. The for-loop version of the above example is just 5 lines long, and your brain doesn't take longer to parse it. From ff00fff4ea48f542f2d2f0e0ea568d816501b062 Mon Sep 17 00:00:00 2001 From: janehe Date: Tue, 9 Dec 2025 22:10:16 -0800 Subject: [PATCH 16/16] add communication --- CONTRIBUTING.md | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 8813f5d9598..eaedaa27d49 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -44,8 +44,6 @@ Thank you for your interest in contributing! - [Unit tests](#unit-tests) - [Integration tests](#integration-tests) ---- - ## Ways to Contribute There are many ways to contribute, including: @@ -58,9 +56,12 @@ There are many ways to contribute, including: - **Support & Triage**: Help evaluate reported issues or contribute to discussions. - **Verify Releases**: Verify the release artifacts work correctly in your environment, when a release is proposed in the mailing list. -All contributions go through JIRA + GitHub + mailing list, as described below. +### Communication +1. **Mailing List**: https://groups.google.com/a/lists.datastax.com/g/java-driver-user +2. **JIRA**: https://issues.apache.org/jira/projects/CASSJAVA +3. **GitHub Repository**: https://github.com/apache/cassandra-java-driver +4. **Slack**: #cassandar-drivers channel, if you are in the Apache Software Foundation [Slack](https://infra.apache.org/slack.html) ---- ## Contribution Process @@ -227,7 +228,7 @@ in the documented item's signature. Builder withLimit(int limit) { ``` -### Logging +### Logs We use SLF4J; loggers are declared like this: