Only check Java tests, PR feedback

- Name checker was ran for Groovy tests that don't adhere to the same
  convections causing the check to fail
- implement PR feedback

Author: alpar-t

buildSrc/src/main/groovy/com/carrotsearch/gradle/junit4/RandomizedTestingPlugin.groovy

@@ -45,7 +45,7 @@ class RandomizedTestingPlugin implements Plugin<Project> {
     }
 
     static void replaceTestTask(Project project) {
-        def tasks = project.tasks
+        TaskContainer tasks = project.tasks
         Test oldTestTask = tasks.findByPath('test')
         if (oldTestTask == null) {
             // no test task, ok, user will use testing task on their own

buildSrc/src/main/groovy/com/carrotsearch/gradle/junit4/RandomizedTestingTask.groovy

@@ -98,7 +98,6 @@ class RandomizedTestingTask extends DefaultTask {
     }
 
     void basedOn(RandomizedTestingTask other) {
-        configure(BuildPlugin.commonTestConfig(project))
         classpath = other.classpath;
         testClassesDirs = other.testClassesDirs;
         dependsOn = other.dependsOn
@@ -224,7 +223,7 @@ class RandomizedTestingTask extends DefaultTask {
 
         DefaultLogger listener = null
         ByteArrayOutputStream antLoggingBuffer = null
-        if (!logger.isInfoEnabled()) {
+        if (false == logger.isInfoEnabled()) {
             // in info logging, ant already outputs info level, so we see everything
             // but on errors or when debugging, we want to see info level messages
             // because junit4 emits jvm output with ant logging

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

@@ -67,7 +67,7 @@ public class NamingConventionsTask extends LoggedExec {
 
     NamingConventionsTask() {
         // Extra classpath contains the actual test
-        if (!project.configurations.names.contains('namingConventions')) {
+        if (false == project.configurations.names.contains('namingConventions')) {
             project.configurations.create('namingConventions')
             Dependency buildToolsDep = project.dependencies.add('namingConventions',
                     "org.elasticsearch.gradle:build-tools:${VersionProperties.elasticsearch}")
@@ -81,17 +81,11 @@ public class NamingConventionsTask extends LoggedExec {
         description = "Tests that test classes aren't misnamed or misplaced"
         executable = new File(project.runtimeJavaHome, 'bin/java')
 
-        def dirsToUse = (checkForTestsInMain ?
-            project.sourceSets.main.output.classesDirs :
-            project.sourceSets.test.output.classesDirs)
-                .filter {it.exists()}
-                .collect {it.getAbsolutePath()}
-
-        if (!checkForTestsInMain) {
+        if (false == checkForTestsInMain) {
             /* This task is created by default for all subprojects with this
              * setting and there is no point in running it if the files don't
              * exist. */
-            onlyIf { dirsToUse.size() != 0 }
+            onlyIf { getJavaClassesDir() != null && getJavaClassesDir().exists() }
         }
 
         /*
@@ -124,10 +118,20 @@ public class NamingConventionsTask extends LoggedExec {
                 } else {
                     args('--')
                 }
-                args(dirsToUse.join(File.pathSeparator))
+                args(getJavaClassesDir().absolutePath)
             }
         }
         doLast { successMarker.setText("", 'UTF-8') }
     }
 
+    File getJavaClassesDir() {
+        FileCollection classesDirs = (checkForTestsInMain ?
+                project.sourceSets.main.output.classesDirs :
+                project.sourceSets.test.output.classesDirs)
+        if (classesDirs.isEmpty()) { return null }
+        // see https://docs.gradle.org/current/dsl/org.gradle.api.tasks.SourceSetOutput.html the deprecated property
+        // classesDir is the first in the FileCollection. There seems to be no other way to refer to the Java output dir
+        // We can't run the check against all classes as Groovy tests don't adhere to naming conventions.
+        return classesDirs[0]
+    }
 }

server/build.gradle

@@ -328,6 +328,7 @@ dependencyLicenses {
 if (isEclipse == false || project.path == ":server-tests") {
   task integTest(type: RandomizedTestingTask,group: JavaBasePlugin.VERIFICATION_GROUP) {
     description = 'Multi-node tests'
+    configure(BuildPlugin.commonTestConfig(project))
     basedOn tasks.test
     include '**/*IT.class'
   }

x-pack/plugin/ml/build.gradle

@@ -92,6 +92,7 @@ integTest.enabled = false
 
 // Instead we create a separate task to run the tests based on ESIntegTestCase
 task internalClusterTest(type: RandomizedTestingTask, group: JavaBasePlugin.VERIFICATION_GROUP) {
+    configure(BuildPlugin.commonTestConfig(project))
     description =  'Multi-node tests'
     basedOn tasks.test
     include '**/*IT.class'

x-pack/plugin/monitoring/build.gradle

@@ -57,6 +57,7 @@ integTest.enabled = false
 // Instead we create a separate task to run the tests based on ESIntegTestCase
 task internalClusterTest(type: RandomizedTestingTask, group: JavaBasePlugin.VERIFICATION_GROUP) {
     description = 'Multi-node tests'
+    configure(BuildPlugin.commonTestConfig(project))
     basedOn test
     include '**/*IT.class'
     systemProperty 'es.set.netty.runtime.available.processors', 'false'

x-pack/plugin/rollup/build.gradle

@@ -37,6 +37,7 @@ integTest.enabled = false
 // Instead we create a separate task to run the tests based on ESIntegTestCase
 task internalClusterTest(type: RandomizedTestingTask, group: JavaBasePlugin.VERIFICATION_GROUP) {
     description =  'Multi-node tests'
+    configure(BuildPlugin.commonTestConfig(project))
     basedOn tasks.test
     include '**/*IT.class'
     systemProperty 'es.set.netty.runtime.available.processors', 'false'

x-pack/plugin/upgrade/build.gradle

@@ -31,6 +31,7 @@ integTest.enabled = false
 // tests based on ESIntegTestCase
 task internalClusterTest(type: RandomizedTestingTask, group: JavaBasePlugin.VERIFICATION_GROUP) {
     description = 'Multi-node tests'
+    configure(BuildPlugin.commonTestConfig(project))
     basedOn tasks.test
     include '**/*IT.class'
     systemProperty 'es.set.netty.runtime.available.processors', 'false'