absurdfarce commented on code in PR #1738:
URL: 
https://github.com/apache/cassandra-java-driver/pull/1738#discussion_r1382265876


##########
test-infra/src/main/java/com/datastax/oss/driver/api/testinfra/ccm/CcmBridge.java:
##########
@@ -430,17 +433,15 @@ private static File createTempStore(String storePath) {
    *
    * @return major version of current JVM
    */
-  private static int getCurrentJvmMajorVersion() {
+  @VisibleForTesting
+  static int getCurrentJvmMajorVersion() {
     String version = System.getProperty("java.version");
-    if (version.startsWith("1.")) {
-      version = version.substring(2, 3);
-    } else {
-      int dot = version.indexOf(".");
-      if (dot != -1) {
-        version = version.substring(0, dot);
-      }
+    Pattern pattern = Pattern.compile("^(1\\.)?(?<major>[0-9]+)\\..*$");

Review Comment:
   Nit: I don't think the "$" is doing much for you here



##########
test-infra/src/main/java/com/datastax/oss/driver/api/testinfra/ccm/CcmBridge.java:
##########
@@ -430,17 +433,15 @@ private static File createTempStore(String storePath) {
    *
    * @return major version of current JVM
    */
-  private static int getCurrentJvmMajorVersion() {
+  @VisibleForTesting
+  static int getCurrentJvmMajorVersion() {

Review Comment:
   Nit: I'm kinda wondering if instead of making this visible for testing we 
don't just change things so that all the logic lives in a 
getCurrentJvmMajorVersion() impl which takes a version string as an arg:
   
   ```java
   private static int getCurrentJvmMajorVersion() {
      return getJvmMajorVersion(System.getProperty("java.version"));
   }
   
   static int getJvmMajorVersion(String versionSysprop) {
      // Same logic in here (without the sysprop access)
   }
   ```
   
   Would probably simplify your tests quite a bit.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org

Reply via email to