You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by vl...@apache.org on 2018/08/01 11:02:42 UTC

calcite git commit: [CALCITE-2428] Cassandra unit test fails to parse JDK version string (Andrei Sereda)

Repository: calcite
Updated Branches:
  refs/heads/master 8b0973687 -> a0983768d


[CALCITE-2428] Cassandra unit test fails to parse JDK version string (Andrei Sereda)

Previous version failed to parse correctly versions like: 9, 10-ea, 11-pre etc.
Change method TestUtil.getJavaMajorVersion() to use regex to detect major JDK version from
a string (usually 'java.version' system property). At some point migrate to
Runtime.version() (which is available 9+)

closes #770


Project: http://git-wip-us.apache.org/repos/asf/calcite/repo
Commit: http://git-wip-us.apache.org/repos/asf/calcite/commit/a0983768
Tree: http://git-wip-us.apache.org/repos/asf/calcite/tree/a0983768
Diff: http://git-wip-us.apache.org/repos/asf/calcite/diff/a0983768

Branch: refs/heads/master
Commit: a0983768d28e48ddd6ebd0488834f80ab54b85d7
Parents: 8b09736
Author: Andrei Sereda <an...@nospam.com>
Authored: Mon Jul 30 12:07:23 2018 -0400
Committer: Vladimir Sitnikov <si...@gmail.com>
Committed: Wed Aug 1 14:02:10 2018 +0300

----------------------------------------------------------------------
 .../calcite/test/CassandraAdapterTest.java      |  8 +-
 .../java/org/apache/calcite/util/TestUtil.java  | 44 +++++++---
 .../org/apache/calcite/util/TestUtilTest.java   | 92 ++++++++++++++++++++
 3 files changed, 130 insertions(+), 14 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/calcite/blob/a0983768/cassandra/src/test/java/org/apache/calcite/test/CassandraAdapterTest.java
----------------------------------------------------------------------
diff --git a/cassandra/src/test/java/org/apache/calcite/test/CassandraAdapterTest.java b/cassandra/src/test/java/org/apache/calcite/test/CassandraAdapterTest.java
index adc5d90..72caa80 100644
--- a/cassandra/src/test/java/org/apache/calcite/test/CassandraAdapterTest.java
+++ b/cassandra/src/test/java/org/apache/calcite/test/CassandraAdapterTest.java
@@ -16,6 +16,8 @@
  */
 package org.apache.calcite.test;
 
+import org.apache.calcite.util.Bug;
+import org.apache.calcite.util.TestUtil;
 import org.apache.calcite.util.Util;
 
 import org.apache.cassandra.config.DatabaseDescriptor;
