Skip to content

Commit

Permalink
Add sendPartialResults option to report work is ongoing to Harbormast…
Browse files Browse the repository at this point in the history
…er instead of pass/fail
  • Loading branch information
Greg Magolan committed Jul 8, 2020
1 parent e77a51b commit 70eb11d
Show file tree
Hide file tree
Showing 13 changed files with 121 additions and 37 deletions.
Expand Up @@ -22,6 +22,7 @@

import com.uber.jenkins.phabricator.conduit.Differential;
import com.uber.jenkins.phabricator.conduit.DifferentialClient;
import com.uber.jenkins.phabricator.conduit.HarbormasterClient.MessageType;
import com.uber.jenkins.phabricator.coverage.CodeCoverageMetrics;
import com.uber.jenkins.phabricator.coverage.CoverageConverter;
import com.uber.jenkins.phabricator.coverage.CoverageProvider;
Expand Down Expand Up @@ -203,8 +204,10 @@ public void sendComment(boolean commentWithConsoleLinkOnFailure) {
*
* @return whether we were able to successfully send the result
*/
public boolean processHarbormaster() {
final boolean harbormasterSuccess = getBuildResult().isBetterOrEqualTo(Result.SUCCESS);
public boolean processHarbormaster(boolean sendPartialResults) {
final MessageType messageType = sendPartialResults ?
MessageType.work :
(getBuildResult().isBetterOrEqualTo(Result.SUCCESS) ? MessageType.pass : MessageType.fail);

if (runHarbormaster) {
logger.info("harbormaster", "Sending Harbormaster BUILD_URL via PHID: " + phid);
Expand Down Expand Up @@ -239,17 +242,17 @@ public boolean processHarbormaster() {

logger.info(
LOGGING_TAG,
String.format("Sending build result to Harbormaster with PHID %s, success: %s",
String.format("Sending build result to Harbormaster with PHID %s, message type: %s",
phid,
harbormasterSuccess
messageType.name()
)
);

Task.Result result = new SendHarbormasterResultTask(
logger,
diffClient,
phid,
harbormasterSuccess,
messageType,
unitResults,
harbormasterCoverage,
lintResults
Expand Down
Expand Up @@ -25,6 +25,7 @@
import com.uber.jenkins.phabricator.conduit.ConduitAPIException;
import com.uber.jenkins.phabricator.conduit.Differential;
import com.uber.jenkins.phabricator.conduit.DifferentialClient;
import com.uber.jenkins.phabricator.conduit.HarbormasterClient.MessageType;
import com.uber.jenkins.phabricator.credentials.ConduitCredentials;
import com.uber.jenkins.phabricator.tasks.ApplyPatchTask;
import com.uber.jenkins.phabricator.tasks.SendHarbormasterResultTask;
Expand Down Expand Up @@ -212,7 +213,7 @@ logger, starter, baseCommit, diffID, conduitUrl, conduitToken, getArcPath(),

if (result != Task.Result.SUCCESS) {
logger.warn("arcanist", "Error applying arc patch; got non-zero exit code " + result);
Task.Result failureResult = new SendHarbormasterResultTask(logger, diffClient, phid, false, null, null,
Task.Result failureResult = new SendHarbormasterResultTask(logger, diffClient, phid, MessageType.fail, null, null,
null).run();
if (failureResult != Task.Result.SUCCESS) {
// such failure, very broke
Expand Down
Expand Up @@ -90,6 +90,7 @@ public class PhabricatorNotifier extends Notifier implements SimpleBuildStep {
private final String lintFile;
private final String lintFileSize;
private final String coverageReportPattern;
private final boolean sendPartialResults;
private transient UberallsClient uberallsClient;

// Fields in config.jelly must match the parameter names in the "DataBoundConstructor"
Expand All @@ -99,7 +100,7 @@ public PhabricatorNotifier(
double coverageThreshold, double minCoverageThreshold, String coverageReportPattern,
boolean preserveFormatting, String commentFile, String commentSize,
boolean commentWithConsoleLinkOnFailure, boolean customComment, boolean processLint,
String lintFile, String lintFileSize) {
String lintFile, String lintFileSize, boolean sendPartialResults) {
this.commentOnSuccess = commentOnSuccess;
this.uberallsEnabled = uberallsEnabled;
this.coverageCheckSettings = new CoverageCheckSettings(coverageCheck, coverageThreshold, minCoverageThreshold);
Expand All @@ -112,6 +113,7 @@ public PhabricatorNotifier(
this.customComment = customComment;
this.processLint = processLint;
this.coverageReportPattern = coverageReportPattern;
this.sendPartialResults = sendPartialResults;
}

public BuildStepMonitor getRequiredMonitorService() {
Expand Down Expand Up @@ -216,7 +218,8 @@ public final void perform(
phid,
conduitClient,
buildResult,
buildUrl
buildUrl,
this.sendPartialResults
).run();
if (result == Task.Result.SUCCESS) {
return;
Expand Down Expand Up @@ -283,7 +286,7 @@ public final void perform(
}

// Fail the build if we can't report to Harbormaster
if (!resultProcessor.processHarbormaster()) {
if (!resultProcessor.processHarbormaster(this.sendPartialResults)) {
throw new AbortException();
}

Expand Down Expand Up @@ -499,6 +502,11 @@ public String getLintFileSize() {
return lintFileSize;
}

@SuppressWarnings("UnusedDeclaration")
public boolean isSendPartialResults() {
return sendPartialResults;
}

private ConduitCredentials getConduitCredentials(Job owner) {
return getDescriptor().getCredentials(owner);
}
Expand Down
Expand Up @@ -20,6 +20,7 @@

package com.uber.jenkins.phabricator.conduit;

import com.uber.jenkins.phabricator.conduit.HarbormasterClient.MessageType;
import com.uber.jenkins.phabricator.lint.LintResults;
import com.uber.jenkins.phabricator.unit.UnitResults;

Expand Down Expand Up @@ -99,18 +100,18 @@ public JSONObject fetchDiff() throws IOException, ConduitAPIException {
* Sets a sendHarbormasterMessage build status
*
* @param phid Phabricator object ID
* @param pass whether or not the build passed
* @param messageType type of message to send; either 'pass', 'fail' or 'work'
* @param unitResults the results from the unit tests
* @param coverage the results from the coverage provider
* @return the Conduit API response
* @throws IOException if there is a network error talking to Conduit
* @throws ConduitAPIException if any error is experienced talking to Conduit
*/
public JSONObject sendHarbormasterMessage(
String phid, boolean pass, UnitResults unitResults,
String phid, MessageType messageType, UnitResults unitResults,
Map<String, String> coverage,
LintResults lintResults) throws ConduitAPIException, IOException {
return new HarbormasterClient(conduit).sendHarbormasterMessage(phid, pass, unitResults, coverage, lintResults);
return new HarbormasterClient(conduit).sendHarbormasterMessage(phid, messageType, unitResults, coverage, lintResults);
}

/**
Expand Down
Expand Up @@ -12,6 +12,15 @@

public class HarbormasterClient {

/**
* See https://secure.phabricator.com/conduit/method/harbormaster.sendmessage/
*/
public enum MessageType {
pass,
fail,
work,
}

private final ConduitAPIClient conduit;

public HarbormasterClient(ConduitAPIClient conduit) {
Expand All @@ -22,7 +31,7 @@ public HarbormasterClient(ConduitAPIClient conduit) {
* Sets a sendHarbormasterMessage build status
*
* @param phid Phabricator object ID
* @param pass whether or not the build passed
* @param messageType type of message to send; either 'pass', 'fail' or 'work'
* @param unitResults the results from the unit tests
* @param coverage the results from the coverage provider
* @param lintResults
Expand All @@ -32,7 +41,7 @@ public HarbormasterClient(ConduitAPIClient conduit) {
*/
public JSONObject sendHarbormasterMessage(
String phid,
boolean pass,
MessageType messageType,
UnitResults unitResults,
Map<String, String> coverage,
LintResults lintResults) throws ConduitAPIException, IOException {
Expand All @@ -58,7 +67,7 @@ public JSONObject sendHarbormasterMessage(
}

JSONObject params = new JSONObject();
params.element("type", pass ? "pass" : "fail")
params.element("type", messageType.name())
.element("buildTargetPHID", phid);

if (!unit.isEmpty()) {
Expand Down
Expand Up @@ -23,6 +23,7 @@
import com.uber.jenkins.phabricator.conduit.ConduitAPIClient;
import com.uber.jenkins.phabricator.conduit.ConduitAPIException;
import com.uber.jenkins.phabricator.conduit.HarbormasterClient;
import com.uber.jenkins.phabricator.conduit.HarbormasterClient.MessageType;
import com.uber.jenkins.phabricator.utils.Logger;

import java.io.IOException;
Expand All @@ -35,27 +36,32 @@ public class NonDifferentialHarbormasterTask extends Task {
private final String buildUrl;
private final HarbormasterClient harbormaster;
private final Logger logger;
private final boolean sendPartialResults;

/**
* Task constructor.
*
* @param logger The logger where logs go to.
* @param phid Phabricator object ID
* @param conduitClient
* @param result
* @param buildUrl
* @param sendPartialResults Send a 'work' message type instead of 'pass'/'fail' if true.
*/
public NonDifferentialHarbormasterTask(
Logger logger,
String phid,
ConduitAPIClient conduitClient,
hudson.model.Result result,
String buildUrl) {
String buildUrl,
boolean sendPartialResults) {
super(logger);
this.logger = logger;
this.phid = phid;
this.conduit = conduitClient;
this.buildResult = result;
this.buildUrl = buildUrl;
this.sendPartialResults = sendPartialResults;

this.harbormaster = new HarbormasterClient(conduit);
}
Expand All @@ -81,13 +87,15 @@ protected void setup() {
*/
@Override
protected void execute() {
final boolean pass = buildResult.isBetterOrEqualTo(hudson.model.Result.SUCCESS);
info(String.format("Sending diffusion result as: %s", buildResult.toString()));
final MessageType messageType = this.sendPartialResults ?
MessageType.work :
(buildResult.isBetterOrEqualTo(hudson.model.Result.SUCCESS) ? MessageType.pass : MessageType.fail);
info(String.format("Sending diffusion result as: %s, message type: %s", buildResult.toString(), messageType.name()));

try {
harbormaster.sendHarbormasterUri(phid, buildUrl);
// Only send pass/fail, since coverage and unit aren't viewable outside of differentials
harbormaster.sendHarbormasterMessage(phid, pass, null, null, null);
harbormaster.sendHarbormasterMessage(phid, messageType, null, null, null);
result = Result.SUCCESS;
return;
} catch (ConduitAPIException e) {
Expand Down
Expand Up @@ -2,6 +2,7 @@

import com.uber.jenkins.phabricator.conduit.ConduitAPIException;
import com.uber.jenkins.phabricator.conduit.DifferentialClient;
import com.uber.jenkins.phabricator.conduit.HarbormasterClient.MessageType;
import com.uber.jenkins.phabricator.lint.LintResults;
import com.uber.jenkins.phabricator.unit.UnitResults;
import com.uber.jenkins.phabricator.utils.Logger;
Expand All @@ -16,20 +17,20 @@ public class SendHarbormasterResultTask extends Task {

private final DifferentialClient diffClient;
private final String phid;
private final boolean harbormasterSuccess;
private final MessageType messageType;
private final Map<String, String> coverage;
private final LintResults lintResults;
private UnitResults unitResults;

public SendHarbormasterResultTask(
Logger logger, DifferentialClient diffClient, String phid,
boolean harbormasterSuccess, UnitResults unitResults,
MessageType messageType, UnitResults unitResults,
Map<String, String> harbormasterCoverage,
LintResults lintResults) {
super(logger);
this.diffClient = diffClient;
this.phid = phid;
this.harbormasterSuccess = harbormasterSuccess;
this.messageType = messageType;
this.unitResults = unitResults;
this.coverage = harbormasterCoverage;
this.lintResults = lintResults;
Expand Down Expand Up @@ -87,7 +88,7 @@ protected void tearDown() {
*/
private boolean sendMessage(UnitResults unitResults, Map<String, String> coverage, LintResults lintResults) throws
IOException, ConduitAPIException {
JSONObject result = diffClient.sendHarbormasterMessage(phid, harbormasterSuccess, unitResults, coverage,
JSONObject result = diffClient.sendHarbormasterMessage(phid, messageType, unitResults, coverage,
lintResults);

if (result.containsKey("error_info") && !(result.get("error_info") instanceof JSONNull)) {
Expand Down
Expand Up @@ -61,4 +61,9 @@
<f:textbox default="100000" />
</f:entry>
</f:optionalBlock>

<f:entry title="Send partial results" field="sendPartialResults"
description="Report to Harbormaster that work is ongoing instead of reporting pass/fail. Pass/fail must be reported outside of this job. For example, if this is a child job, the parent job should report the final pass/fail status to Harbormaster once all child jobs complete. If you have multiple Jenkins jobs triggered by the same harbormaster buildable, use this to avoid posting multiple (possibly conflicting) pass/fail statuses.">
<f:checkbox default="false" />
</f:entry>
</j:jelly>
Expand Up @@ -183,7 +183,7 @@ private BuildResultProcessor runProcessLintViolationsTest(String lintFileContent
new CoverageCheckSettings(true, 0.0, 100.0)
);
processor.processLintResults(fileName, "1000");
processor.processHarbormaster();
processor.processHarbormaster(false);
return processor;
}

Expand Down

0 comments on commit 70eb11d

Please sign in to comment.