This is an automated email from the ASF dual-hosted git repository.

zabetak pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/calcite.git


The following commit(s) were added to refs/heads/main by this push:
     new f996bc9993 [CALCITE-5923] SqlOperatorTest using safeParameters are not 
using overridable fixture
f996bc9993 is described below

commit f996bc9993546019d0fd475b2daa99ac8b1b9259
Author: Runkang He <hrun...@gmail.com>
AuthorDate: Sun Aug 13 18:06:37 2023 +0800

    [CALCITE-5923] SqlOperatorTest using safeParameters are not using 
overridable fixture
    
    SqlOperatorTest cases using the safeParameters method to generate
    parameterized inputs are not using the overridable fixture from the
    subclasses. Due to this the customized behavior introduced by
    subclasses of SqlOperatorTest is not taken into account by these tests.
    
    This violates the design principle of the SqlOperatorTest class which
    explicitly states that subclasses are meant to override the method
    and basically decreases code coverage since subclasses will not have
    any effect on the affected tests.
    
    1. Remove static modifier from safeParameters to allow the use of
    overridable fixture() method.
    2. Add TestInstance.Lifecycle.PER_CLASS annotation to allow
    @MethodSource to be non-static (see junit-team/junit5#984).
    3. Adapt expected results for test cases casting string to timestamps
    based on changes from CALCITE-5678 in Avatica.
    4. Pickup avatica.version from project properties to run the appropriate
    tests while waiting for the Avatica upgrade.
    
    Close apache/calcite#3364
---
 build.gradle.kts                                   |  1 +
 .../org/apache/calcite/test/SqlOperatorTest.java   | 52 ++++++++++++++++------
 .../java/org/apache/calcite/util/TestUtil.java     |  2 +
 3 files changed, 42 insertions(+), 13 deletions(-)

diff --git a/build.gradle.kts b/build.gradle.kts
index 9d4a6e8af9..f026c48583 100644
--- a/build.gradle.kts
+++ b/build.gradle.kts
@@ -820,6 +820,7 @@ allprojects {
                 passProperty("user.language", "TR")
                 passProperty("user.country", "tr")
                 passProperty("user.timezone", "UTC")
+                passProperty("calcite.avatica.version", 
props.string("calcite.avatica.version"))
                 val props = System.getProperties()
                 for (e in props.propertyNames() as 
`java.util`.Enumeration<String>) {
                     if (e.startsWith("calcite.") || e.startsWith("avatica.")) {
diff --git a/testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java 
b/testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java
index 746c330720..fd11a31f05 100644
--- a/testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java
+++ b/testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java
@@ -83,6 +83,7 @@ import org.checkerframework.checker.nullness.qual.Nullable;
 import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Tag;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestInstance;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.Arguments;
 import org.junit.jupiter.params.provider.MethodSource;
@@ -197,6 +198,7 @@ import static java.nio.charset.StandardCharsets.UTF_8;
  * null arguments or null results.</li>
  * </ul>
  */
+@TestInstance(TestInstance.Lifecycle.PER_CLASS)
 @SuppressWarnings("MethodCanBeStatic")
 public class SqlOperatorTest {
   //~ Static fields/initializers ---------------------------------------------
@@ -441,8 +443,9 @@ public class SqlOperatorTest {
   }
 
   /** Generates parameters to test both regular and safe cast. */
-  static Stream<Arguments> safeParameters() {
-    SqlOperatorFixture f = SqlOperatorFixtureImpl.DEFAULT;
+  @SuppressWarnings("unused")
+  private Stream<Arguments> safeParameters() {
+    SqlOperatorFixture f = fixture();
     SqlOperatorFixture f2 =
         
SqlOperatorFixtures.safeCastWrapper(f.withLibrary(SqlLibrary.BIG_QUERY), 
"SAFE_CAST");
     SqlOperatorFixture f3 =
@@ -1238,8 +1241,6 @@ public class SqlOperatorTest {
 
     f.checkScalar("cast('1945-02-24 12:42:25' as TIMESTAMP)",
         "1945-02-24 12:42:25", "TIMESTAMP(0) NOT NULL");
-    f.checkScalar("cast('1945-2-2 12:2:5' as TIMESTAMP)",
-        "1945-02-02 12:02:05", "TIMESTAMP(0) NOT NULL");
     f.checkScalar("cast('  1945-02-24 12:42:25  ' as TIMESTAMP)",
         "1945-02-24 12:42:25", "TIMESTAMP(0) NOT NULL");
     f.checkScalar("cast('1945-02-24 12:42:25.34' as TIMESTAMP)",
@@ -1253,15 +1254,40 @@ public class SqlOperatorTest {
       f.checkScalar("cast('1945-02-24 12:42:25.34' as TIMESTAMP(2))",
           "1945-02-24 12:42:25.34", "TIMESTAMP(2) NOT NULL");
     }
+    // Remove the if condition and the else block once CALCITE-6053 is fixed
+    if (TestUtil.AVATICA_VERSION.startsWith("1.0.0-dev-main")) {
+      if (castType == CastType.CAST) {
+        f.checkFails("cast('1945-2-2 12:2:5' as TIMESTAMP)",
+            "Invalid DATE value, '1945-2-2 12:2:5'", true);
+        f.checkFails("cast('1241241' as TIMESTAMP)",
+            "Invalid DATE value, '1241241'", true);
+        f.checkFails("cast('1945-20-24 12:42:25.34' as TIMESTAMP)",
+            "Invalid DATE value, '1945-20-24 12:42:25.34'", true);
+        f.checkFails("cast('1945-01-24 25:42:25.34' as TIMESTAMP)",
+            "Value of HOUR field is out of range in '1945-01-24 25:42:25.34'", 
true);
+        f.checkFails("cast('1945-1-24 12:23:34.454' as TIMESTAMP)",
+            "Invalid DATE value, '1945-1-24 12:23:34.454'", true);
+      } else {
+        // test cases for 'SAFE_CAST' and 'TRY_CAST'
+        f.checkNull("cast('1945-2-2 12:2:5' as TIMESTAMP)");
+        f.checkNull("cast('1241241' as TIMESTAMP)");
+        f.checkNull("cast('1945-20-24 12:42:25.34' as TIMESTAMP)");
+        f.checkNull("cast('1945-01-24 25:42:25.34' as TIMESTAMP)");
+        f.checkNull("cast('1945-1-24 12:23:34.454' as TIMESTAMP)");
+      }
+    } else {
+      f.checkScalar("cast('1945-2-2 12:2:5' as TIMESTAMP)",
+          "1945-02-02 12:02:05", "TIMESTAMP(0) NOT NULL");
+      f.checkScalar("cast('1241241' as TIMESTAMP)",
+          "1241-01-01 00:00:00", "TIMESTAMP(0) NOT NULL");
+      f.checkScalar("cast('1945-20-24 12:42:25.34' as TIMESTAMP)",
+          "1946-08-26 12:42:25", "TIMESTAMP(0) NOT NULL");
+      f.checkScalar("cast('1945-01-24 25:42:25.34' as TIMESTAMP)",
+          "1945-01-25 01:42:25", "TIMESTAMP(0) NOT NULL");
+      f.checkScalar("cast('1945-1-24 12:23:34.454' as TIMESTAMP)",
+          "1945-01-24 12:23:34", "TIMESTAMP(0) NOT NULL");
+    }
     f.checkFails("cast('nottime' as TIMESTAMP)", BAD_DATETIME_MESSAGE, true);
-    f.checkScalar("cast('1241241' as TIMESTAMP)",
-        "1241-01-01 00:00:00", "TIMESTAMP(0) NOT NULL");
-    f.checkScalar("cast('1945-20-24 12:42:25.34' as TIMESTAMP)",
-        "1946-08-26 12:42:25", "TIMESTAMP(0) NOT NULL");
-    f.checkScalar("cast('1945-01-24 25:42:25.34' as TIMESTAMP)",
-        "1945-01-25 01:42:25", "TIMESTAMP(0) NOT NULL");
-    f.checkScalar("cast('1945-1-24 12:23:34.454' as TIMESTAMP)",
-        "1945-01-24 12:23:34", "TIMESTAMP(0) NOT NULL");
 
     // date <-> string
     f.checkCastToString("DATE '1945-02-24'", null, "1945-02-24", castType);
@@ -12947,7 +12973,7 @@ public class SqlOperatorTest {
   }
 
   @Test void testArgMin() {
-    final SqlOperatorFixture f0 = fixture().withTester(t -> TESTER);
+    final SqlOperatorFixture f0 = fixture();
     final String[] xValues = {"2", "3", "4", "4", "5", "7"};
 
     final Consumer<SqlOperatorFixture> consumer = f -> {
diff --git a/testkit/src/main/java/org/apache/calcite/util/TestUtil.java 
b/testkit/src/main/java/org/apache/calcite/util/TestUtil.java
index a661f8d4d6..6caf26d303 100644
--- a/testkit/src/main/java/org/apache/calcite/util/TestUtil.java
+++ b/testkit/src/main/java/org/apache/calcite/util/TestUtil.java
@@ -54,6 +54,8 @@ public abstract class TestUtil {
   private static final String JAVA_VERSION =
       System.getProperties().getProperty("java.version");
 
+  public static final String AVATICA_VERSION =
+      System.getProperty("calcite.avatica.version");
   private static final Supplier<Integer> GUAVA_MAJOR_VERSION =
       Suppliers.memoize(TestUtil::computeGuavaMajorVersion);
 

Reply via email to