JENKINS-23786: Allow Shell jobs to set a return code for unstable

Currently a shell job has to make a HTTP call back to Jenkins to set its build result as unstable. This is slow, requires the slave to have access to the master's HTTP interface, and is fiddly. The alternative, the TextFinder plugin, is no better. Instead, allow a job to set the build result to unstable with a return value. Adds the Advanced parameter "unstableReturn" which, if non-zero, is the code the script must return to set the build as unstable.
jenkinsci · Sep 22, 2016 · e773eb4 · e773eb4
1 parent 3c85fd7
commit e773eb4
Show file tree

Hide file tree

Showing 3 changed files with 124 additions and 3 deletions.
diff --git a/core/src/main/java/hudson/tasks/Shell.java b/core/src/main/java/hudson/tasks/Shell.java
@@ -27,7 +27,10 @@
 import hudson.Functions;
 import hudson.Util;
 import hudson.Extension;
+import hudson.Proc;
 import hudson.model.AbstractProject;
+import hudson.model.Result;
+import hudson.remoting.Callable;
 import hudson.remoting.VirtualChannel;
 import hudson.util.FormValidation;
 import java.io.IOException;
@@ -53,16 +56,25 @@
  */
 public class Shell extends CommandInterpreter {
     @DataBoundConstructor
-    public Shell(String command) {
+    public Shell(String command, Integer unstableReturn) {
         super(LineEndingConversion.convertEOL(command, LineEndingConversion.EOLType.Unix));
+        if (unstableReturn != null && unstableReturn.equals(0))
+            unstableReturn = null;
+        this.unstableReturn = unstableReturn;
+    }
+
+    public Shell(String command) {
+        this(command, null);
     }
 
+    private final Integer unstableReturn;
+
     /**
      * Older versions of bash have a bug where non-ASCII on the first line
      * makes the shell think the file is a binary file and not a script. Adding
      * a leading line feed works around this problem.
      */
-    private static String addLineFeedForNonASCII(String s) {
+    private static String addCrForNonASCII(String s) {
         if(!s.startsWith("#!")) {
             if (s.indexOf('\n')!=0) {
                 return "\n" + s;
@@ -87,13 +99,31 @@ public String[] buildCommandLine(FilePath script) {
     }
 
     protected String getContents() {
-        return addLineFeedForNonASCII(LineEndingConversion.convertEOL(command,LineEndingConversion.EOLType.Unix));
+        return addCrForNonASCII(fixCrLf(command));
     }
 
     protected String getFileExtension() {
         return ".sh";
     }
 
+    public final Integer getUnstableReturn() {
+        return unstableReturn;
+    }
+
+    /**
+      * Allow the user to define a result for "unstable":
+      */
+    @Override
+    protected int join(Proc p) throws IOException, InterruptedException {
+        final int result = p.join();
+        if (this.unstableReturn != null && result != 0 && this.unstableReturn.equals(result)) {
+            getBuild().setResult(Result.UNSTABLE);
+            return 0;
+        }
+        else
+            return result;
+    }
+
     @Override
     public DescriptorImpl getDescriptor() {
         return (DescriptorImpl)super.getDescriptor();
@@ -161,6 +191,17 @@ public String getDisplayName() {
             return Messages.Shell_DisplayName();
         }
 
+        @Override
+        public Builder newInstance(StaplerRequest req, JSONObject data) {
+            final String unstableReturnStr = data.getString("unstableReturn");
+            Integer unstableReturn = null;
+            if (unstableReturnStr != null && ! unstableReturnStr.isEmpty()) {
+                /* Already validated by f.number in the form */
+                unstableReturn = (Integer)Integer.parseInt(unstableReturnStr, 10);
+            }
+            return new Shell(data.getString("command"), unstableReturn);
+        }
+
         @Override
         public boolean configure(StaplerRequest req, JSONObject data) throws FormException {
             req.bindJSON(this, data);

diff --git a/core/src/main/resources/hudson/tasks/Shell/config.groovy b/core/src/main/resources/hudson/tasks/Shell/config.groovy
@@ -27,3 +27,11 @@ f=namespace(lib.FormTagLib)
 f.entry(title:_("Command"),description:_("description",rootURL)) {
     f.textarea(name: "command", value: instance?.command, class: "fixed-width", 'codemirror-mode': 'shell', 'codemirror-config': "mode: 'text/x-sh'")
 }
+
+f.advanced() {
+
+    f.entry(title:_("Return code to set build unstable"), description:_("If set, the script return code that will be interpreted as an unstable build result.")) {
+        f.number(name: "unstableReturn", value: instance?.unstableReturn, min:1, max:255, step:1)
+    }
+
+}
diff --git a/test/src/test/java/hudson/tasks/ShellTest.java b/test/src/test/java/hudson/tasks/ShellTest.java
@@ -2,15 +2,20 @@
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assume.assumeTrue;
+
 
 import hudson.Functions;
 import hudson.Launcher.ProcStarter;
 import hudson.Proc;
+import hudson.model.Result;
 import hudson.model.FreeStyleBuild;
 import hudson.model.FreeStyleProject;
 import org.apache.commons.io.FileUtils;
 import org.jvnet.hudson.test.FakeLauncher;
+import org.jvnet.hudson.test.HudsonTestCase;
 import org.jvnet.hudson.test.PretendSlave;
+import org.jvnet.hudson.test.Issue;
 
 import java.io.File;
 import java.io.IOException;
@@ -74,4 +79,71 @@ public Proc onLaunch(ProcStarter p) throws IOException {
         assertEquals(1,s.numLaunch);
         assertTrue(FileUtils.readFileToString(b.getLogFile()).contains("Hudson was here"));
     }
+
+    /* A FakeLauncher that just returns the specified error code */
+    private class ReturnCodeFakeLauncher implements FakeLauncher {
+        final int code;
+
+        ReturnCodeFakeLauncher(int code)
+        {
+            super();
+            this.code = code;
+        }
+
+        @Override
+        public Proc onLaunch(ProcStarter p) throws IOException {
+            return new FinishedProc(this.code);
+        }
+    }
+
+    @Issue("JENKINS-23786")
+    public void testUnstableReturn() throws Exception {
+        if(Functions.isWindows())
+            return;
+
+        PretendSlave returns2 = rule.createPretendSlave(new ReturnCodeFakeLauncher(2));
+        PretendSlave returns1 = rule.createPretendSlave(new ReturnCodeFakeLauncher(1));
+        PretendSlave returns0 = rule.createPretendSlave(new ReturnCodeFakeLauncher(0));
+
+        FreeStyleProject p;
+        FreeStyleBuild b;
+
+        /* Unstable=2, error codes 0/1/2 */
+        p = rule.createFreeStyleProject();
+        p.getBuildersList().add(new Shell("", 2));
+        p.setAssignedNode(returns2);
+        b = assertBuildStatus(Result.UNSTABLE, p.scheduleBuild2(0).get());
+
+        p = rule.createFreeStyleProject();
+        p.getBuildersList().add(new Shell("", 2));
+        p.setAssignedNode(returns1);
+        b = assertBuildStatus(Result.FAILURE, p.scheduleBuild2(0).get());
+
+        p = rule.createFreeStyleProject();
+        p.getBuildersList().add(new Shell("", 2));
+        p.setAssignedNode(returns0);
+        b = assertBuildStatus(Result.SUCCESS, p.scheduleBuild2(0).get());
+
+        /* unstable=null, error codes 0/1/2 */
+        p = rule.createFreeStyleProject();
+        p.getBuildersList().add(new Shell("", null));
+        p.setAssignedNode(returns2);
+        b = assertBuildStatus(Result.FAILURE, p.scheduleBuild2(0).get());
+
+        p = rule.createFreeStyleProject();
+        p.getBuildersList().add(new Shell("", null));
+        p.setAssignedNode(returns1);
+        b = assertBuildStatus(Result.FAILURE, p.scheduleBuild2(0).get());
+
+        p = rule.createFreeStyleProject();
+        p.getBuildersList().add(new Shell("", null));
+        p.setAssignedNode(returns0);
+        b = assertBuildStatus(Result.SUCCESS, p.scheduleBuild2(0).get());
+
+        /* Creating unstable=0 produces unstable=null */
+        assertNull( new Shell("",0).getUnstableReturn() );
+
+    }
+
+
 }