Merge pull request #46 from oleg-nenashev/JENKINS-23641

[JENKINS-23641,JENKINS-25312] - Handle possible nulls if Git checkout fails

src/main/java/com/cloudbees/jenkins/GitHubCommitNotifier.java

@@ -29,29 +29,85 @@
 import java.io.IOException;
 
 import static hudson.model.Result.*;
+import hudson.plugins.git.Revision;
+import hudson.util.ListBoxModel;
+import javax.annotation.Nonnull;
 
 /**
  * Create commit status notifications on the commits based on the outcome of the build.
  *
  * @author <a href="mailto:nicolas.deloof@gmail.com">Nicolas De Loof</a>
+ * @since TODO: define a version Result on failure is configurable.
  */
 public class GitHubCommitNotifier extends Notifier {
 
-
+    private final String resultOnFailure;
+    private static final Result[] SUPPORTED_RESULTS = {FAILURE, UNSTABLE, SUCCESS};
+
     @DataBoundConstructor
+    public GitHubCommitNotifier(String resultOnFailure) {
+        this.resultOnFailure = resultOnFailure;
+    }
+
+    @Deprecated
     public GitHubCommitNotifier() {
+        this(getDefaultResultOnFailure().toString());
     }
 
+    public @Nonnull String getResultOnFailure() {
+        return resultOnFailure != null ? resultOnFailure : getDefaultResultOnFailure().toString();
+    }
+
+    public static @Nonnull Result getDefaultResultOnFailure() {
+        return SUPPORTED_RESULTS[0];
+    }
+
+    /*package*/ @Nonnull Result getEffectiveResultOnFailure() {
+        if (resultOnFailure == null) {
+            return getDefaultResultOnFailure();
+        }
+
+        for (Result result : SUPPORTED_RESULTS) {
+            if (result.toString().equals(resultOnFailure)) return result;
+        }
+        return getDefaultResultOnFailure();
+    }
+
     public BuildStepMonitor getRequiredMonitorService() {
         return BuildStepMonitor.NONE;
     }
 
     @Override
     public boolean perform(AbstractBuild<?, ?> build, Launcher launcher, BuildListener listener) throws InterruptedException, IOException {
-
+        try {
+            updateCommitStatus(build, listener);
+            return true;
+        } catch (IOException error) {
+            final Result buildResult = getEffectiveResultOnFailure();
+            if (buildResult.equals(Result.FAILURE)) {
+                throw error;
+            } else {
+                listener.error("[GitHub Commit Notifier] - " + error.getMessage());
+                if (buildResult.isWorseThan(build.getResult())) {
+                    listener.getLogger().println("[GitHub Commit Notifier] - Build result will be set to " + buildResult);
+                    build.setResult(buildResult);
+                }
+            }
+        }
+        return true;
+    }
+
+    private void updateCommitStatus(@Nonnull AbstractBuild<?, ?> build, @Nonnull BuildListener listener) throws InterruptedException, IOException {
         BuildData buildData = build.getAction(BuildData.class);
-        String sha1 = ObjectId.toString(buildData.getLastBuiltRevision().getSha1());
-
+        if (buildData == null) {
+            throw new IOException(Messages.GitHubCommitNotifier_NoBuildDataError());
+        }
+        final Revision lastBuildRevision = buildData.getLastBuiltRevision();
+        final ObjectId sha1 = lastBuildRevision != null ? lastBuildRevision.getSha1() : null;
+        if (sha1 == null) { // Nowhere to report => fail the build
+            throw new IOException(Messages.GitHubCommitNotifier_NoLastRevisionError());
+        }
+
         for (GitHubRepositoryName name : GitHubRepositoryNameContributor.parseAssociatedNames(build.getProject())) {
             for (GHRepository repository : name.resolve()) {
                 GHCommitState state;
@@ -73,10 +129,9 @@ public boolean perform(AbstractBuild<?, ?> build, Launcher launcher, BuildListen
                 }
 
                 listener.getLogger().println(Messages.GitHubCommitNotifier_SettingCommitStatus(repository.getUrl() + "/commit/" + sha1));
-                repository.createCommitStatus(sha1, state, build.getAbsoluteUrl(), msg);
+                repository.createCommitStatus(ObjectId.toString(sha1), state, build.getAbsoluteUrl(), msg);
             }
         }
-        return true;
     }
 
     @Extension
@@ -89,6 +144,14 @@ public boolean isApplicable(Class<? extends AbstractProject> aClass) {
         public String getDisplayName() {
             return "Set build status on GitHub commit";
         }
+
+        public ListBoxModel doFillResultOnFailureItems() {
+            ListBoxModel items = new ListBoxModel();
+            for (Result result : SUPPORTED_RESULTS) {
+                items.add(result.toString());
+            }
+            return items;
+        }
     }
 
 }

