IT-related changes pulled out of PR apache#12368

paul-rogers · Jun 24, 2022 · 76b5b47 · 76b5b47
1 parent 6ddb828
commit 76b5b47
Show file tree

Hide file tree

Showing 11 changed files with 133 additions and 61 deletions.
diff --git a/integration-tests/README.md b/integration-tests/README.md
diff --git a/integration-tests/script/copy_resources_template.sh b/integration-tests/script/copy_resources_template.sh
@@ -30,7 +30,7 @@ cp -R docker $SHARED_DIR/docker
 
 pushd ../
 rm -rf distribution/target/apache-druid-$DRUID_VERSION-integration-test-bin
-mvn -DskipTests -T1C -Danimal.sniffer.skip=true -Dcheckstyle.skip=true -Ddruid.console.skip=true -Denforcer.skip=true -Dforbiddenapis.skip=true -Dmaven.javadoc.skip=true -Dpmd.skip=true -Dspotbugs.skip=true install -Pintegration-test
+mvn -P skip-static-checks,skip-tests -T1C -Danimal.sniffer.skip=true -Dcheckstyle.skip=true -Ddruid.console.skip=true -Denforcer.skip=true -Dforbiddenapis.skip=true -Dmaven.javadoc.skip=true -Dpmd.skip=true -Dspotbugs.skip=true install -Pintegration-test
 mv distribution/target/apache-druid-$DRUID_VERSION-integration-test-bin/lib $SHARED_DIR/docker/lib
 mv distribution/target/apache-druid-$DRUID_VERSION-integration-test-bin/extensions $SHARED_DIR/docker/extensions
 popd

diff --git a/integration-tests/src/main/java/org/apache/druid/cli/CliCustomNodeRole.java b/integration-tests/src/main/java/org/apache/druid/cli/CliCustomNodeRole.java
@@ -66,6 +66,7 @@ public class CliCustomNodeRole extends ServerRunnable
   public static final String SERVICE_NAME = "custom-node-role";
   public static final int PORT = 9301;
   public static final int TLS_PORT = 9501;
