Jenkins pipeline support for notifier (#185)

Author: balihb

Diff for: .gitignore

@@ -6,3 +6,4 @@ target/
 .gradle
 local.properties
 **build
+/bin/

Diff for: build.gradle

@@ -47,7 +47,7 @@ dependencies {
   jenkinsPlugins 'org.jenkins-ci.plugins:credentials:1.22@jar'
 
   optionalJenkinsPlugins 'org.jenkins-ci.plugins:junit:1.6@jar'
-  optionalJenkinsPlugins 'org.jenkins-ci.plugins:cobertura:1.9.6@jar'
+  optionalJenkinsPlugins 'org.jenkins-ci.plugins:cobertura:1.11@jar'
 
   testCompile 'junit:junit:4.12'
   testCompile 'org.apache.httpcomponents:httpclient:4.3:tests'

Diff for: docs/advanced.md

@@ -80,3 +80,18 @@ avoid confusion. Please note that builds on the same diff triggered by the same
 
 Also note that if you pass additional arguments to your harbormaster request they may need to be included in the `ABORT_ON_REVISION_ID` field as well. A good example is when you use the same CI job to build multiple targets on a single diff. So for example, if the jenkins request params have
 `TARGET=some_target`, then to ensure other targets are not cancelled for the same diff, you may want to set `ABORT_ON_REVISION_ID=some_target_${buildable.revision}`.
+
+Pipeline
+--------
+
+Typically the Phabricator Notifier is used as a reporting step in a Jenkins Pipeline. The test result collector step must be run before the Notifier.
+
+```groovy
+stage ('report') {
+    //...
+    // junit()
+    step([$class: 'PhabricatorNotifier', commentOnSuccess: true, commentWithConsoleLinkOnFailure: true])
+    //...
+}
+
+```

Diff for: src/main/java/com/uber/jenkins/phabricator/BuildResultProcessor.java

@@ -37,8 +37,8 @@
 import com.uber.jenkins.phabricator.utils.CommonUtils;
 import com.uber.jenkins.phabricator.utils.Logger;
 import hudson.FilePath;
-import hudson.model.AbstractBuild;
 import hudson.model.Result;
+import hudson.model.Run;
 import net.sf.json.JSONException;
 import net.sf.json.JSONObject;
 
@@ -59,15 +59,15 @@ public class BuildResultProcessor {
     private final String buildUrl;
     private final boolean runHarbormaster;
     private final FilePath workspace;
-    private final AbstractBuild build;
+    private final Run<?, ?> build;
     private String commentAction;
     private final CommentBuilder commenter;
     private UnitResults unitResults;
     private Map<String, String> harbormasterCoverage;
     private LintResults lintResults;
 
     public BuildResultProcessor(
-            Logger logger, AbstractBuild build, Differential diff, DifferentialClient diffClient,
+            Logger logger, Run<?, ?> build, FilePath workspace, Differential diff, DifferentialClient diffClient,
             String phid, CodeCoverageMetrics coverageResult, String buildUrl, boolean preserveFormatting,
             CoverageCheckSettings coverageCheckSettings) {
         this.logger = logger;
@@ -76,7 +76,7 @@ public BuildResultProcessor(
         this.phid = phid;
         this.buildUrl = buildUrl;
         this.build = build;
-        this.workspace = build.getWorkspace();
+        this.workspace = workspace;
 
         this.commentAction = "none";
         this.commenter = new CommentBuilder(logger, build.getResult(), coverageResult, buildUrl, preserveFormatting,
@@ -85,7 +85,11 @@ public BuildResultProcessor(
     }
 
     public Result getBuildResult() {
-        return this.build.getResult();
+        if (this.build.getResult() == null) {
+            return Result.SUCCESS;
+        } else {
+            return this.build.getResult();
+        }
     }
 
     /**

Diff for: src/main/java/com/uber/jenkins/phabricator/PhabricatorNotifier.java

@@ -35,17 +35,22 @@
 import com.uber.jenkins.phabricator.unit.UnitTestProvider;
 import com.uber.jenkins.phabricator.utils.CommonUtils;
 import com.uber.jenkins.phabricator.utils.Logger;
+
+import hudson.AbortException;
 import hudson.EnvVars;
+import hudson.FilePath;
 import hudson.Launcher;
-import hudson.model.AbstractBuild;
-import hudson.model.BuildListener;
 import hudson.model.Job;
 import hudson.model.Result;
+import hudson.model.Run;
+import hudson.model.TaskListener;
 import hudson.tasks.BuildStepMonitor;
 import hudson.tasks.Notifier;
 import jenkins.model.CauseOfInterruption;
 import jenkins.model.InterruptedBuildAction;
 import jenkins.model.Jenkins;
+import jenkins.tasks.SimpleBuildStep;
+
 import org.apache.commons.io.FilenameUtils;
 import org.kohsuke.stapler.DataBoundConstructor;
 
@@ -55,7 +60,7 @@
 import java.util.List;
 import java.util.Set;
 
-public class PhabricatorNotifier extends Notifier {
+public class PhabricatorNotifier extends Notifier implements SimpleBuildStep {
     public static final String COBERTURA_CLASS_NAME = "com.uber.jenkins.phabricator.coverage.CoberturaCoverageProvider";
 
     private static final String JUNIT_PLUGIN_NAME = "junit";
@@ -105,8 +110,8 @@ public BuildStepMonitor getRequiredMonitorService() {
     }
 
     @Override
-    public final boolean perform(final AbstractBuild<?, ?> build, final Launcher launcher,
-                                 final BuildListener listener) throws InterruptedException, IOException {
+    public final void perform(final Run<?, ?> build, FilePath workspace, final Launcher launcher,
+                                 final TaskListener listener) throws InterruptedException, IOException {
         EnvVars environment = build.getEnvironment(listener);
         Logger logger = new Logger(listener.getLogger());
 
@@ -128,7 +133,7 @@ public final boolean perform(final AbstractBuild<?, ?> build, final Launcher lau
                 if (cause instanceof PhabricatorCauseOfInterruption) {
                     logger.warn(ABORT_TAG, "Skipping notification step since this build was interrupted"
                             + " by a newer build with the same differential revision");
-                    return true;
+                    return;
                 }
             }
         }
@@ -144,7 +149,7 @@ public final boolean perform(final AbstractBuild<?, ?> build, final Launcher lau
                 build.addAction(PhabricatorPostbuildAction.createShortText(branch, null));
             }
 
-            coverageProvider = getCoverageProvider(build, listener, Collections.<String>emptySet());
+            coverageProvider = getCoverageProvider(build, workspace, listener, Collections.<String>emptySet());
             CodeCoverageMetrics coverageResult = null;
             if (coverageProvider != null) {
                 coverageResult = coverageProvider.getMetrics();
@@ -160,7 +165,7 @@ public final boolean perform(final AbstractBuild<?, ?> build, final Launcher lau
 
             // Ignore the result.
             nonDifferentialBuildTask.run();
-            return true;
+            return;
         }
 
         ConduitAPIClient conduitClient;
@@ -169,7 +174,7 @@ public final boolean perform(final AbstractBuild<?, ?> build, final Launcher lau
         } catch (ConduitAPIException e) {
             e.printStackTrace(logger.getStream());
             logger.warn(CONDUIT_TAG, e.getMessage());
-            return false;
+            throw new AbortException();
         }
 
         final String buildUrl = environment.get("BUILD_URL");
@@ -183,7 +188,11 @@ public final boolean perform(final AbstractBuild<?, ?> build, final Launcher lau
                     build.getResult(),
                     buildUrl
             ).run();
-            return result == Task.Result.SUCCESS;
+            if (result == Task.Result.SUCCESS) {
+                return;
+            } else {
+                throw new AbortException();
+            }
         }
 
         DifferentialClient diffClient = new DifferentialClient(diffID, conduitClient);
@@ -194,7 +203,7 @@ public final boolean perform(final AbstractBuild<?, ?> build, final Launcher lau
             e.printStackTrace(logger.getStream());
             logger.warn(CONDUIT_TAG, "Unable to fetch differential from Conduit API");
             logger.warn(CONDUIT_TAG, e.getMessage());
-            return true;
+            return;
         }
 
         if (needsDecoration) {
@@ -206,7 +215,7 @@ public final boolean perform(final AbstractBuild<?, ?> build, final Launcher lau
             includeFileNames.add(FilenameUtils.getName(file));
         }
 
-        coverageProvider = getCoverageProvider(build, listener, includeFileNames);
+        coverageProvider = getCoverageProvider(build, workspace, listener, includeFileNames);
         CodeCoverageMetrics coverageResult = null;
         if (coverageProvider != null) {
             coverageResult = coverageProvider.getMetrics();
@@ -215,6 +224,7 @@ public final boolean perform(final AbstractBuild<?, ?> build, final Launcher lau
         BuildResultProcessor resultProcessor = new BuildResultProcessor(
             logger,
             build,
+            workspace,
             diff,
             diffClient,
             environment.get(PhabricatorPlugin.PHID_FIELD),
@@ -247,14 +257,12 @@ public final boolean perform(final AbstractBuild<?, ?> build, final Launcher lau
 
         // Fail the build if we can't report to Harbormaster
         if (!resultProcessor.processHarbormaster()) {
-            return false;
+            throw new AbortException();
         }
 
         resultProcessor.processRemoteComment(commentFile, commentSize);
 
         resultProcessor.sendComment(commentWithConsoleLinkOnFailure);
-
-        return true;
     }
 
     protected UberallsClient getUberallsClient(Logger logger, String gitUrl, String branch) {
@@ -291,9 +299,15 @@ private ConduitAPIClient getConduitClient(Job owner) throws ConduitAPIException
      * @param listener The build listener
      * @return The current cobertura coverage, if any
      */
-    private CoverageProvider getCoverageProvider(AbstractBuild build, BuildListener listener,
+    private CoverageProvider getCoverageProvider(Run<?, ?> build, FilePath workspace, TaskListener listener,
                                                  Set<String> includeFileNames) {
-        if (!build.getResult().isBetterOrEqualTo(Result.UNSTABLE)) {
+        Result buildResult = null;
+        if (build.getResult() == null) {
+            buildResult = Result.SUCCESS;
+        } else {
+            buildResult = build.getResult();
+        }
+        if (!buildResult.isBetterOrEqualTo(Result.UNSTABLE)) {
             return null;
         }
 
@@ -311,6 +325,7 @@ private CoverageProvider getCoverageProvider(AbstractBuild build, BuildListener
         }
 
         coverage.setBuild(build);
+        coverage.setWorkspace(workspace);
         coverage.setIncludeFileNames(includeFileNames);
         coverage.setCoverageReportPattern(coverageReportPattern);
         if (coverage.hasCoverage()) {
@@ -321,7 +336,7 @@ private CoverageProvider getCoverageProvider(AbstractBuild build, BuildListener
         }
     }
 
-    private UnitTestProvider getUnitProvider(AbstractBuild build, BuildListener listener) {
+    private UnitTestProvider getUnitProvider(Run<?, ?> build, TaskListener listener) {
         Logger logger = new Logger(listener.getLogger());
 
         InstanceProvider<UnitTestProvider> provider = new InstanceProvider<UnitTestProvider>(

Diff for: src/main/java/com/uber/jenkins/phabricator/conduit/Differential.java

@@ -23,18 +23,14 @@
 import com.uber.jenkins.phabricator.PhabricatorPostbuildAction;
 import com.uber.jenkins.phabricator.PhabricatorPostbuildSummaryAction;
 
-import hudson.EnvVars;
-import hudson.model.AbstractBuild;
-
+import hudson.model.Run;
 import net.sf.json.JSONArray;
 import net.sf.json.JSONNull;
 import net.sf.json.JSONObject;
 
 import java.net.MalformedURLException;
 import java.net.URL;
-import java.util.ArrayList;
 import java.util.HashSet;
-import java.util.List;
 import java.util.Set;
 
 public class Differential {
@@ -78,7 +74,7 @@ public String getPhabricatorLink(String phabricatorURL) {
         }
     }
 
-    public void decorate(AbstractBuild build, String phabricatorURL) {
+    public void decorate(Run<?, ?> build, String phabricatorURL) {
         // Add a badge next to the build
         build.addAction(PhabricatorPostbuildAction.createShortText(
                 getRevisionID(true),

Diff for: src/main/java/com/uber/jenkins/phabricator/coverage/CoberturaCoverageProvider.java

@@ -38,6 +38,7 @@
 
 import hudson.FilePath;
 import hudson.model.AbstractBuild;
+import hudson.model.Run;
 import hudson.plugins.cobertura.Ratio;
 import hudson.plugins.cobertura.targets.CoverageMetric;
 import hudson.plugins.cobertura.targets.CoverageResult;
@@ -96,7 +97,8 @@ Map<String, List<Integer>> parseReports(CoberturaXMLParser parser, File[] report
     }
 
     private void computeCoverage() {
-        AbstractBuild build = getBuild();
+        Run<?, ?> build = getBuild();
+        FilePath workspace = getWorkspace();
         if (build == null) {
             mHasComputedCoverage = true;
             return;
@@ -114,7 +116,7 @@ private void computeCoverage() {
         }
 
         // Fallback to scanning for the reports
-        copyCoverageToJenkinsMaster(build);
+        copyCoverageToJenkinsMaster(build, workspace);
         File[] reports = getCoberturaReports(build);
         CoverageResult result = null;
         if (reports != null) {
@@ -142,17 +144,14 @@ private void computeCoverage() {
     }
 
     private void computeLineCoverage() {
-        FilePath workspace = getBuild().getWorkspace();
+        FilePath workspace = getWorkspace();
         File[] reports = getCoberturaReports(getBuild());
         CoberturaXMLParser parser = new CoberturaXMLParser(workspace, getIncludeFileNames());
         mLineCoverage = parseReports(parser, reports);
     }
 
-    private void copyCoverageToJenkinsMaster(AbstractBuild build) {
-        final FilePath[] moduleRoots = build.getModuleRoots();
-        final boolean multipleModuleRoots =
-                moduleRoots != null && moduleRoots.length > 1;
-        final FilePath moduleRoot = multipleModuleRoots ? build.getWorkspace() : build.getModuleRoot();
+    private void copyCoverageToJenkinsMaster(Run<?, ?> build, FilePath workspace) {
+        final FilePath moduleRoot = workspace;
         final File buildCoberturaDir = build.getRootDir();
         FilePath buildTarget = new FilePath(buildCoberturaDir);
 
@@ -225,7 +224,7 @@ private static float getCoveragePercentage(CoverageResult result, CoverageMetric
         return ratio.getPercentageFloat();
     }
 
-    private File[] getCoberturaReports(AbstractBuild build) {
+    private File[] getCoberturaReports(Run<?, ?> build) {
         return build.getRootDir().listFiles(CoberturaPublisher.COBERTURA_FILENAME_FILTER);
     }
 }

Diff for: src/main/java/com/uber/jenkins/phabricator/coverage/CoverageProvider.java

@@ -20,14 +20,16 @@
 
 package com.uber.jenkins.phabricator.coverage;
 
-import hudson.model.AbstractBuild;
+import hudson.FilePath;
+import hudson.model.Run;
 
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
 public abstract class CoverageProvider {
-    private AbstractBuild<?, ?> build;
+    private Run<?, ?> build;
+    private FilePath workspace;
     private Set<String> includeFileNames;
     private String coverageReportPattern;
 
@@ -47,14 +49,22 @@ protected Set<String> getIncludeFileNames() {
      * Set the owning build for this provider
      * @param build The build that is associated with the current run
      */
-    public void setBuild(AbstractBuild<?, ?> build) {
+    public void setBuild(Run<?, ?> build) {
         this.build = build;
     }
 
-    protected AbstractBuild getBuild() {
+    protected Run<?, ?> getBuild() {
         return build;
     }
 
+    public void setWorkspace(FilePath workspace) {
+        this.workspace = workspace;
+    }
+
+    protected FilePath getWorkspace() {
+        return workspace;
+    }
+
     /**
      * Set the coverage report pattern to scan for
      * @param coverageReportPattern The coverage report pattern to scan for