@@ -69,7 +71,6 @@ public class CassandraAdapterTest {
    *
    * <p>As of this wiring Cassandra 4.x is not yet released and we're using 3.x
    * (which fails on JDK11). All cassandra tests will be skipped if running on JDK11.
-   * TODO: remove JDK check once current adapter supports Cassandra 4.x
    *
    * @see <a href="https://issues.apache.org/jira/browse/CASSANDRA-9608">CASSANDRA-9608</a>
    * @return {@code true} if test is compatible with current environment,
@@ -78,9 +79,8 @@ public class CassandraAdapterTest {
   private static boolean enabled() {
     final boolean enabled =
         Util.getBooleanProperty("calcite.test.cassandra", true);
-    final int major = Integer.parseInt(System.getProperty("java.version").split("\\.")[0]);
-    final boolean compatibleJdk = major != 11;
-
+    Bug.upgrade("remove JDK version check once current adapter supports Cassandra 4.x");
+    final boolean compatibleJdk = TestUtil.getJavaMajorVersion() != 11;
     return enabled && compatibleJdk;
   }
 

http://git-wip-us.apache.org/repos/asf/calcite/blob/a0983768/core/src/test/java/org/apache/calcite/util/TestUtil.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/calcite/util/TestUtil.java b/core/src/test/java/org/apache/calcite/util/TestUtil.java
index b530d3b..0bacb51 100644
--- a/core/src/test/java/org/apache/calcite/util/TestUtil.java
+++ b/core/src/test/java/org/apache/calcite/util/TestUtil.java
@@ -16,8 +16,12 @@
  */
 package org.apache.calcite.util;
 
+import com.google.common.annotations.VisibleForTesting;
+
 import org.junit.ComparisonFailure;
 
+import java.util.Objects;
+import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
 /**
@@ -194,19 +198,39 @@ public abstract class TestUtil {
         .replaceAll("\\]", "\\\\]");
   }
 
-  /** Returns the Java major version: 7 for JDK 1.7, 8 for JDK 8, 10 for
-   * JDK 10, etc. */
+  /**
+   * Returns the Java major version: 7 for JDK 1.7, 8 for JDK 8, 10 for
+   * JDK 10, etc. depending on current system property {@code java.version}.
+   */
   public static int getJavaMajorVersion() {
-    if (JAVA_VERSION.startsWith("1.7")) {
-      return 7;
-    } else if (JAVA_VERSION.startsWith("1.8")) {
-      return 8;
-    } else if (JAVA_VERSION.startsWith("1.9")) {
-      return 9;
-    } else {
-      return 10;
+    return majorVersionFromString(JAVA_VERSION);
+  }
+
+  /**
+   * Detects java major version given long format of full JDK version.
+   * See <a href="http://openjdk.java.net/jeps/223">JEP 223: New Version-String Scheme</a>.
+   *
+   * @param version current version as string usually from {@code java.version} property.
+   * @return major java version ({@code 8, 9, 10, 11} etc.)
+   */
+  @VisibleForTesting
+  static int majorVersionFromString(String version) {
+    Objects.requireNonNull(version, "version");
+
+    if (version.startsWith("1.")) {
+      // running on version <= 8 (expecting string of type: x.y.z*)
+      final String[] versions = version.split("\\.");
+      return Integer.parseInt(versions[1]);
+    }
+    // probably running on > 8 (just get first integer which is major version)
+    Matcher matcher = Pattern.compile("^\\d+").matcher(version);
+    if (!matcher.lookingAt()) {
+      throw new IllegalArgumentException("Can't parse (detect) JDK version from " + version);
     }
+
+    return Integer.parseInt(matcher.group());
   }
+
 }
 
 // End TestUtil.java

http://git-wip-us.apache.org/repos/asf/calcite/blob/a0983768/core/src/test/java/org/apache/calcite/util/TestUtilTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/calcite/util/TestUtilTest.java b/core/src/test/java/org/apache/calcite/util/TestUtilTest.java
new file mode 100644
index 0000000..0ccad50
--- /dev/null
+++ b/core/src/test/java/org/apache/calcite/util/TestUtilTest.java
@@ -0,0 +1,92 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/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 org.apache.calcite.util;
+
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+/**
+ * Tests for TestUtil
+ */
+public class TestUtilTest {
+
+  @Test
+  public void current() {
+    // shouldn't throw any exceptions (for current JDK)
+    assertTrue(TestUtil.getJavaMajorVersion() > 6);
+  }
+
+  @Test
+  public void majorVersionFromString() {
+    testJavaVersion(4, "1.4.2_03");
+    testJavaVersion(5, "1.5.0_16");
+    testJavaVersion(6, "1.6.0_22");
+    testJavaVersion(7, "1.7.0_65-b20");
+    testJavaVersion(8, "1.8.0_72-internal");
+    testJavaVersion(8, "1.8.0_151");
+    testJavaVersion(8, "1.8.0_141");
+    testJavaVersion(9, "1.9.0_20-b62");
+    testJavaVersion(9, "1.9.0-ea-b19");
+    testJavaVersion(9, "9");
+    testJavaVersion(9, "9.0");
+    testJavaVersion(9, "9.0.1");
+    testJavaVersion(9, "9-ea");
+    testJavaVersion(9, "9.0.1");
+    testJavaVersion(9, "9.1-ea");
+    testJavaVersion(9, "9.1.1-ea");
+    testJavaVersion(9, "9.1.1-ea+123");
+    testJavaVersion(10, "10");
+    testJavaVersion(10, "10+456");
+    testJavaVersion(10, "10-ea");
+    testJavaVersion(10, "10-ea42");
+    testJavaVersion(10, "10-ea+555");
+    testJavaVersion(10, "10-ea42+555");
+    testJavaVersion(10, "10.0");
+    testJavaVersion(10, "10.0.0");
+    testJavaVersion(10, "10.0.0.0.0");
+    testJavaVersion(10, "10.1.2.3.4.5.6.7.8");
+    testJavaVersion(10, "10.0.1");
+    testJavaVersion(10, "10.1.1-foo");
+    testJavaVersion(11, "11");
+    testJavaVersion(11, "11+111");
+    testJavaVersion(11, "11-ea");
+    testJavaVersion(11, "11.0");
+    testJavaVersion(12, "12.0");
+    testJavaVersion(20, "20.0");
+    testJavaVersion(42, "42");
+    testJavaVersion(100, "100");
+    testJavaVersion(100, "100.0");
+    testJavaVersion(1000, "1000");
+    testJavaVersion(2000, "2000");
+    testJavaVersion(205, "205.0");
+    testJavaVersion(2017, "2017");
+    testJavaVersion(2017, "2017.0");
+    testJavaVersion(2017, "2017.12");
+    testJavaVersion(2017, "2017.12-pre");
+    testJavaVersion(2017, "2017.12.31");
+  }
+
+  private void testJavaVersion(int expectedMajorVersion, String versionString) {
+    assertEquals(versionString, expectedMajorVersion,
+        TestUtil.majorVersionFromString(versionString));
+  }
+
+}
+
+// End TestUtilTest.java