+  public static final NodeRole NODE_ROLE = new NodeRole(CliCustomNodeRole.SERVICE_NAME);
 
   public CliCustomNodeRole()
   {
@@ -75,7 +76,7 @@ public CliCustomNodeRole()
   @Override
   protected Set<NodeRole> getNodeRoles(Properties properties)
   {
-    return ImmutableSet.of(new NodeRole(CliCustomNodeRole.SERVICE_NAME));
+    return ImmutableSet.of(NODE_ROLE);
   }
 
   @Override

diff --git a/integration-tests/src/main/java/org/apache/druid/cli/CustomNodeRoleCommandCreator.java b/integration-tests/src/main/java/org/apache/druid/cli/CustomNodeRoleCommandCreator.java
@@ -23,6 +23,7 @@
 
 public class CustomNodeRoleCommandCreator implements CliCommandCreator
 {
+  @SuppressWarnings({ "rawtypes", "unchecked" })
   @Override
   public void addCommands(CliBuilder builder)
   {

diff --git a/integration-tests/src/main/java/org/apache/druid/testing/IntegrationTestingConfig.java b/integration-tests/src/main/java/org/apache/druid/testing/IntegrationTestingConfig.java
@@ -23,6 +23,19 @@
 import java.util.Map;
 
 /**
+ * Configuration for tests. Opinionated about the shape of the cluster:
+ * there is one or two Coordinators or Overlords, zero or one of
+ * everything else.
+ * <p>
+ * To work in Docker (and K8s) there are two methods per host:
+ * {@code get<service>Host()} which returns the host as seen from
+ * the test machine (meaning the proxy host), and
+ * {@code get<service>InternalHost()} which returns the name of
+ * the host as seen by itself and other services: the host published
+ * in ZK, which is the host known to the Docker/K8s overlay network.
+ * <p>
+ * The {@code get<service>Url()} methods return URLs relative to
+ * the test, using the proxy host for Docker and K8s.
  */
 public interface IntegrationTestingConfig
 {

diff --git a/integration-tests/src/main/java/org/apache/druid/testing/clients/SqlResourceTestClient.java b/integration-tests/src/main/java/org/apache/druid/testing/clients/SqlResourceTestClient.java
@@ -30,7 +30,6 @@
 
 public class SqlResourceTestClient extends AbstractQueryResourceTestClient<SqlQuery>
 {
-
   @Inject
   SqlResourceTestClient(
       ObjectMapper jsonMapper,

diff --git a/integration-tests/src/main/java/org/apache/druid/testing/utils/AbstractTestQueryHelper.java b/integration-tests/src/main/java/org/apache/druid/testing/utils/AbstractTestQueryHelper.java
@@ -41,7 +41,6 @@
 
 public abstract class AbstractTestQueryHelper<QueryResultType extends AbstractQueryWithResults>
 {
-
   public static final Logger LOG = new Logger(TestQueryHelper.class);
 
   protected final AbstractQueryResourceTestClient queryClient;
@@ -54,7 +53,7 @@ public abstract class AbstractTestQueryHelper<QueryResultType extends AbstractQu
   @Inject
   AbstractTestQueryHelper(
       ObjectMapper jsonMapper,
-      AbstractQueryResourceTestClient queryClient,
+      AbstractQueryResourceTestClient<?> queryClient,
       IntegrationTestingConfig config
   )
   {
@@ -70,7 +69,7 @@ public abstract class AbstractTestQueryHelper<QueryResultType extends AbstractQu
 
   AbstractTestQueryHelper(
       ObjectMapper jsonMapper,
-      AbstractQueryResourceTestClient queryClient,
+      AbstractQueryResourceTestClient<?> queryClient,
       String broker,
       String brokerTLS,
       String router,
@@ -103,9 +102,13 @@ public void testQueriesFromFile(String filePath) throws Exception
   public void testQueriesFromString(String str) throws Exception
   {
     testQueriesFromString(getQueryURL(broker), str);
-    testQueriesFromString(getQueryURL(brokerTLS), str);
+    if (!broker.equals(brokerTLS)) {
+      testQueriesFromString(getQueryURL(brokerTLS), str);
+    }
     testQueriesFromString(getQueryURL(router), str);
-    testQueriesFromString(getQueryURL(routerTLS), str);
+    if (!router.equals(routerTLS)) {
+      testQueriesFromString(getQueryURL(routerTLS), str);
+    }
   }
 
   public void testQueriesFromFile(String url, String filePath) throws Exception

diff --git a/integration-tests/src/main/java/org/apache/druid/testing/utils/DruidClusterAdminClient.java b/integration-tests/src/main/java/org/apache/druid/testing/utils/DruidClusterAdminClient.java
@@ -272,7 +272,7 @@ private void waitUntilInstanceReady(final String host)
           }
           catch (Throwable e) {
             //
-            // supress stack trace logging for some specific exceptions
+            // suppress stack trace logging for some specific exceptions
             // to reduce excessive stack trace messages when waiting druid nodes to start up
             //
             if (e.getCause() instanceof ChannelException) {

diff --git a/integration-tests/src/main/java/org/testng/DruidTestRunnerFactory.java b/integration-tests/src/main/java/org/testng/DruidTestRunnerFactory.java
@@ -19,7 +19,6 @@
 
 package /*CHECKSTYLE.OFF: PackageName*/org.testng/*CHECKSTYLE.ON: PackageName*/;
 
-import org.apache.druid.java.util.common.logger.Logger;
 import org.apache.druid.testing.utils.SuiteListener;
 import org.testng.internal.IConfiguration;
 import org.testng.internal.Systematiser;
@@ -34,7 +33,6 @@
  */
 public class DruidTestRunnerFactory implements ITestRunnerFactory
 {
-  private static final Logger LOG = new Logger(DruidTestRunnerFactory.class);
   private static final SuiteListener SUITE_LISTENER = new SuiteListener();
 
   @Override

diff --git a/...t/java/org/apache/druid/tests/indexer/ITS3AssumeRoleWithOverrideCredentialsIndexTest.java b/...t/java/org/apache/druid/tests/indexer/ITS3AssumeRoleWithOverrideCredentialsIndexTest.java
@@ -40,7 +40,7 @@
  *    -Ddruid.test.config.s3AssumeRoleWithExternalId or setting "s3_assume_role_with_external_id" in the config file.
  *    -Ddruid.test.config.s3AssumeRoleExternalId or setting "s3_assume_role_external_id" in the config file.
  *    -Ddruid.test.config.s3AssumeRoleWithoutExternalId or setting "s3_assume_role_without_external_id" in the config file.
- *     The credientials provided in OVERRIDE_S3_ACCESS_KEY and OVERRIDE_S3_SECRET_KEY must be able to assume these roles.
+ *     The credentials provided in OVERRIDE_S3_ACCESS_KEY and OVERRIDE_S3_SECRET_KEY must be able to assume these roles.
  *     These roles must also have access to the bucket and path for your data in #1.
  *     (s3AssumeRoleExternalId is the external id for s3AssumeRoleWithExternalId, while s3AssumeRoleWithoutExternalId
  *     should not have external id set)

diff --git a/pom.xml b/pom.xml
@@ -127,6 +127,13 @@
 
         <!-- Allow the handful of flaky tests with transient failures to pass. -->
         <surefire.rerunFailingTestsCount>3</surefire.rerunFailingTestsCount>
+
+        <!-- Using -DskipTests or -P skip-tests will skip both the unit tests and
+             the "new" integration tests. To skip just the unit tests (but run the 
+             new integration tests by setting the required profile), use -DskipUTs=true.
+             -->
+        <skipUTs>false</skipUTs>
+        <skipITs>false</skipITs>
     </properties>
 
     <modules>
@@ -1169,6 +1176,7 @@
                     <excludes>
                         <!-- Ignore initialization classes, these are tested by the integration tests. -->
                         <exclude>org/apache/druid/cli/Cli*</exclude>
+                        <exclude>org/apache/druid/cli/GuiceRunnable.class</exclude>
                         <exclude>org/apache/druid/cli/*JettyServerInitializer*</exclude>
                         <exclude>org/apache/druid/server/initialization/*Module*</exclude>
                         <exclude>org/apache/druid/server/initialization/jetty/*Module*</exclude>
@@ -1183,6 +1191,13 @@
                         <exclude>org/apache/druid/benchmark/**/*</exclude>  <!-- benchmarks -->
                         <exclude>org/apache/druid/**/*Benchmark*</exclude>  <!-- benchmarks -->
                         <exclude>org/apache/druid/testing/**/*</exclude>  <!-- integration-tests -->
+                        <!-- The ITs has test code sprinked through the module tree. Remove the following
+                             once the old version is retired. -->
+                        <exclude>org/apache/druid/server/coordination/ServerManagerForQueryErrorTest.class</exclude>
+                        <exclude>org/apache/druid/guice/SleepModule.class</exclude>
+                        <exclude>org/apache/druid/guice/CustomNodeRoleClientModule.class</exclude>
+                        <exclude>org/apache/druid/cli/CustomNodeRoleCommandCreator.class</exclude>
+                        <exclude>org/apache/druid/cli/QueryRetryTestCommandCreator.class</exclude>
 
                         <!-- Exceptions -->
                         <exclude>org/apache/druid/query/TruncatedResponseContextException.class</exclude>
@@ -1513,6 +1528,9 @@
                             <!--@TODO After fixing https://github.com/apache/druid/issues/4964 remove this parameter-->
                             -Ddruid.indexing.doubleStorage=double
                         </argLine>
+                        <!-- Skip the tests which Surefire runs. Surefire runs the unit tests,
+                             while its sister plugin, Failsafe, runs the "new" ITs. -->
+                        <skipTests>${skipUTs}</skipTests>
                         <trimStackTrace>false</trimStackTrace>
                         <!-- our tests are very verbose, let's keep the volume down -->
                         <redirectTestOutputToFile>true</redirectTestOutputToFile>
@@ -1829,14 +1847,16 @@
                                 <exclude>docker/service-supervisords/*.conf</exclude>
                                 <exclude>target/**</exclude>
                                 <exclude>licenses/**</exclude>
-                                <exclude>**/test/resources/**</exclude>
+                                <exclude>**/test/resources/**</exclude> <!-- test data for "old" ITs. -->
                                 <exclude>**/derby.log</exclude>
                                 <exclude>**/jvm.config</exclude>
                                 <exclude>**/*.avsc</exclude>
                                 <exclude>**/*.iml</exclude>
                                 <exclude>**/*.json</exclude>
                                 <exclude>**/*.parq</exclude>
                                 <exclude>**/*.parquet</exclude>
+                                <exclude>**/*.pmd</exclude> <!-- Artifact of maven-pmd-plugin -->
+                                <exclude>**/*.pmdruleset.xml</exclude> <!-- Artifact of maven-pmd-plugin -->
                                 <exclude>**/docker/schema-registry/*</exclude>
                                 <exclude>LICENSE</exclude>
                                 <exclude>LICENSE.BINARY</exclude>
@@ -1911,8 +1931,12 @@
         </profile>
         <profile>
           <id>skip-tests</id>
+          <activation>
+            <activeByDefault>false</activeByDefault>
+          </activation>
           <properties>
-            <skipTests>true</skipTests>
+            <skipUTs>true</skipUTs> <!-- Skip only UTs -->
+            <skipITs>true</skipITs> <!-- ITs are also behind a profile -->
             <jacoco.skip>true</jacoco.skip>
           </properties>
         </profile>