Skip to content
This repository has been archived by the owner on Oct 29, 2024. It is now read-only.

NonDifferentialHarbormasterTask doesn't record Uberalls coverage information #256

Open
maroux opened this issue Jun 21, 2018 · 5 comments
Open

Comments

@maroux
Copy link
Contributor

maroux commented Jun 21, 2018

We use 2 different jobs for one project - one for DIFF and one for CI.
The job for DIFF works fine and records coverage and posts as comment. However, the job for CI doesn't record coverage automatically because it runs NonDifferentialHarbormasterTask which doesn't have uberalls integrated. I think the fix is pretty simple - basically need to copy bits from NonDifferentialBuildTask - thoughts?

This way you don't need a cron job running coverage for master branch - instead it can just rely on the coverage information recorded from the CI build.

@maroux
Copy link
Contributor Author

maroux commented Jun 21, 2018

diff --git a/src/main/java/com/uber/jenkins/phabricator/PhabricatorNotifier.java b/src/main/java/com/uber/jenkins/phabricator/PhabricatorNotifier.java
index 2690414..0c809e7 100644
--- a/src/main/java/com/uber/jenkins/phabricator/PhabricatorNotifier.java
+++ b/src/main/java/com/uber/jenkins/phabricator/PhabricatorNotifier.java
@@ -144,7 +144,7 @@ public class PhabricatorNotifier extends Notifier implements SimpleBuildStep {
         // a Harbormaster build on a commit rather than a differential, but still wants build status.
         // If DIFF_ID is present but PHID is not, it means somebody is doing a Differential build without Harbormaster.
         // So only skip build result processing if both are blank (e.g. master runs to update coverage data)
-        if (CommonUtils.isBlank(phid) && !isDifferential) {
+        if (!isDifferential) {
             if (needsDecoration) {
                 build.addAction(PhabricatorPostbuildAction.createShortText(branch, null));
             }
@@ -165,7 +165,10 @@ public class PhabricatorNotifier extends Notifier implements SimpleBuildStep {
 
             // Ignore the result.
             nonDifferentialBuildTask.run();
-            return;
+            // if PHID is present, Harbormaster needs build status
+            if (CommonUtils.isBlank(phid)) {
+                return;
+            }
         }
 
         ConduitAPIClient conduitClient;

is a possible solution.

@aadis
Copy link

aadis commented Jun 27, 2018

Also, how do we call NonDifferentialHarbormasterTask from jenkins pipeline?

@maroux
Copy link
Contributor Author

maroux commented Jun 27, 2018

@aadis that task is triggered if you run a job with phid and no diff id (or revision id). For example, if you enable phab integration for CI builds.

@maroux
Copy link
Contributor Author

maroux commented Aug 21, 2018

Any change this will get fixed?

@aadis
Copy link

aadis commented Aug 21, 2018

What I realised by looking in the code is it only triggers for Stable builds. I wanted coverage even for unstable builds so removed that check.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants