Simplify gradle test task error reporting (#59760) (#59966)

* Simplify test error reporting - avoid using extra plugin - avoid extra task listener (should be avoided related to #57918 ) - keep all logic in the listener
elastic · Jul 21, 2020 · b5ca9c5 · b5ca9c5
1 parent aae08e5
commit b5ca9c5
Show file tree

Hide file tree

Showing 2 changed files with 14 additions and 35 deletions.
diff --git a/buildSrc/src/main/java/org/elasticsearch/gradle/ElasticsearchJavaPlugin.java b/buildSrc/src/main/java/org/elasticsearch/gradle/ElasticsearchJavaPlugin.java
@@ -36,7 +36,6 @@
 import org.gradle.api.artifacts.ModuleDependency;
 import org.gradle.api.artifacts.ProjectDependency;
 import org.gradle.api.artifacts.ResolutionStrategy;
-import org.gradle.api.execution.TaskActionListener;
 import org.gradle.api.file.FileCollection;
 import org.gradle.api.plugins.BasePlugin;
 import org.gradle.api.plugins.JavaLibraryPlugin;
@@ -72,8 +71,6 @@ public class ElasticsearchJavaPlugin implements Plugin<Project> {
     public void apply(Project project) {
         // make sure the global build info plugin is applied to the root project
         project.getRootProject().getPluginManager().apply(GlobalBuildInfoPlugin.class);
-        // apply global test task failure listener
-        project.getRootProject().getPluginManager().apply(TestFailureReportingPlugin.class);
         // common repositories setup
         project.getPluginManager().apply(RepositoriesSetupPlugin.class);
         project.getPluginManager().apply(JavaLibraryPlugin.class);
@@ -223,7 +220,7 @@ public static void configureTestTasks(Project project) {
         project.getTasks().withType(Test.class).configureEach(test -> {
             File testOutputDir = new File(test.getReports().getJunitXml().getDestination(), "output");
 
-            ErrorReportingTestListener listener = new ErrorReportingTestListener(test.getTestLogging(), testOutputDir);
+            ErrorReportingTestListener listener = new ErrorReportingTestListener(test.getTestLogging(), test.getLogger(), testOutputDir);
             test.getExtensions().add("errorReportingTestListener", listener);
             test.addTestOutputListener(listener);
             test.addTestListener(listener);
@@ -473,31 +470,4 @@ private static void configureJavadoc(Project project) {
         // ensure javadoc task is run with 'check'
         project.getTasks().named(LifecycleBasePlugin.CHECK_TASK_NAME).configure(t -> t.dependsOn(javadoc));
     }
-
-    static class TestFailureReportingPlugin implements Plugin<Project> {
-        @Override
-        public void apply(Project project) {
-            if (project != project.getRootProject()) {
-                throw new IllegalStateException(this.getClass().getName() + " can only be applied to the root project.");
-            }
-
-            project.getGradle().addListener(new TaskActionListener() {
-                @Override
-                public void beforeActions(Task task) {}
-
-                @Override
-                public void afterActions(Task task) {
-                    if (task instanceof Test) {
-                        ErrorReportingTestListener listener = task.getExtensions().findByType(ErrorReportingTestListener.class);
-                        if (listener != null && listener.getFailedTests().size() > 0) {
-                            task.getLogger().lifecycle("\nTests with failures:");
-                            for (ErrorReportingTestListener.Descriptor failure : listener.getFailedTests()) {
-                                task.getLogger().lifecycle(" - " + failure.getFullName());
-                            }
-                        }
-                    }
-                }
-            });
-        }
-    }
 }
diff --git a/buildSrc/src/main/java/org/elasticsearch/gradle/test/ErrorReportingTestListener.java b/buildSrc/src/main/java/org/elasticsearch/gradle/test/ErrorReportingTestListener.java
@@ -21,7 +21,6 @@
 import org.gradle.api.internal.tasks.testing.logging.FullExceptionFormatter;
 import org.gradle.api.internal.tasks.testing.logging.TestExceptionFormatter;
 import org.gradle.api.logging.Logger;
-import org.gradle.api.logging.Logging;
 import org.gradle.api.tasks.testing.TestDescriptor;
 import org.gradle.api.tasks.testing.TestListener;
 import org.gradle.api.tasks.testing.TestOutputEvent;
@@ -49,17 +48,18 @@
 import java.util.concurrent.ConcurrentHashMap;
 
 public class ErrorReportingTestListener implements TestOutputListener, TestListener {
-    private static final Logger LOGGER = Logging.getLogger(ErrorReportingTestListener.class);
     private static final String REPRODUCE_WITH_PREFIX = "REPRODUCE WITH";
 
     private final TestExceptionFormatter formatter;
     private final File outputDirectory;
+    private final Logger taskLogger;
     private Map<Descriptor, EventWriter> eventWriters = new ConcurrentHashMap<>();
     private Map<Descriptor, Deque<String>> reproductionLines = new ConcurrentHashMap<>();
     private Set<Descriptor> failedTests = new LinkedHashSet<>();
 
-    public ErrorReportingTestListener(TestLogging testLogging, File outputDirectory) {
+    public ErrorReportingTestListener(TestLogging testLogging, Logger taskLogger, File outputDirectory) {
         this.formatter = new FullExceptionFormatter(testLogging);
+        this.taskLogger = taskLogger;
         this.outputDirectory = outputDirectory;
     }
 
@@ -120,6 +120,15 @@ public void afterSuite(final TestDescriptor suite, TestResult result) {
                     }
                 }
             }
+            if (suite.getParent() == null) {
+                // per test task top level gradle test run suite finished
+                if (getFailedTests().size() > 0) {
+                    taskLogger.lifecycle("\nTests with failures:");
+                    for (ErrorReportingTestListener.Descriptor failure : getFailedTests()) {
+                        taskLogger.lifecycle(" - " + failure.getFullName());
+                    }
+                }
+            }
         } catch (IOException e) {
             throw new UncheckedIOException("Error reading test suite output", e);
         } finally {
@@ -129,7 +138,7 @@ public void afterSuite(final TestDescriptor suite, TestResult result) {
                 try {
                     writer.close();
                 } catch (IOException e) {
-                    LOGGER.error("Failed to close test suite output stream", e);
+                    taskLogger.error("Failed to close test suite output stream", e);
                 }
             }
         }