Merge branch 'master' into snapshot_only_role

Author: albertzaharovits

build.gradle

@@ -159,8 +159,8 @@ task verifyVersions {
  * the enabled state of every bwc task. It should be set back to true
  * after the backport of the backcompat code is complete.
  */
-final boolean bwc_tests_enabled = true
-final String bwc_tests_disabled_issue = "" /* place a PR link here when committing bwc changes */
+final boolean bwc_tests_enabled = false
+final String bwc_tests_disabled_issue = "backporting https://github.com/elastic/elasticsearch/pull/37639" /* place a PR link here when committing bwc changes */
 if (bwc_tests_enabled == false) {
   if (bwc_tests_disabled_issue.isEmpty()) {
     throw new GradleException("bwc_tests_disabled_issue must be set when bwc_tests_enabled == false")

buildSrc/build.gradle

@@ -235,11 +235,6 @@ if (project != rootProject) {
     exclude '**/ForbiddenPatternsTask.java'
   }
 
-  namingConventions {
-    testClass = 'org.elasticsearch.gradle.test.GradleUnitTestCase'
-    integTestClass = 'org.elasticsearch.gradle.test.GradleIntegrationTestCase'
-  }
-
   testingConventions {
     naming.clear()
     naming {

buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy

@@ -904,6 +904,7 @@ class BuildPlugin implements Plugin<Project> {
         project.tasks.withType(RandomizedTestingTask) {task ->
             jvm "${project.runtimeJavaHome}/bin/java"
             parallelism System.getProperty('tests.jvms', project.rootProject.ext.defaultParallel)
+            ifNoTests 'fail'
             onNonEmptyWorkDirectory 'wipe'
             leaveTemporary true
             project.sourceSets.matching { it.name == "test" }.all { test ->

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

@@ -72,11 +72,6 @@ public class PluginBuildPlugin extends BuildPlugin {
             if (isModule == false || isXPackModule) {
                 addNoticeGeneration(project)
             }
-
-            project.namingConventions {
-                // Plugins declare integration tests as "Tests" instead of IT.
-                skipIntegTestInDisguise = true
-            }
         }
         project.testingConventions {
             naming.clear()

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

@@ -43,7 +43,6 @@ class PrecommitTasks {
         List<Task> precommitTasks = [
             configureCheckstyle(project),
             configureForbiddenApisCli(project),
-            configureNamingConventions(project),
             project.tasks.create('forbiddenPatterns', ForbiddenPatternsTask.class),
             project.tasks.create('licenseHeaders', LicenseHeadersTask.class),
             project.tasks.create('filepermissions', FilePermissionsTask.class),
@@ -230,15 +229,6 @@ class PrecommitTasks {
         return checkstyleTask
     }
 
-    private static Task configureNamingConventions(Project project) {
-        if (project.sourceSets.findByName("test")) {
-            Task namingConventionsTask = project.tasks.create('namingConventions', NamingConventionsTask)
-            namingConventionsTask.javaHome = project.compilerJavaHome
-            return namingConventionsTask
-        }
-        return null
-    }
-
     private static Task configureLoggerUsage(Project project) {
         project.configurations.create('loggerUsagePlugin')
         project.dependencies.add('loggerUsagePlugin',

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

@@ -27,6 +27,8 @@
 import org.gradle.api.file.FileTree;
 import org.gradle.api.tasks.Input;
 import org.gradle.api.tasks.OutputFile;
+import org.gradle.api.tasks.SourceSet;
+import org.gradle.api.tasks.SourceSetContainer;
 import org.gradle.api.tasks.TaskAction;
 import org.gradle.api.tasks.testing.Test;
 import org.gradle.api.tasks.util.PatternFilterable;
@@ -122,6 +124,23 @@ public void naming(Closure<TestingConventionRule> action) {
         naming.configure(action);
     }
 
+    @Input
+    public Set<String> getMainClassNamedLikeTests() {
+        SourceSetContainer javaSourceSets = Boilerplate.getJavaSourceSets(getProject());
+        if (javaSourceSets.findByName(SourceSet.MAIN_SOURCE_SET_NAME) == null) {
+            // some test projects don't have a main source set
+            return Collections.emptySet();
+        }
+        return javaSourceSets.getByName(SourceSet.MAIN_SOURCE_SET_NAME)
+            .getOutput().getClassesDirs().getAsFileTree()
+            .getFiles().stream()
+            .filter(file -> file.getName().endsWith(".class"))
+            .map(File::getName)
+            .map(name -> name.substring(0, name.length() - 6))
+            .filter(this::implementsNamingConvention)
+            .collect(Collectors.toSet());
+    }
+
     @TaskAction
     public void doCheck() throws IOException {
         final String problems;
@@ -235,10 +254,12 @@ public void doCheck() throws IOException {
                             );
                         }).sorted()
                         .collect(Collectors.joining("\n"))
-                )
+                ),
                 // TODO: check that the testing tasks are included in the right task based on the name ( from the rule )
-                // TODO: check to make sure that the main source set doesn't have classes that match
-                //         the naming convention (just the names, don't load classes)
+                checkNoneExists(
+                    "Classes matching the test naming convention should be in test not main",
+                    getMainClassNamedLikeTests()
+                )
             );
         }
 
@@ -296,6 +317,18 @@ private String checkNoneExists(String message, Stream<? extends Class<?>> stream
         }
     }
 
+    private String checkNoneExists(String message, Set<? extends String> candidates) {
+        String problem = candidates.stream()
+            .map(each -> "  * " + each)
+            .sorted()
+            .collect(Collectors.joining("\n"));
+        if (problem.isEmpty() == false) {
+            return message + ":\n" + problem;
+        } else {
+            return "";
+        }
+    }
+
     private String checkAtLeastOneExists(String message, Stream<? extends Class<?>> stream) {
         if (stream.findAny().isPresent()) {
             return "";
@@ -337,10 +370,14 @@ private boolean seemsLikeATest(Class<?> clazz) {
     }
 
     private boolean implementsNamingConvention(Class<?> clazz) {
+        return implementsNamingConvention(clazz.getName());
+    }
+
+    private boolean implementsNamingConvention(String className) {
         if (naming.stream()
             .map(TestingConventionRule::getSuffix)
-            .anyMatch(suffix -> clazz.getName().endsWith(suffix))) {
-            getLogger().debug("{} is a test because it matches the naming convention", clazz.getName());
+            .anyMatch(suffix -> className.endsWith(suffix))) {
+            getLogger().debug("{} is a test because it matches the naming convention", className);
             return true;
         }
         return false;