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);