Build: do not load integ test class if --skip-integ-tests-in-disguise is specified in NamingConventionsCheck

Projects that don't depend on elasticsearch-test fail otherwise because org.elasticsearch.test.EsIntegTestCase (default integ test class) is not in the classpath. They should provide their onw integ test base class, but having integration tests should not be mandatory. One can simply set skipIntegTestsInDisguise to true to prevent loading of integ test class.

buildSrc/src/main/groovy/org/elasticsearch/gradle/plugin/PluginBuildPlugin.groovy

@@ -19,7 +19,6 @@
 package org.elasticsearch.gradle.plugin
 
 import nebula.plugin.publishing.maven.MavenBasePublishPlugin
-import nebula.plugin.publishing.maven.MavenManifestPlugin
 import nebula.plugin.publishing.maven.MavenScmPlugin
 import org.elasticsearch.gradle.BuildPlugin
 import org.elasticsearch.gradle.test.RestIntegTestTask
@@ -27,7 +26,6 @@ import org.elasticsearch.gradle.test.RunTask
 import org.gradle.api.Project
 import org.gradle.api.tasks.SourceSet
 import org.gradle.api.tasks.bundling.Zip
-
 /**
  * Encapsulates build configuration for an Elasticsearch plugin.
  */
@@ -56,7 +54,7 @@ public class PluginBuildPlugin extends BuildPlugin {
             }
 
             project.namingConventions {
-                // Plugins decalare extensions of ESIntegTestCase as "Tests" instead of IT.
+                // Plugins declare integration tests as "Tests" instead of IT.
                 skipIntegTestInDisguise = true
             }
         }

buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/NamingConventionsTask.groovy

@@ -26,7 +26,6 @@ import org.gradle.api.file.FileCollection
 import org.gradle.api.tasks.Input
 import org.gradle.api.tasks.InputFiles
 import org.gradle.api.tasks.OutputFile
-
 /**
  * Runs NamingConventionsCheck on a classpath/directory combo to verify that
  * tests are named according to our conventions so they'll be picked up by
@@ -90,9 +89,11 @@ public class NamingConventionsTask extends LoggedExec {
             doFirst {
                 args('-Djna.nosys=true')
                 args('-cp', (classpath + extraClasspath).asPath, 'org.elasticsearch.test.NamingConventionsCheck')
-                args(testClass, integTestClass)
+                args('--test-class', testClass)
                 if (skipIntegTestInDisguise) {
                     args('--skip-integ-tests-in-disguise')
+                } else {
+                    args('--integ-test-class', integTestClass)
                 }
                 /*
                  * The test framework has classes that fail the checks to validate that the checks fail properly.

buildSrc/src/main/java/org/elasticsearch/test/NamingConventionsCheck.java

@@ -44,31 +44,36 @@ import java.util.Set;
  */
 public class NamingConventionsCheck {
     public static void main(String[] args) throws IOException {
-        int i = 0;
-        NamingConventionsCheck check = new NamingConventionsCheck(
-                loadClassWithoutInitializing(args[i++]),
-                loadClassWithoutInitializing(args[i++]));
+        Class<?> testClass = null;
+        Class<?> integTestClass = null;
+        Path rootPath = null;
         boolean skipIntegTestsInDisguise = false;
         boolean selfTest = false;
-        while (true) {
-            switch (args[i]) {
+        for (int i = 0; i < args.length; i++) {
+            String arg = args[i];
+            switch (arg) {
+                case "--test-class":
+                    testClass = loadClassWithoutInitializing(args[++i]);
+                    break;
+                case "--integ-test-class":
+                    integTestClass = loadClassWithoutInitializing(args[++i]);
+                    break;
                 case "--skip-integ-tests-in-disguise":
                     skipIntegTestsInDisguise = true;
-                i++;
-                continue;
+                    break;
                 case "--self-test":
                     selfTest = true;
-                i++;
-                continue;
+                    break;
                 case "--":
-                i++;
+                    rootPath = Paths.get(args[++i]);
                     break;
                 default:
-                fail("Expected -- before a path.");
+                    fail("unsupported argument '" + arg + "'");
             }
-            break;
         }
-        check.check(Paths.get(args[i]));
+
+        NamingConventionsCheck check = new NamingConventionsCheck(testClass, integTestClass);
+        check.check(rootPath, skipIntegTestsInDisguise);
 
         if (selfTest) {
             assertViolation("WrongName", check.missingSuffix);
@@ -87,9 +92,9 @@ public class NamingConventionsCheck {
         assertNoViolations("Found inner classes that are tests, which are excluded from the test runner", check.innerClasses);
         assertNoViolations("Pure Unit-Test found must subclass [" + check.testClass.getSimpleName() + "]", check.pureUnitTest);
         assertNoViolations("Classes ending with [Tests] must subclass [" + check.testClass.getSimpleName() + "]", check.notImplementing);
-        if (!skipIntegTestsInDisguise) {
-            assertNoViolations("Subclasses of ESIntegTestCase should end with IT as they are integration tests",
-                    check.integTestsInDisguise);
+        if (skipIntegTestsInDisguise == false) {
+            assertNoViolations("Subclasses of " + check.integTestClass.getSimpleName() +
+                    " should end with IT as they are integration tests", check.integTestsInDisguise);
         }
     }
 
@@ -108,7 +113,7 @@ public class NamingConventionsCheck {
         this.integTestClass = integTestClass;
     }
 
-    public void check(Path rootPath) throws IOException {
+    public void check(Path rootPath, boolean skipTestsInDisguised) throws IOException {
         Files.walkFileTree(rootPath, new FileVisitor<Path>() {
             /**
              * The package name of the directory we are currently visiting. Kept as a string rather than something fancy because we load
@@ -143,7 +148,7 @@ public class NamingConventionsCheck {
                     String className = filename.substring(0, filename.length() - ".class".length());
                     Class<?> clazz = loadClassWithoutInitializing(packageName + className);
                     if (clazz.getName().endsWith("Tests")) {
-                        if (integTestClass.isAssignableFrom(clazz)) {
+                        if (skipTestsInDisguised == false && integTestClass.isAssignableFrom(clazz)) {
                             integTestsInDisguise.add(clazz);
                         }
                         if (Modifier.isAbstract(clazz.getModifiers()) || Modifier.isInterface(clazz.getModifiers())) {

client-sniffer/build.gradle

@@ -61,8 +61,11 @@ forbiddenApisTest {
 
 //JarHell is part of es core, which we don't want to pull in
 jarHell.enabled=false
-//NamingConventionCheck is part of test-framework, which we don't want to pull in as it depends on es core
-namingConventions.enabled=false
+
+namingConventions {
+  //we don't have integration tests
+  skipIntegTestInDisguise = true
+}
 
 dependencyLicenses {
   dependencies = project.configurations.runtime.fileCollection {

client/build.gradle

@@ -59,8 +59,11 @@ forbiddenApisTest {
 
 //JarHell is part of es core, which we don't want to pull in
 jarHell.enabled=false
-//NamingConventionCheck is part of test-framework, which we don't want to pull in as it depends on es core
-namingConventions.enabled=false
+
+namingConventions {
+  //we don't have integration tests
+  skipIntegTestInDisguise = true
+}
 
 thirdPartyAudit.excludes = [
   //commons-logging optional dependencies