Avoid tasks materialized during configuration phase (elastic#65922)

* Avoid tasks materialized during configuration phase * Fix RestTestFromSnippet testRoot setup
chengyang14 · Dec 14, 2020 · c6c226d · c6c226d
1 parent 75f720c
commit c6c226d
Show file tree

Hide file tree

Showing 99 changed files with 361 additions and 319 deletions.
diff --git a/build.gradle b/build.gradle
@@ -287,23 +287,17 @@ gradle.projectsEvaluated {
       // :test:framework:test cannot run before and after :server:test
       return
     }
-    tasks.matching {it.name.equals('integTest')}.configureEach {integTestTask ->
+    tasks.matching { it.name.equals('integTest')}.configureEach {integTestTask ->
       integTestTask.mustRunAfter tasks.matching { it.name.equals("test") }
     }
 
     configurations.matching { it.canBeResolved }.all { Configuration configuration ->
       dependencies.matching { it instanceof ProjectDependency }.all { ProjectDependency dep ->
         Project upstreamProject = dep.dependencyProject
-        if (upstreamProject != null) {
-          if (project.path == upstreamProject.path) {
-            // TODO: distribution integ tests depend on themselves (!), fix that
-            return
-          }
+        if (project.path != upstreamProject?.path) {
           for (String taskName : ['test', 'integTest']) {
-            Task task = project.tasks.findByName(taskName)
-            Task upstreamTask = upstreamProject.tasks.findByName(taskName)
-            if (task != null && upstreamTask != null) {
-              task.shouldRunAfter(upstreamTask)
+            project.tasks.matching { it.name == taskName }.configureEach {task ->
+              task.shouldRunAfter(upstreamProject.tasks.matching { upStreamTask -> upStreamTask.name == taskName })
             }
           }
         }
@@ -487,7 +481,7 @@ allprojects {
 subprojects {
   project.ext.disableTasks = { String... tasknames ->
     for (String taskname : tasknames) {
-      project.tasks.named(taskname).configure { onlyIf { false } }
+      project.tasks.named(taskname).configure { enabled = false }
     }
   }
 }
diff --git a/buildSrc/reaper/build.gradle b/buildSrc/reaper/build.gradle
@@ -1,4 +1,4 @@
-jar {
+tasks.named("jar").configure {
   archiveFileName = "${project.name}.jar"
   manifest {
     attributes 'Main-Class': 'org.elasticsearch.gradle.reaper.Reaper'

diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/doc/DocsTestPlugin.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/doc/DocsTestPlugin.groovy
@@ -23,7 +23,9 @@ import org.elasticsearch.gradle.Version
 import org.elasticsearch.gradle.VersionProperties
 import org.gradle.api.Plugin
 import org.gradle.api.Project
-import org.gradle.api.Task
+import org.gradle.api.file.Directory
+import org.gradle.api.file.DirectoryProperty
+import org.gradle.api.provider.Provider
 import org.gradle.api.tasks.TaskProvider
 
 /**
@@ -72,8 +74,13 @@ class DocsTestPlugin implements Plugin<Project> {
             }
         }
 
-        project.tasks.register('buildRestTests', RestTestsFromSnippetsTask) {
+        Provider<Directory> restRootDir = project.getLayout().buildDirectory.dir("rest")
+        TaskProvider<RestTestsFromSnippetsTask> buildRestTests = project.tasks.register('buildRestTests', RestTestsFromSnippetsTask) {
             defaultSubstitutions = commonDefaultSubstitutions
+            testRoot.convention(restRootDir)
         }
+
+        // TODO: This effectively makes testRoot not customizable, which we don't do anyway atm
+        project.sourceSets.test.output.dir(restRootDir, builtBy: buildRestTests)
     }
 }
diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/doc/RestTestsFromSnippetsTask.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/doc/RestTestsFromSnippetsTask.groovy
@@ -22,10 +22,13 @@ package org.elasticsearch.gradle.doc
 import groovy.transform.PackageScope
 import org.elasticsearch.gradle.doc.SnippetsTask.Snippet
 import org.gradle.api.InvalidUserDataException
+import org.gradle.api.file.DirectoryProperty
 import org.gradle.api.tasks.Input
 import org.gradle.api.tasks.Internal
 import org.gradle.api.tasks.OutputDirectory
+import org.gradle.api.model.ObjectFactory
 
+import javax.inject.Inject;
 import java.nio.file.Files
 import java.nio.file.Path
 
@@ -54,19 +57,16 @@ class RestTestsFromSnippetsTask extends SnippetsTask {
 
     /**
      * Root directory of the tests being generated. To make rest tests happy
-     * we generate them in a testRoot() which is contained in this directory.
+     * we generate them in a testRoot which is contained in this directory.
      */
-    @OutputDirectory
-    File testRoot = project.file('build/rest')
+    private DirectoryProperty testRoot
 
     @Internal
     Set<String> names = new HashSet<>()
 
-    RestTestsFromSnippetsTask() {
-        project.afterEvaluate {
-            // Wait to set this so testRoot can be customized
-            project.sourceSets.test.output.dir(testRoot, builtBy: this)
-        }
+    @Inject
+    RestTestsFromSnippetsTask(ObjectFactory objectFactory) {
+        testRoot = objectFactory.directoryProperty()
         TestBuilder builder = new TestBuilder()
         doFirst { outputRoot().delete() }
         perSnippet builder.&handleSnippet
@@ -79,10 +79,14 @@ class RestTestsFromSnippetsTask extends SnippetsTask {
      * contained within testRoot.
      */
     File outputRoot() {
-        return new File(testRoot, '/rest-api-spec/test')
+        return new File(testRoot.get().asFile, '/rest-api-spec/test')
     }
 
-    /**
+    @OutputDirectory
+    DirectoryProperty getTestRoot() {
+        return testRoot
+    }
+/**
      * Is this snippet a candidate for conversion to `// CONSOLE`?
      */
     static isConsoleCandidate(Snippet snippet) {

diff --git a/buildSrc/src/main/java/org/elasticsearch/gradle/util/GradleUtils.java b/buildSrc/src/main/java/org/elasticsearch/gradle/util/GradleUtils.java
@@ -32,6 +32,7 @@
 import org.gradle.api.services.BuildService;
 import org.gradle.api.services.BuildServiceRegistration;
 import org.gradle.api.services.BuildServiceRegistry;
+import org.gradle.api.specs.Spec;
 import org.gradle.api.tasks.SourceSet;
 import org.gradle.api.tasks.SourceSetContainer;
 import org.gradle.api.tasks.TaskContainer;
@@ -82,11 +83,7 @@ public static <T extends Task> void maybeConfigure(
         Class<? extends T> type,
         Action<? super T> config
     ) {
-        tasks.withType(type).configureEach(task -> {
-            if (task.getName().equals(name)) {
-                config.execute(task);
-            }
-        });
+        tasks.withType(type).matching((Spec<T>) t -> t.getName().equals(name)).configureEach(task -> { config.execute(task); });
     }
 
     public static TaskProvider<?> findByName(TaskContainer tasks, String name) {

diff --git a/client/rest-high-level/build.gradle b/client/rest-high-level/build.gradle
@@ -79,7 +79,7 @@ tasks.named("integTest").configure {
 }
 
 // Requires https://github.com/elastic/elasticsearch/pull/64403 to have this moved to task avoidance api.
-RestIntegTestTask asyncIntegTest = tasks.create("asyncIntegTest", RestIntegTestTask) {
+TaskProvider<RestIntegTestTask> asyncIntegTest = tasks.register("asyncIntegTest", RestIntegTestTask) {
     systemProperty 'tests.rest.async', 'true'
     systemProperty 'tests.rest.cluster.username', System.getProperty('tests.rest.cluster.username', 'test_user')
     systemProperty 'tests.rest.cluster.password', System.getProperty('tests.rest.cluster.password', 'test-password')

diff --git a/client/test/build.gradle b/client/test/build.gradle
@@ -45,7 +45,7 @@ tasks.named('forbiddenApisTest').configure {
 
 // JarHell is part of es server, which we don't want to pull in
 // TODO: Not anymore. Now in :libs:elasticsearch-core
-jarHell.enabled = false
+tasks.named("jarHell").configure { enabled = false }
 
 // TODO: should we have licenses for our test deps?
 tasks.named("dependencyLicenses").configure { enabled = false }

diff --git a/distribution/packages/build.gradle b/distribution/packages/build.gradle
@@ -486,7 +486,7 @@ subprojects {
     tasks.register("checkLicense") {
       dependsOn buildDist, "checkExtraction"
     }
-    check.dependsOn "checkLicense"
+    tasks.named("check").configure { dependsOn "checkLicense" }
     if (project.name.contains('deb')) {
       tasks.named("checkLicense").configure {
         onlyIf dpkgExists