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 2019/11/19 08:15:42 UTC

[calcite] 07/11: [CALCITE-2457] Druid: JUnit4 -> JUnit5

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

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

commit 27055fbc674b22d5d2307c2488b31996c2113485
Author: Vladimir Sitnikov <si...@gmail.com>
AuthorDate: Tue Nov 12 21:46:31 2019 +0300

    [CALCITE-2457] Druid: JUnit4 -> JUnit5
    
    This commit introduces JUnit 5 test execution engine.
    It is used by default for all the modules (even JUnit4 tests are executed with JUnit5),
    and there's an option to top-out of junit4 dependency (junit4=false in gradle.properties)
---
 build.gradle.kts                                   | 83 ++++++++++++++++++++--
 .../calcite/config/CalciteSystemProperty.java      |  2 +-
 druid/build.gradle.kts                             | 24 +++----
 druid/gradle.properties                            |  1 +
 .../adapter/druid/DruidQueryFilterTest.java        | 17 ++---
 .../org/apache/calcite/test/DruidAdapter2IT.java   | 19 +++--
 .../org/apache/calcite/test/DruidAdapterIT.java    | 19 +++--
 .../java/org/apache/calcite/test/DruidChecker.java |  2 +-
 .../calcite/test/DruidDateRangeRulesTest.java      |  4 +-
 gradle.properties                                  |  2 +-
 10 files changed, 131 insertions(+), 42 deletions(-)

diff --git a/build.gradle.kts b/build.gradle.kts
index 0ad9e43..9fa93f7 100644
--- a/build.gradle.kts
+++ b/build.gradle.kts
@@ -23,6 +23,7 @@ import com.github.vlsi.gradle.git.FindGitAttributes
 import com.github.vlsi.gradle.git.dsl.gitignore
 import com.github.vlsi.gradle.properties.dsl.lastEditYear
 import com.github.vlsi.gradle.properties.dsl.props
+import com.github.vlsi.gradle.properties.dsl.stringProperty
 import com.github.vlsi.gradle.release.RepositoryType
 import de.thetaphi.forbiddenapis.gradle.CheckForbiddenApis
 import de.thetaphi.forbiddenapis.gradle.CheckForbiddenApisExtension
@@ -68,6 +69,8 @@ val includeTestTags by props("!org.apache.calcite.test.SlowTests")
 // By default use Java implementation to sign artifacts
 // When useGpgCmd=true, then gpg command line tool is used for signing
 val useGpgCmd by props()
+val slowSuiteLogThreshold = stringProperty("slowSuiteLogThreshold")?.toLong() ?: 0
+val slowTestLogThreshold = stringProperty("slowTestLogThreshold")?.toLong() ?: 2000
 
 ide {
     copyrightToAsf()
@@ -229,8 +232,16 @@ allprojects {
             testImplementation("org.junit.jupiter:junit-jupiter-params")
             testImplementation("org.hamcrest:hamcrest")
             testRuntimeOnly("org.junit.jupiter:junit-jupiter-engine")
-            testRuntimeOnly("org.junit.vintage:junit-vintage-engine")
-            testImplementation("junit:junit")
+            if (project.props.bool("junit4", default = true)) {
+                // Allow projects to opt-out of junit dependency, so they can be JUnit5-only
+                testImplementation("junit:junit")
+                testRuntimeOnly("org.junit.vintage:junit-vintage-engine")
+            } else {
+                // org.apache.calcite.test.CalciteAssert depends on org/junit/ComparisonFailure
+                // and it is commonly used. So we keep junit on the test runtime classpath.
+                // However, it still prevents JUnit4 types in autocomplete which is fine.
+                testRuntimeOnly("junit:junit")
+            }
         }
     }
 
@@ -481,13 +492,75 @@ allprojects {
                         includeTags.add(includeTestTags)
                     }
                 }
-                exclude("**/*Suite*")
-                jvmArgs("-Xmx1536m")
-                jvmArgs("-Djdk.net.URLClassPath.disableClassPathURLCheck=true")
                 testLogging {
                     exceptionFormat = TestExceptionFormat.FULL
                     showStandardStreams = true
                 }
