[JENKINS-54128] Avoid calling Run.getLogFile (#46)

Jenkinsfile

@@ -1 +1,6 @@
-buildPlugin(configurations: buildPlugin.recommendedConfigurations())
+buildPlugin(configurations: [
+  [ platform: "linux", jdk: "8", jenkins: null ],
+  [ platform: "windows", jdk: "8", jenkins: null ],
+  [ platform: "linux", jdk: "8", jenkins: "2.164.1", javaLevel: "8" ],
+  [ platform: "windows", jdk: "8", jenkins: "2.164.1", javaLevel: "8" ]
+])

pom.xml

@@ -16,7 +16,7 @@
     <properties>
         <revision>1.12</revision>
         <changelist>-SNAPSHOT</changelist>
-        <jenkins.version>2.60.3</jenkins.version>
+        <jenkins.version>2.121.1</jenkins.version>
         <java.level>8</java.level>
     </properties>
 
@@ -68,31 +68,49 @@
                 <artifactId>symbol-annotation</artifactId>
                 <version>1.3</version>
             </dependency>
+            <dependency>
+                <groupId>org.jenkins-ci.plugins.workflow</groupId>
+                <artifactId>workflow-step-api</artifactId>
+                <version>2.18</version>
+            </dependency>
         </dependencies>
     </dependencyManagement>
+
     <dependencies>
+        <dependency>
+            <groupId>org.jenkins-ci.plugins.workflow</groupId>
+            <artifactId>workflow-api</artifactId>
+            <version>2.33</version>
+            <scope>test</scope>
+        </dependency>
         <dependency>
             <groupId>org.jenkins-ci.plugins.workflow</groupId>
             <artifactId>workflow-basic-steps</artifactId>
-            <version>2.1</version>
+            <version>2.15</version>
             <scope>test</scope>
         </dependency>
         <dependency>
             <groupId>org.jenkins-ci.plugins.workflow</groupId>
             <artifactId>workflow-cps</artifactId>
-            <version>2.10</version>
+            <version>2.58</version>
             <scope>test</scope>
         </dependency>
         <dependency>
             <groupId>org.jenkins-ci.plugins.workflow</groupId>
             <artifactId>workflow-durable-task-step</artifactId>
-            <version>2.4</version>
+            <version>2.26</version>
             <scope>test</scope>
         </dependency>
         <dependency>
             <groupId>org.jenkins-ci.plugins.workflow</groupId>
             <artifactId>workflow-job</artifactId>
-            <version>2.4</version>
+            <version>2.31</version>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.jenkins-ci.plugins.workflow</groupId>
+            <artifactId>workflow-support</artifactId>
+            <version>2.21</version>
             <scope>test</scope>
         </dependency>
     </dependencies>

src/main/java/hudson/plugins/textfinder/TextFinderPublisher.java

@@ -4,6 +4,7 @@
 
 import hudson.Extension;
 import hudson.FilePath;
+import hudson.Functions;
 import hudson.Launcher;
 import hudson.Util;
 import hudson.model.AbstractProject;
@@ -21,8 +22,10 @@
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.IOException;
+import java.io.InputStream;
 import java.io.InputStreamReader;
 import java.io.PrintStream;
+import java.io.Reader;
 import java.io.Serializable;
 import java.nio.charset.Charset;
 import java.util.regex.Matcher;
@@ -31,7 +34,6 @@
 import javax.servlet.ServletException;
 import jenkins.MasterToSlaveFileCallable;
 import jenkins.tasks.SimpleBuildStep;
-import org.apache.commons.io.IOUtils;
 import org.apache.tools.ant.DirectoryScanner;
 import org.apache.tools.ant.Project;
 import org.apache.tools.ant.types.FileSet;
@@ -130,13 +132,7 @@ private void findText(Run<?, ?> run, FilePath workspace, TaskListener listener)
             if (alsoCheckConsoleOutput) {
                 // Do not mention the pattern we are looking for to avoid false positives
                 logger.println("[Text Finder] Scanning console output...");
-                foundText |=
-                        checkFile(
-                                run.getLogFile(),
-                                compilePattern(logger, regexp),
-                                logger,
-                                run.getCharset(),
-                                true);
+                foundText |= checkConsole(run, compilePattern(logger, regexp), logger);
                 logger.println(
                         "[Text Finder] Finished looking for pattern "
                                 + "'"
@@ -176,30 +172,31 @@ private void findText(Run<?, ?> run, FilePath workspace, TaskListener listener)
     }
 
     /**
-     * Search the given regexp pattern in the file.
+     * Search the given regexp pattern.
      *
      * @param abortAfterFirstHit true to return immediately as soon as the first hit is found. this
      *     is necessary when we are scanning the console output, because otherwise we'll loop
      *     forever.
      */
-    private static boolean checkFile(
-            File f,
+    private static boolean checkPattern(
+            Reader r,
             Pattern pattern,
             PrintStream logger,
-            Charset charset,
-            boolean abortAfterFirstHit) {
+            String header,
+            boolean abortAfterFirstHit)
+            throws IOException {
         boolean logFilename = true;
         boolean foundText = false;
-        BufferedReader reader = null;
-        try {
+        try (BufferedReader reader = new BufferedReader(r)) {
             // Assume default encoding and text files
             String line;
-            reader = new BufferedReader(new InputStreamReader(new FileInputStream(f), charset));
             while ((line = reader.readLine()) != null) {
                 Matcher matcher = pattern.matcher(line);
                 if (matcher.find()) {
                     if (logFilename) { // first occurrence
-                        logger.println(f + ":");
+                        if (header != null) {
+                            logger.println(header);
+                        }
                         logFilename = false;
                     }
                     logger.println(line);
@@ -209,12 +206,31 @@ private static boolean checkFile(
                     }
                 }
             }
+        }
+        return foundText;
+    }
+
+    private static boolean checkConsole(Run<?, ?> build, Pattern pattern, PrintStream logger) {
+        try (Reader r = build.getLogReader()) {
+            return checkPattern(r, pattern, logger, null, true);
+        } catch (IOException e) {
+            logger.println("[Text Finder] Error reading console output -- ignoring");
+            Functions.printStackTrace(e, logger);
+        }
+
+        return false;
+    }
+
+    private static boolean checkFile(File f, Pattern pattern, PrintStream logger, Charset charset) {
+        try (InputStream is = new FileInputStream(f);
+                Reader r = new InputStreamReader(is, charset)) {
+            return checkPattern(r, pattern, logger, f + ":", false);
         } catch (IOException e) {
             logger.println("[Text Finder] Error reading file '" + f + "' -- ignoring");
-        } finally {
-            IOUtils.closeQuietly(reader);
+            Functions.printStackTrace(e, logger);
         }
-        return foundText;
+
+        return false;
     }
 
     private static Pattern compilePattern(PrintStream logger, String regexp) {
@@ -284,7 +300,7 @@ public FileChecker(RemoteOutputStream ros, String fileSet, String regexp) {
 
         @Override
         public Boolean invoke(File ws, VirtualChannel channel) throws IOException {
-            PrintStream logger = new PrintStream(ros, false, Charset.defaultCharset().toString());
+            PrintStream logger = new PrintStream(ros, true, Charset.defaultCharset().toString());
 
             // Collect list of files for searching
             FileSet fs = new FileSet();
@@ -318,7 +334,7 @@ public Boolean invoke(File ws, VirtualChannel channel) throws IOException {
                     continue;
                 }
 
-                foundText |= checkFile(f, pattern, logger, Charset.defaultCharset(), false);
+                foundText |= checkFile(f, pattern, logger, Charset.defaultCharset());
             }
 
             return foundText;

src/test/java/hudson/plugins/textfinder/TestUtils.java

@@ -0,0 +1,59 @@
+package hudson.plugins.textfinder;
+
+import static org.junit.Assert.assertNotNull;
+
+import hudson.Functions;
+import hudson.model.Run;
+import java.io.File;
+import java.io.IOException;
+import org.jenkinsci.plugins.workflow.actions.WorkspaceAction;
+import org.jenkinsci.plugins.workflow.flow.FlowExecution;
+import org.jenkinsci.plugins.workflow.graph.FlowGraphWalker;
+import org.jenkinsci.plugins.workflow.graph.FlowNode;
+import org.jenkinsci.plugins.workflow.job.WorkflowRun;
+import org.jvnet.hudson.test.JenkinsRule;
+
+/** Utilities for testing Text Finder */
+public class TestUtils {
+
+    private static void assertContainsMatch(
+            String header, String text, JenkinsRule rule, Run<?, ?> build, boolean isShell)
+            throws IOException {
+        String prompt;
+        if (isShell) {
+            prompt = Functions.isWindows() ? ">" : "+ ";
+        } else {
+            prompt = "";
+        }
+        rule.assertLogContains(String.format("%s%s%s", header, prompt, text), build);
+    }
+
+    public static void assertConsoleContainsMatch(
+            String text, JenkinsRule rule, Run<?, ?> build, boolean isShell) throws IOException {
+        assertContainsMatch("", text, rule, build, isShell);
+    }
+
+    public static void assertFileContainsMatch(
+            File file, String text, JenkinsRule rule, Run<?, ?> build, boolean isShell)
+            throws IOException {
+        assertContainsMatch(
+                String.format("%s:%s", file, System.getProperty("line.separator")),
+                text,
+                rule,
+                build,
+                isShell);
+    }
+
+    public static File getWorkspace(WorkflowRun build) {
+        FlowExecution execution = build.getExecution();
+        assertNotNull(execution);
+        FlowGraphWalker walker = new FlowGraphWalker(execution);
+        for (FlowNode node : walker) {
+            WorkspaceAction action = node.getAction(WorkspaceAction.class);
+            if (action != null) {
+                return new File(action.getWorkspace().getRemote());
+            }
+        }
+        throw new IllegalStateException("Failed to find workspace");
+    }
+}

src/test/java/hudson/plugins/textfinder/TextFinderPublisherAgentTest.java

@@ -1,16 +1,9 @@
 package hudson.plugins.textfinder;
 
-import hudson.Functions;
 import hudson.model.Result;
 import hudson.slaves.DumbSlave;
 import java.io.File;
-import java.io.IOException;
-import java.util.ArrayList;
-import java.util.List;
-import org.jenkinsci.plugins.workflow.actions.WorkspaceAction;
 import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition;
-import org.jenkinsci.plugins.workflow.graph.FlowGraphWalker;
-import org.jenkinsci.plugins.workflow.graph.FlowNode;
 import org.jenkinsci.plugins.workflow.job.WorkflowJob;
 import org.jenkinsci.plugins.workflow.job.WorkflowRun;
 import org.junit.Rule;
@@ -24,61 +17,46 @@ public class TextFinderPublisherAgentTest {
 
     @Rule public JenkinsRule rule = new JenkinsRule();
 
-    private void assertLogContainsMatch(File file, String text, WorkflowRun build, boolean isShell)
-            throws IOException {
-        String prompt;
-        if (isShell) {
-            prompt = Functions.isWindows() ? ">" : "+ ";
-        } else {
-            prompt = "";
-        }
-        rule.assertLogContains(
-                String.format(
-                        "%s:%s%s%s", file, System.getProperty("line.separator"), prompt, text),
-                build);
-    }
-
-    private File getWorkspace(WorkflowRun build) {
-        FlowGraphWalker walker = new FlowGraphWalker(build.getExecution());
-        List<WorkspaceAction> actions = new ArrayList<>();
-        for (FlowNode node : walker) {
-            WorkspaceAction action = node.getAction(WorkspaceAction.class);
-            if (action != null) {
-                return new File(action.getWorkspace().getRemote());
-            }
-        }
-        throw new IllegalStateException("Failed to find workspace");
-    }
-
     @Test
     public void failureIfFoundInFileOnAgent() throws Exception {
         DumbSlave agent = rule.createOnlineSlave();
-        WorkflowJob project = rule.createProject(WorkflowJob.class, "pipeline");
+        WorkflowJob project = rule.createProject(WorkflowJob.class);
         project.setDefinition(
                 new CpsFlowDefinition(
                         String.format(
-                                "node('%s') {writeFile file: 'out.txt', text: 'foobar'}\n"
-                                        + "node('%s') {findText regexp: 'foobar', fileSet: 'out.txt'}\n",
-                                agent.getNodeName(), agent.getNodeName())));
+                                "node('%s') {\n"
+                                        + "  writeFile file: 'out.txt', text: 'foobar'\n"
+                                        + "  findText regexp: 'foobar', fileSet: 'out.txt'\n"
+                                        + "}\n",
+                                agent.getNodeName()),
+                        true));
         WorkflowRun build = project.scheduleBuild2(0).get();
         rule.waitForCompletion(build);
         rule.assertLogContains(
                 "[Text Finder] Looking for pattern " + "'" + UNIQUE_TEXT + "'" + " in the files at",
                 build);
-        assertLogContainsMatch(new File(getWorkspace(build), "out.txt"), UNIQUE_TEXT, build, false);
+        TestUtils.assertFileContainsMatch(
+                new File(TestUtils.getWorkspace(build), "out.txt"),
+                UNIQUE_TEXT,
+                rule,
+                build,
+                false);
         rule.assertBuildStatus(Result.FAILURE, build);
     }
 
     @Test
     public void failureIfFoundInConsoleOnAgent() throws Exception {
         DumbSlave agent = rule.createOnlineSlave();
-        WorkflowJob project = rule.createProject(WorkflowJob.class, "pipeline");
+        WorkflowJob project = rule.createProject(WorkflowJob.class);
         project.setDefinition(
                 new CpsFlowDefinition(
                         String.format(
-                                "node('%s') {isUnix() ? sh('echo foobar') : bat(\"prompt \\$G\\r\\necho foobar\")}\n"
-                                        + "node('%s') {findText regexp: 'foobar', alsoCheckConsoleOutput: true}\n",
-                                agent.getNodeName(), agent.getNodeName())));
+                                "node('%s') {\n"
+                                        + "  isUnix() ? sh('echo foobar') : bat(\"prompt \\$G\\r\\necho foobar\")\n"
+                                        + "  findText regexp: 'foobar', alsoCheckConsoleOutput: true\n"
+                                        + "}\n",
+                                agent.getNodeName()),
+                        true));
         WorkflowRun build = project.scheduleBuild2(0).get();
         rule.waitForCompletion(build);
         rule.assertLogContains("[Text Finder] Scanning console output...", build);
@@ -87,7 +65,7 @@ public void failureIfFoundInConsoleOnAgent() throws Exception {
                         + UNIQUE_TEXT
                         + "' in the console output",
                 build);
-        assertLogContainsMatch(build.getLogFile(), ECHO_UNIQUE_TEXT, build, true);
+        TestUtils.assertConsoleContainsMatch(ECHO_UNIQUE_TEXT, rule, build, true);
         rule.assertBuildStatus(Result.FAILURE, build);
     }
 }