fix: Task does not fail if report generation fails (#929)

* test: reproduce the reported issue as #909 Signed-off-by: Kengo TODA <[email protected]> * test: fix test case Signed-off-by: Kengo TODA <[email protected]> * fix: Task does not fail if report generation fails Signed-off-by: Kengo TODA <[email protected]> * chore: format code by spotless Signed-off-by: Kengo TODA <[email protected]> * chore: resolve problems reported by SonarQube Signed-off-by: Kengo TODA <[email protected]> --------- Signed-off-by: Kengo TODA <[email protected]>
spotbugs · Aug 8, 2023 · c6f2ec9 · c6f2ec9
1 parent 4ebb221
commit c6f2ec9
Show file tree

Hide file tree

Showing 5 changed files with 149 additions and 2 deletions.
diff --git a/src/functionalTest/groovy/com/github/spotbugs/snom/Issue909.groovy b/src/functionalTest/groovy/com/github/spotbugs/snom/Issue909.groovy
@@ -0,0 +1,73 @@
+/*
+ * Copyright 2023 SpotBugs team
+ *
+ * <p>Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file
+ * except in compliance with the License. You may obtain a copy of the License at
+ *
+ * <p>http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * <p>Unless required by applicable law or agreed to in writing, software distributed under the
+ * License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
+ * express or implied. See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.github.spotbugs.snom
+
+import org.gradle.testkit.runner.GradleRunner
+import org.gradle.testkit.runner.TaskOutcome
+import spock.lang.Specification
+
+import java.nio.file.Files
+import java.nio.file.Path
+
+/**
+ * @see <a href="https://github.com/spotbugs/spotbugs-gradle-plugin/issues/909">GitHub Issues</a>
+ */
+class Issue909 extends Specification {
+    Path rootDir
+    Path buildFile
+
+    def setup() {
+        rootDir = Files.createTempDirectory("spotbugs-gradle-plugin")
+        buildFile = rootDir.resolve("build.gradle")
+        buildFile.toFile() << """
+plugins {
+  id "java"
+  id "com.github.spotbugs"
+}
+repositories {
+  mavenCentral()
+}
+tasks.spotbugsMain {
+    reports {
+        html {
+            required = true
+            stylesheet = resources.text.fromString("I am not valid XSL")
+        }
+    }
+}
+        """
+        Path sourceDir = rootDir.resolve("src").resolve("main").resolve("java")
+        sourceDir.toFile().mkdirs()
+        Path sourceFile = sourceDir.resolve("Foo.java")
+        sourceFile.toFile() << """
+public class Foo {
+    public static void main(String... args) {
+        System.out.println("Hello, SpotBugs!");
+    }
+}
+"""
+    }
+
+    def "cannot generate HTML report if invalid XSL is provided"() {
+        when:
+        def result = GradleRunner.create()
+                .withProjectDir(rootDir.toFile())
+                .withArguments('spotbugsMain')
+                .withPluginClasspath()
+                .buildAndFail()
+
+        then:
+        TaskOutcome.FAILED == result.task(":spotbugsMain").outcome
+    }
+}
diff --git a/src/main/groovy/com/github/spotbugs/snom/internal/OutputScanner.java b/src/main/groovy/com/github/spotbugs/snom/internal/OutputScanner.java
@@ -0,0 +1,58 @@
+/*
+ * Copyright 2023 SpotBugs team
+ *
+ * <p>Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file
+ * except in compliance with the License. You may obtain a copy of the License at
+ *
+ * <p>http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * <p>Unless required by applicable law or agreed to in writing, software distributed under the
+ * License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
+ * express or implied. See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.github.spotbugs.snom.internal;
+
+import java.io.ByteArrayOutputStream;
+import java.io.FilterOutputStream;
+import java.io.IOException;
+import java.io.OutputStream;
+import org.jetbrains.annotations.NotNull;
+
+/**
+ * Monitors the stdout of forked process, and report when it contains some problems reported by
+ * SpotBugs core.
+ */
+class OutputScanner extends FilterOutputStream {
+  private final ByteArrayOutputStream builder = new ByteArrayOutputStream();
+  private boolean failedToReport = false;
+
+  public OutputScanner(OutputStream out) {
+    super(out);
+  }
+
+  boolean isFailedToReport() {
+    return failedToReport;
+  }
+
+  @Override
+  public void write(byte @NotNull [] b, int off, int len) throws IOException {
+    super.write(b, off, len);
+    builder.write(b, off, len);
+  }
+
+  @Override
+  public void write(int b) throws IOException {
+    super.write(b);
+    builder.write(b);
+
+    if (b == '\n') {
+      String line = builder.toString();
+      if (line.contains("Could not generate HTML output")) {
+        failedToReport = true;
+      }
+
+      builder.reset();
+    }
+  }
+}
diff --git a/src/main/groovy/com/github/spotbugs/snom/internal/SpotBugsRunnerForHybrid.java b/src/main/groovy/com/github/spotbugs/snom/internal/SpotBugsRunnerForHybrid.java
@@ -17,7 +17,7 @@
 import com.github.spotbugs.snom.SpotBugsTask;
 import edu.umd.cs.findbugs.annotations.NonNull;
 import groovy.lang.Closure;
-import java.io.File;
+import java.io.*;
 import java.net.URI;
 import java.nio.file.Path;
 import java.util.ArrayList;
@@ -119,6 +119,8 @@ public abstract static class SpotBugsExecutor implements WorkAction<SpotBugsWork
     private final Logger log = LoggerFactory.getLogger(getClass());
     private final ExecOperations execOperations;
 
+    private OutputScanner stderrOutputScanner;
+
     @Inject
     public SpotBugsExecutor(ExecOperations execOperations) {
       this.execOperations = Objects.requireNonNull(execOperations);
@@ -131,11 +133,15 @@ public void execute() {
 
       final int exitValue =
           execOperations.javaexec(configureJavaExec(params)).rethrowFailure().getExitValue();
+      final boolean ignoreFailures = params.getIgnoreFailures().getOrElse(Boolean.FALSE);
       if (ignoreMissingClassFlag(exitValue) == 0) {
+        if (stderrOutputScanner.isFailedToReport() && !ignoreFailures) {
+          throw new GradleException("SpotBugs analysis succeeded but report generation failed");
+        }
         return;
       }
 
-      if (params.getIgnoreFailures().getOrElse(Boolean.FALSE)) {
+      if (ignoreFailures) {
         log.warn("SpotBugs ended with exit code " + exitValue);
         return;
       }
@@ -180,6 +186,8 @@ private Action<? super JavaExecSpec> configureJavaExec(SpotBugsWorkParameters pa
           spec.setExecutable(params.getJavaToolchainExecutablePath().get());
         }
         spec.setIgnoreExitValue(true);
+        this.stderrOutputScanner = new OutputScanner(System.err);
+        spec.setErrorOutput(stderrOutputScanner);
       };
     }
   }

diff --git a/src/main/groovy/com/github/spotbugs/snom/internal/SpotBugsRunnerForJavaExec.java b/src/main/groovy/com/github/spotbugs/snom/internal/SpotBugsRunnerForJavaExec.java
@@ -37,6 +37,8 @@ public class SpotBugsRunnerForJavaExec extends SpotBugsRunner {
   private final Logger log = LoggerFactory.getLogger(SpotBugsRunnerForJavaExec.class);
   private final Property<JavaLauncher> javaLauncher;
 
+  private OutputScanner stderrOutputScanner;
+
   public SpotBugsRunnerForJavaExec(Property<JavaLauncher> javaLauncher) {
     this.javaLauncher = javaLauncher;
   }
@@ -46,6 +48,9 @@ public void run(@NonNull SpotBugsTask task) {
     // TODO print version of SpotBugs and Plugins
     try {
       task.getProject().javaexec(configureJavaExec(task)).rethrowFailure().assertNormalExitValue();
+      if (stderrOutputScanner.isFailedToReport() && !task.getIgnoreFailures()) {
+        throw new GradleException("SpotBugs analysis succeeded but report generation failed");
+      }
     } catch (ExecException e) {
       if (task.getIgnoreFailures()) {
         log.warn("SpotBugs reported failures", task.getShowStackTraces() ? e : null);
@@ -81,6 +86,8 @@ private Action<? super JavaExecSpec> configureJavaExec(SpotBugsTask task) {
       if (maxHeapSize != null) {
         spec.setMaxHeapSize(maxHeapSize);
       }
+      stderrOutputScanner = new OutputScanner(System.err);
+      spec.setErrorOutput(stderrOutputScanner);
       if (javaLauncher.isPresent()) {
         log.info(
             "Spotbugs will be executed using Java Toolchain configuration: Vendor: {} | Version: {}",

diff --git a/src/main/groovy/com/github/spotbugs/snom/internal/SpotBugsRunnerForWorker.java b/src/main/groovy/com/github/spotbugs/snom/internal/SpotBugsRunnerForWorker.java
@@ -40,6 +40,7 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+@Deprecated
 public class SpotBugsRunnerForWorker extends SpotBugsRunner {
   private final Logger log = LoggerFactory.getLogger(SpotBugsRunnerForWorker.class);
   private final WorkerExecutor workerExecutor;