+                exclude("**/*Suite*")
+                jvmArgs("-Xmx1536m")
+                jvmArgs("-Djdk.net.URLClassPath.disableClassPathURLCheck=true")
+                // Pass the property to tests
+                fun passProperty(name: String, default: String? = null) {
+                    val value = System.getProperty(name) ?: default
+                    value?.let { systemProperty(name, it) }
+                }
+                passProperty("java.awt.headless")
+                passProperty("junit.jupiter.execution.parallel.enabled", "true")
+                passProperty("junit.jupiter.execution.timeout.default", "5 m")
+                passProperty("user.language", "TR")
+                passProperty("user.country", "tr")
+                val props = System.getProperties()
+                for (e in props.propertyNames() as `java.util`.Enumeration<String>) {
+                    if (e.startsWith("calcite.") || e.startsWith("avatica.")) {
+                        passProperty(e)
+                    }
+                }
+                // https://github.com/junit-team/junit5/issues/2041
+                // Gradle does not print parameterized test names yet :(
+                // Hopefully it will be fixed in Gradle 6.1
+                fun String?.withDisplayName(displayName: String?, separator: String = ", "): String? = when {
+                    displayName == null -> this
+                    this == null -> displayName
+                    endsWith(displayName) -> this
+                    else -> "$this$separator$displayName"
+                }
+                fun printResult(descriptor: TestDescriptor, result: TestResult) {
+                    val test = descriptor as org.gradle.api.internal.tasks.testing.TestDescriptorInternal
+                    val classDisplayName = test.className.withDisplayName(test.classDisplayName)
+                    val testDisplayName = test.name.withDisplayName(test.displayName)
+                    val duration = "%5.1fsec".format((result.endTime - result.startTime) / 1000f)
+                    val displayName = classDisplayName.withDisplayName(testDisplayName, " > ")
+                    // Hide SUCCESS from output log, so FAILURE/SKIPPED are easier to spot
+                    val resultType = result.resultType
+                        .takeUnless { it == TestResult.ResultType.SUCCESS }
+                        ?.toString()
+                        ?: (if (result.skippedTestCount > 0 || result.testCount == 0L) "WARNING" else "       ")
+                    if (!descriptor.isComposite) {
+                        println("$resultType $duration, $displayName")
+                    } else {
+                        val completed = result.testCount.toString().padStart(4)
+                        val failed = result.failedTestCount.toString().padStart(3)
+                        val skipped = result.skippedTestCount.toString().padStart(3)
+                        println("$resultType $duration, $completed completed, $failed failed, $skipped skipped, $displayName")
+                    }
+                }
+                afterTest(KotlinClosure2<TestDescriptor, TestResult, Any>({ descriptor, result ->
+                    // There are lots of skipped tests, so it is not clear how to log them
+                    // without making build logs too verbose
+                    if (result.resultType == TestResult.ResultType.FAILURE ||
+                        result.endTime - result.startTime >= slowTestLogThreshold) {
+                        printResult(descriptor, result)
+                    }
+                }))
+                afterSuite(KotlinClosure2<TestDescriptor, TestResult, Any>({ descriptor, result ->
+                    if (descriptor.name.startsWith("Gradle Test Executor")) {
+                        return@KotlinClosure2
+                    }
+                    if (result.resultType == TestResult.ResultType.FAILURE ||
+                        result.endTime - result.startTime >= slowSuiteLogThreshold) {
+                        printResult(descriptor, result)
+                    }
+                }))
             }
             withType<SpotBugsTask>().configureEach {
                 group = LifecycleBasePlugin.VERIFICATION_GROUP
diff --git a/core/src/main/java/org/apache/calcite/config/CalciteSystemProperty.java b/core/src/main/java/org/apache/calcite/config/CalciteSystemProperty.java
index b701786..fe7a7a1 100644
--- a/core/src/main/java/org/apache/calcite/config/CalciteSystemProperty.java
+++ b/core/src/main/java/org/apache/calcite/config/CalciteSystemProperty.java
@@ -195,7 +195,7 @@ public final class CalciteSystemProperty<T> {
    * Whether to run Druid tests.
    */
   public static final CalciteSystemProperty<Boolean> TEST_DRUID =
-      booleanProperty("calcite.test.druid", true);
+      booleanProperty("calcite.test.druid", false);
 
   /**
    * Whether to run Cassandra tests.
diff --git a/druid/build.gradle.kts b/druid/build.gradle.kts
index 66141b4..9f87b19 100644
--- a/druid/build.gradle.kts
+++ b/druid/build.gradle.kts
@@ -17,19 +17,19 @@
  */
 
 dependencies {
-    api(platform(project(":bom")))
-
-    api("org.apache.calcite.avatica:avatica-core")
     api(project(":core"))
-    testImplementation(project(":core", "testClasses"))
     api(project(":linq4j"))
-    api("org.apache.commons:commons-lang3")
-    api("com.fasterxml.jackson.core:jackson-core")
-    api("com.fasterxml.jackson.core:jackson-databind")
-    api("com.google.code.findbugs:jsr305")
-    api("com.google.guava:guava")
-    api("joda-time:joda-time")
-    api("org.slf4j:slf4j-api")
-    testImplementation("org.slf4j:slf4j-log4j12")
+
+    implementation("com.fasterxml.jackson.core:jackson-core")
+    implementation("com.fasterxml.jackson.core:jackson-databind")
+    implementation("com.google.code.findbugs:jsr305")
+    implementation("com.google.guava:guava")
+    implementation("joda-time:joda-time")
+    implementation("org.apache.calcite.avatica:avatica-core")
+    implementation("org.apache.commons:commons-lang3")
+    implementation("org.slf4j:slf4j-api")
+
+    testImplementation(project(":core", "testClasses"))
     testImplementation("org.mockito:mockito-core")
+    testRuntimeOnly("org.slf4j:slf4j-log4j12")
 }
diff --git a/druid/gradle.properties b/druid/gradle.properties
index 0c3a310..68af2a9 100644
--- a/druid/gradle.properties
+++ b/druid/gradle.properties
@@ -17,3 +17,4 @@
 #
 description=Druid adapter for Calcite
 artifact.name=Calcite Druid
+junit4=false
diff --git a/druid/src/test/java/org/apache/calcite/adapter/druid/DruidQueryFilterTest.java b/druid/src/test/java/org/apache/calcite/adapter/druid/DruidQueryFilterTest.java
index b42821d..4ae617b 100644
--- a/druid/src/test/java/org/apache/calcite/adapter/druid/DruidQueryFilterTest.java
+++ b/druid/src/test/java/org/apache/calcite/adapter/druid/DruidQueryFilterTest.java
@@ -30,9 +30,8 @@ import com.fasterxml.jackson.core.JsonGenerator;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 
-import org.junit.Assert;
-import org.junit.Before;
-import org.junit.Test;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
 import org.mockito.Mockito;
 
 import java.io.IOException;
@@ -40,6 +39,8 @@ import java.io.StringWriter;
 import java.math.BigDecimal;
 import java.util.List;
 
+import static org.hamcrest.CoreMatchers.notNullValue;
+import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.core.Is.is;
 
 /**
@@ -48,7 +49,7 @@ import static org.hamcrest.core.Is.is;
 public class DruidQueryFilterTest {
 
   private DruidQuery druidQuery;
-  @Before
+  @BeforeEach
   public void testSetup() {
     druidQuery = Mockito.mock(DruidQuery.class);
     final CalciteConnectionConfig connectionConfigMock = Mockito
@@ -73,14 +74,14 @@ public class DruidQueryFilterTest {
         f.rexBuilder.makeCall(SqlStdOperatorTable.IN, listRexNodes);
     DruidJsonFilter returnValue = DruidJsonFilter
         .toDruidFilters(inRexNode, f.varcharRowType, druidQuery);
-    Assert.assertNotNull("Filter is null", returnValue);
+    assertThat("Filter is null", returnValue, notNullValue());
     JsonFactory jsonFactory = new JsonFactory();
     final StringWriter sw = new StringWriter();
     JsonGenerator jsonGenerator = jsonFactory.createGenerator(sw);
     returnValue.write(jsonGenerator);
     jsonGenerator.close();
 
-    Assert.assertThat(sw.toString(),
+    assertThat(sw.toString(),
         is("{\"type\":\"in\",\"dimension\":\"dimensionName\","
             + "\"values\":[\"1\",\"5\",\"value1\"]}"));
   }
@@ -98,13 +99,13 @@ public class DruidQueryFilterTest {
 
     DruidJsonFilter returnValue = DruidJsonFilter
         .toDruidFilters(betweenRexNode, f.varcharRowType, druidQuery);
-    Assert.assertNotNull("Filter is null", returnValue);
+    assertThat("Filter is null", returnValue, notNullValue());
     JsonFactory jsonFactory = new JsonFactory();
     final StringWriter sw = new StringWriter();
     JsonGenerator jsonGenerator = jsonFactory.createGenerator(sw);
     returnValue.write(jsonGenerator);
     jsonGenerator.close();
-    Assert.assertThat(sw.toString(),
+    assertThat(sw.toString(),
         is("{\"type\":\"bound\",\"dimension\":\"dimensionName\",\"lower\":\"lower-bound\","
             + "\"lowerStrict\":false,\"upper\":\"upper-bound\",\"upperStrict\":false,"
             + "\"ordering\":\"lexicographic\"}"));
diff --git a/druid/src/test/java/org/apache/calcite/test/DruidAdapter2IT.java b/druid/src/test/java/org/apache/calcite/test/DruidAdapter2IT.java
index 3e6ef8f..5f1c48d 100644
--- a/druid/src/test/java/org/apache/calcite/test/DruidAdapter2IT.java
+++ b/druid/src/test/java/org/apache/calcite/test/DruidAdapter2IT.java
@@ -30,7 +30,8 @@ import com.google.common.collect.ArrayListMultimap;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Multimap;
 
-import org.junit.Test;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
 
 import java.net.URL;
 import java.sql.DatabaseMetaData;
@@ -38,10 +39,11 @@ import java.sql.ResultSet;
 import java.sql.SQLException;
 
 import static org.hamcrest.CoreMatchers.is;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertSame;
-import static org.junit.Assert.assertThat;
-import static org.junit.Assert.assertTrue;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertSame;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assumptions.assumeTrue;
 
 /**
  * Tests for the {@code org.apache.calcite.adapter.druid} package.
@@ -77,10 +79,15 @@ public class DruidAdapter2IT {
   private static final String FOODMART_TABLE = "\"foodmart\"";
 
   /** Whether to run this test. */
-  protected boolean enabled() {
+  private static boolean enabled() {
     return CalciteSystemProperty.TEST_DRUID.value();
   }
 
+  @BeforeAll
+  public static void assumeDruidTestsEnabled() {
+    assumeTrue(enabled(), "Druid tests disabled. Add -Dcalcite.test.druid to enable it");
+  }
+
   /**
    * Creates a query against FOODMART with approximate parameters
    * */
diff --git a/druid/src/test/java/org/apache/calcite/test/DruidAdapterIT.java b/druid/src/test/java/org/apache/calcite/test/DruidAdapterIT.java
index c7b3fcc..7080c29 100644
--- a/druid/src/test/java/org/apache/calcite/test/DruidAdapterIT.java
+++ b/druid/src/test/java/org/apache/calcite/test/DruidAdapterIT.java
@@ -30,7 +30,8 @@ import com.google.common.collect.ArrayListMultimap;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Multimap;
 
-import org.junit.Test;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
 
 import java.net.URL;
 import java.sql.DatabaseMetaData;
@@ -38,10 +39,11 @@ import java.sql.ResultSet;
 import java.sql.SQLException;
 
 import static org.hamcrest.CoreMatchers.is;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertSame;
-import static org.junit.Assert.assertThat;
-import static org.junit.Assert.assertTrue;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertSame;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assumptions.assumeTrue;
 
 /**
  * Tests for the {@code org.apache.calcite.adapter.druid} package.
@@ -93,10 +95,15 @@ public class DruidAdapterIT {
   private static final String FOODMART_TABLE = "\"foodmart\"";
 
   /** Whether to run this test. */
-  protected boolean enabled() {
+  private static boolean enabled() {
     return CalciteSystemProperty.TEST_DRUID.value();
   }
 
+  @BeforeAll
+  public static void assumeDruidTestsEnabled() {
+    assumeTrue(enabled(), "Druid tests disabled. Add -Dcalcite.test.druid to enable it");
+  }
+
   /**
    * Creates a query against FOODMART with approximate parameters
    * */
diff --git a/druid/src/test/java/org/apache/calcite/test/DruidChecker.java b/druid/src/test/java/org/apache/calcite/test/DruidChecker.java
index 5fede3d..08b166d 100644
--- a/druid/src/test/java/org/apache/calcite/test/DruidChecker.java
+++ b/druid/src/test/java/org/apache/calcite/test/DruidChecker.java
@@ -23,7 +23,7 @@ import java.util.function.Consumer;
 
 import static org.hamcrest.CoreMatchers.containsString;
 import static org.hamcrest.CoreMatchers.is;
-import static org.junit.Assert.assertThat;
+import static org.hamcrest.MatcherAssert.assertThat;
 
 /**
  * A consumer that checks that a particular Druid query is generated to implement a query.
diff --git a/druid/src/test/java/org/apache/calcite/test/DruidDateRangeRulesTest.java b/druid/src/test/java/org/apache/calcite/test/DruidDateRangeRulesTest.java
index 6f3d3eb..c7f82d2 100644
--- a/druid/src/test/java/org/apache/calcite/test/DruidDateRangeRulesTest.java
+++ b/druid/src/test/java/org/apache/calcite/test/DruidDateRangeRulesTest.java
@@ -29,14 +29,14 @@ import com.google.common.collect.ImmutableList;
 
 import org.hamcrest.Matcher;
 import org.joda.time.Interval;
-import org.junit.Test;
+import org.junit.jupiter.api.Test;
 
 import java.util.Calendar;
 import java.util.List;
 
+import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.core.Is.is;
 import static org.hamcrest.core.IsNull.notNullValue;
-import static org.junit.Assert.assertThat;
 
 /** Unit tests for {@link DateRangeRules} algorithms. */
 public class DruidDateRangeRulesTest {
diff --git a/gradle.properties b/gradle.properties
index 4126703..451859a 100644
--- a/gradle.properties
+++ b/gradle.properties
@@ -102,7 +102,7 @@ joda-time.version=2.8.1
 json-path.version=2.4.0
 jsoup.version=1.11.3
 junit4.version=4.12
-junit5.version=5.5.1
+junit5.version=5.6.0-M1
 kafka-clients.version=2.1.1
 kerby.version=1.1.1
 log4j.version=2.11.0