Ensure that exceptions thrown by various build event actions and list…

…eners are reported on the console and cause the build to fail. Previously, failures thrown by `BuildListener`, `TaskExecutionGraphListener` and `ProjectEvaluationListener` listeners and also by `Action` instances passed to `taskGraph.whenReady()` would be ignored.
gradle · Oct 24, 2018 · e81aae9 · e81aae9
1 parent 1d167a4
commit e81aae9
Show file tree

Hide file tree

Showing 5 changed files with 389 additions and 45 deletions.
diff --git a/subprojects/core/src/integTest/groovy/org/gradle/api/BuildEventsErrorIntegrationTest.groovy b/subprojects/core/src/integTest/groovy/org/gradle/api/BuildEventsErrorIntegrationTest.groovy
@@ -17,14 +17,14 @@
 package org.gradle.api
 
 import org.gradle.integtests.fixtures.AbstractIntegrationSpec
-import spock.lang.Ignore
+import spock.lang.Unroll
 
-public class BuildEventsErrorIntegrationTest extends AbstractIntegrationSpec {
+class BuildEventsErrorIntegrationTest extends AbstractIntegrationSpec {
 
-    def "produces reasonable error message when taskGraph.whenReady action fails"() {
+    def "produces reasonable error message when taskGraph.whenReady closure fails"() {
         buildFile << """
     gradle.taskGraph.whenReady {
-        throw new RuntimeException('broken closure')
+        throw new RuntimeException('broken')
     }
     task a
 """
@@ -33,62 +33,154 @@ public class BuildEventsErrorIntegrationTest extends AbstractIntegrationSpec {
         fails()
 
         then:
-        failure.assertHasDescription("broken closure")
+        failure.assertHasDescription("broken")
                 .assertHasNoCause()
                 .assertHasFileName("Build file '$buildFile'")
                 .assertHasLineNumber(3);
     }
 
-    def "produces reasonable error message when task dependency closure throws exception"() {
+    def "produces reasonable error message when taskGraph.whenReady action fails"() {
         buildFile << """
+    def action = {
+            throw new RuntimeException('broken')
+    } as Action
+    gradle.taskGraph.whenReady(action) 
     task a
-    a.dependsOn {
-        throw new RuntimeException('broken')
+"""
+
+        when:
+        fails()
+
+        then:
+        failure.assertHasDescription("broken")
+                .assertHasNoCause()
+                .assertHasFileName("Build file '$buildFile'")
+                .assertHasLineNumber(3);
     }
+
+    def "produces reasonable error message when taskGraph listener fails"() {
+        buildFile << """
+    def listener = {
+            throw new RuntimeException('broken')
+    } as TaskExecutionGraphListener
+    gradle.taskGraph.addTaskExecutionGraphListener(listener) 
+    task a
 """
+
         when:
-        fails "a"
+        fails()
 
         then:
-        failure.assertHasDescription("Could not determine the dependencies of task ':a'.")
-                .assertHasCause('broken')
+        failure.assertHasDescription("broken")
+                .assertHasNoCause()
                 .assertHasFileName("Build file '$buildFile'")
-                .assertHasLineNumber(4)
+                .assertHasLineNumber(3);
     }
 
     def "produces reasonable error when Gradle.allprojects action fails"() {
         def initScript = file("init.gradle") << """
 allprojects {
-    throw new RuntimeException("broken closure")
+    throw new RuntimeException("broken")
 }
 """
         when:
         executer.usingInitScript(initScript)
         fails "a"
 
         then:
-        failure.assertHasDescription("broken closure")
+        failure.assertHasDescription("broken")
                 .assertHasNoCause()
                 .assertHasFileName("Initialization script '$initScript'")
                 .assertHasLineNumber(3);
     }
 
-    @Ignore
-    def "produces reasonable error when Gradle.buildFinished action fails"() {
-        def initScript = file("init.gradle") << """
-rootProject { task a }
-buildFinished {
-    throw new RuntimeException("broken closure")
+    @Unroll
+    def "produces reasonable error when Gradle.#method closure fails"() {
+        settingsFile << """
+gradle.${method} {
+    throw new RuntimeException("broken")
 }
+gradle.rootProject { task a }
 """
         when:
-        executer.usingInitScript(initScript)
         fails "a"
 
         then:
-        failure.assertHasDescription("broken closure")
+        failure.assertHasDescription("broken")
                 .assertHasNoCause()
-                .assertHasFileName("Initialization script '$initScript'")
-                .assertHasLineNumber(3);
+        // TODO - include location information for buildFinished failure and get rid of the misleading 'build successful'
+        if (hasLocation) {
+            failure.assertHasFileName("Settings file '$settingsFile'")
+                    .assertHasLineNumber(3)
+        }
+
+        where:
+        method              | hasLocation
+        "settingsEvaluated" | true
+        "projectsLoaded"    | true
+        "projectsEvaluated" | true
+        "buildFinished"     | false
+    }
+
+    @Unroll
+    def "produces reasonable error when Gradle.#method action fails"() {
+        settingsFile << """
+def action = {
+    throw new RuntimeException("broken")
+} as Action
+gradle.${method}(action)
+gradle.rootProject { task a }
+"""
+        when:
+        fails "a"
+
+        then:
+        failure.assertHasDescription("broken")
+                .assertHasNoCause()
+        // TODO - include location information for buildFinished failure and get rid of the misleading 'build successful'
+        if (hasLocation) {
+            failure.assertHasFileName("Settings file '$settingsFile'")
+                    .assertHasLineNumber(3)
+        }
+
+        where:
+        method              | hasLocation
+        "settingsEvaluated" | true
+        "projectsLoaded"    | true
+        "projectsEvaluated" | true
+        "buildFinished"     | false
+    }
+
+    @Unroll
+    def "produces reasonable error when BuildListener.#method method fails"() {
+        settingsFile << """
+def listener = new BuildAdapter() {
+    @Override
+    void ${method}(${params}) {
+        throw new RuntimeException("broken")
+    }
+}
+
+gradle.addListener(listener)
+gradle.rootProject { task a }
+"""
+        when:
+        fails "a"
+
+        then:
+        failure.assertHasDescription("broken")
+                .assertHasNoCause()
+        // TODO - include location information for buildFinished failure and get rid of the misleading 'build successful'
+        if (hasLocation) {
+            failure.assertHasFileName("Settings file '$settingsFile'")
+                    .assertHasLineNumber(5)
+        }
+
+        where:
+        method              | params               | hasLocation
+        "settingsEvaluated" | "Settings settings"  | true
+        "projectsLoaded"    | "Gradle gradle"      | true
+        "projectsEvaluated" | "Gradle gradle"      | true
+        "buildFinished"     | "BuildResult result" | false
     }
 }
diff --git a/...ore/src/integTest/groovy/org/gradle/api/ProjectConfigureEventsErrorIntegrationTest.groovy b/...ore/src/integTest/groovy/org/gradle/api/ProjectConfigureEventsErrorIntegrationTest.groovy
@@ -18,13 +18,13 @@ package org.gradle.api
 
 import org.gradle.integtests.fixtures.AbstractIntegrationSpec
 
-public class ProjectConfigureEventsErrorIntegrationTest extends AbstractIntegrationSpec {
+class ProjectConfigureEventsErrorIntegrationTest extends AbstractIntegrationSpec {
 
     def "setup"() {
         settingsFile << "rootProject.name = 'projectConfigure'"
     }
 
-    def "produces reasonable error message when beforeProject action fails"() {
+    def "produces reasonable error message when Gradle.beforeProject closure fails"() {
         when:
         settingsFile << """
     gradle.beforeProject {
@@ -42,7 +42,26 @@ public class ProjectConfigureEventsErrorIntegrationTest extends AbstractIntegrat
                 .assertHasLineNumber(3)
     }
 
-    def "produces reasonable error message when afterProject action fails"() {
+    def "produces reasonable error message when Gradle.beforeProject action fails"() {
+        when:
+        settingsFile << """
+    def action = {
+        throw new RuntimeException("beforeProject failure")
+    } as Action
+    gradle.beforeProject(action)
+"""
+        buildFile << """
+    task test
+"""
+        then:
+        fails('test')
+        failure.assertHasDescription("A problem occurred configuring root project 'projectConfigure'.")
+                .assertHasCause("beforeProject failure")
+                .assertHasFileName("Settings file '${settingsFile.path}'")
+                .assertHasLineNumber(3)
+    }
+
+    def "produces reasonable error message when Gradle.afterProject closure fails"() {
         when:
         settingsFile << """
     gradle.afterProject {
@@ -60,13 +79,93 @@ public class ProjectConfigureEventsErrorIntegrationTest extends AbstractIntegrat
                 .assertHasLineNumber(3)
     }
 
-    def "produces reasonable error message when afterEvaluate action fails"() {
+    def "produces reasonable error message when Gradle.afterProject action fails"() {
+        when:
+        settingsFile << """
+    def action = {
+        throw new RuntimeException("afterProject failure")
+    } as Action
+    gradle.afterProject(action)
+"""
+        buildFile << """
+    task test
+"""
+        then:
+        fails('test')
+        failure.assertHasDescription("A problem occurred configuring root project 'projectConfigure'.")
+                .assertHasCause("afterProject failure")
+                .assertHasFileName("Settings file '${settingsFile.path}'")
+                .assertHasLineNumber(3)
+    }
+
+    def "produces reasonable error message when ProjectEvaluationListener.beforeEvaluate fails"() {
+        when:
+        settingsFile << """
+    class ListenerImpl implements ProjectEvaluationListener {
+        void beforeEvaluate(Project project) {
+            throw new RuntimeException("afterProject failure")
+        }
+        void afterEvaluate(Project project, ProjectState state) {}
+    }
+    gradle.addProjectEvaluationListener(new ListenerImpl())
+"""
+        buildFile << """
+    task test
+"""
+        then:
+        fails('test')
+        failure.assertHasDescription("A problem occurred configuring root project 'projectConfigure'.")
+                .assertHasCause("afterProject failure")
+                .assertHasFileName("Settings file '${settingsFile.path}'")
+                .assertHasLineNumber(4)
+    }
+
+    def "produces reasonable error message when ProjectEvaluationListener.afterEvalutate fails"() {
+        when:
+        settingsFile << """
+    class ListenerImpl implements ProjectEvaluationListener {
+        void beforeEvaluate(Project project) { }
+        void afterEvaluate(Project project, ProjectState state) {
+            throw new RuntimeException("afterProject failure")
+        }
+    }
+    gradle.addProjectEvaluationListener(new ListenerImpl())
+"""
+        buildFile << """
+    task test
+"""
+        then:
+        fails('test')
+        failure.assertHasDescription("A problem occurred configuring root project 'projectConfigure'.")
+                .assertHasCause("afterProject failure")
+                .assertHasFileName("Settings file '${settingsFile.path}'")
+                .assertHasLineNumber(5)
+    }
+
+    def "produces reasonable error message when Project.afterEvaluate closure fails"() {
         when:
         buildFile << """
     project.afterEvaluate {
         throw new RuntimeException("afterEvaluate failure")
     }
     task test
+"""
+        then:
+        fails('test')
+        failure.assertHasDescription("A problem occurred configuring root project 'projectConfigure'.")
+                .assertHasCause("afterEvaluate failure")
+                .assertHasFileName("Build file '${buildFile.path}'")
+                .assertHasLineNumber(3)
+    }
+
+    def "produces reasonable error message when Project.afterEvaluate action fails"() {
+        when:
+        buildFile << """
+    def action = project.afterEvaluate {
+        throw new RuntimeException("afterEvaluate failure")
+    }
+    project.afterEvaluate(action)
+    task test
 """
         then:
         fails('test')

diff --git a/...e/src/integTest/groovy/org/gradle/api/tasks/TaskDependencyInferenceIntegrationTest.groovy b/...e/src/integTest/groovy/org/gradle/api/tasks/TaskDependencyInferenceIntegrationTest.groovy
@@ -293,19 +293,36 @@ The following types/formats are supported:
         result.assertTasksExecuted(":b")
     }
 
+    def "produces reasonable error message when task dependency closure throws exception"() {
+        buildFile << """
+    task a
+    a.dependsOn {
+        throw new RuntimeException('broken')
+    }
+"""
+        when:
+        fails "a"
+
+        then:
+        failure.assertHasDescription("Could not determine the dependencies of task ':a'.")
+                .assertHasCause('broken')
+                .assertHasFileName("Build file '$buildFile'")
+                .assertHasLineNumber(4)
+    }
+
     def "dependency declared using provider with no value fails"() {
         buildFile << """
             def provider = objects.property(String)
-            tasks.register("b") {
+            tasks.register("a") {
                 dependsOn provider
             }
         """
 
         when:
-        fails("b")
+        fails("a")
 
         then:
-        failure.assertHasDescription("Could not determine the dependencies of task ':b'.")
+        failure.assertHasDescription("Could not determine the dependencies of task ':a'.")
         failure.assertHasCause("No value has been specified for this provider.")
     }