src/main/resources/com/cloudbees/jenkins/GitHubCommitNotifier/config.jelly

@@ -0,0 +1,7 @@
+<j:jelly xmlns:j="jelly:core" xmlns:st="jelly:stapler" xmlns:d="jelly:define" xmlns:l="/lib/layout" xmlns:t="/lib/hudson" xmlns:f="/lib/form">
+    <f:advanced>
+        <f:entry title="${%Result on failure}" field="resultOnFailure">
+            <f:select/>
+        </f:entry>
+    </f:advanced>
+</j:jelly>

src/main/resources/com/cloudbees/jenkins/Messages.properties

@@ -2,3 +2,5 @@ CommitNotifier.Success=Build {0} succeeded in {1}
 CommitNotifier.Unstable=Build {0} found unstable in {1}
 CommitNotifier.Failed=Build {0} failed in {1}
 GitHubCommitNotifier.SettingCommitStatus=Setting commit status on GitHub for {0}
+GitHubCommitNotifier.NoBuildDataError=Cannot retrieve Git metadata for the build
+GitHubCommitNotifier.NoLastRevisionError=Cannot determine sha1 of the commit. The status cannot be reported

src/test/java/com/cloudbees/jenkins/GitHubCommitNotifierTest.java

@@ -0,0 +1,65 @@
+/*
+ * To change this license header, choose License Headers in Project Properties.
+ * To change this template file, choose Tools | Templates
+ * and open the template in the editor.
+ */
+
+package com.cloudbees.jenkins;
+
+import hudson.model.Build;
+import hudson.model.FreeStyleProject;
+import hudson.model.Result;
+import hudson.plugins.git.GitSCM;
+import org.junit.Rule;
+import org.junit.Test;
+import org.jvnet.hudson.test.Bug;
+import org.jvnet.hudson.test.HudsonTestCase;
+import org.jvnet.hudson.test.JenkinsRule;
+
+/**
+ * Tests for {@link GitHubCommitNotifier}.
+ * @author Oleg Nenashev <o.v.nenashev@gmail.com>
+ */
+public class GitHubCommitNotifierTest extends HudsonTestCase {
+
+  //  @Rule
+  //  public JenkinsRule r = new JenkinsRule();
+
+
+    @Test
+    @Bug(23641)
+    public void testNoBuildData() throws Exception, InterruptedException  {
+        FreeStyleProject prj = createFreeStyleProject("23641_noBuildData");
+        prj.getPublishersList().add(new GitHubCommitNotifier());
+        Build b = prj.scheduleBuild2(0).get();
+        assertBuildStatus(Result.FAILURE, b);
+        assertLogContains(Messages.GitHubCommitNotifier_NoBuildDataError(), b);
+    }
+
+    @Test
+    @Bug(23641)
+    public void testNoBuildRevision() throws Exception, InterruptedException {
+        FreeStyleProject prj = createFreeStyleProject();
+        prj.setScm(new GitSCM("http://non.existent.git.repo.nowhere/repo.git"));
+        prj.getPublishersList().add(new GitHubCommitNotifier());
+        Build b = prj.scheduleBuild2(0).get();
+        assertBuildStatus(Result.FAILURE, b);
+        assertLogContains(Messages.GitHubCommitNotifier_NoLastRevisionError(), b);
+    }
+
+    @Bug(25312)
+    public @Test void testMarkUnstableOnCommitNotifierFailure() throws Exception, InterruptedException {
+        FreeStyleProject prj = createFreeStyleProject();
+        prj.getPublishersList().add(new GitHubCommitNotifier(Result.UNSTABLE.toString()));
+        Build b = prj.scheduleBuild2(0).get();
+        assertBuildStatus(Result.UNSTABLE, b);
+    }
+
+    @Bug(25312)
+    public @Test void testMarkSuccessOnCommitNotifierFailure() throws Exception, InterruptedException {
+        FreeStyleProject prj = createFreeStyleProject();
+        prj.getPublishersList().add(new GitHubCommitNotifier(Result.SUCCESS.toString()));
+        Build b = prj.scheduleBuild2(0).get();
+        assertBuildStatus(Result.SUCCESS, b);
+    }
+}