Skip to content

Commit

Permalink
Send harbormaster failure on arc patch failure
Browse files Browse the repository at this point in the history
  • Loading branch information
ascandella committed Aug 12, 2015
1 parent aa30e77 commit 2a089b2
Show file tree
Hide file tree
Showing 4 changed files with 155 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import com.uber.jenkins.phabricator.conduit.DifferentialClient;
import com.uber.jenkins.phabricator.credentials.ConduitCredentials;
import com.uber.jenkins.phabricator.tasks.ApplyPatchTask;
import com.uber.jenkins.phabricator.tasks.SendHarbormasterResultTask;
import com.uber.jenkins.phabricator.tasks.Task;
import com.uber.jenkins.phabricator.utils.CommonUtils;
import com.uber.jenkins.phabricator.utils.Logger;
Expand Down Expand Up @@ -65,6 +66,7 @@ public Environment setUp(AbstractBuild build,
return this.ignoreBuild(logger, "No environment variables found?!");
}

String phid = environment.get(PhabricatorPlugin.PHID_FIELD);
String diffID = environment.get(PhabricatorPlugin.DIFFERENTIAL_ID_FIELD);
if (CommonUtils.isBlank(diffID)) {
this.addShortText(build);
Expand All @@ -83,9 +85,9 @@ public Environment setUp(AbstractBuild build,
return null;
}

DifferentialClient diffClient = new DifferentialClient(diffID, conduitClient);
Differential diff;
try {
DifferentialClient diffClient = new DifferentialClient(diffID, conduitClient);
diff = new Differential(diffClient.fetchDiff());
diff.decorate(build, this.getPhabricatorURL(build.getParent()));

Expand All @@ -97,7 +99,7 @@ public Environment setUp(AbstractBuild build,
}
} catch (ConduitAPIException e) {
e.printStackTrace(logger.getStream());
logger.warn(CONDUIT_TAG, "Unable to apply patch");
logger.warn(CONDUIT_TAG, "Unable to fetch differential from Conduit API");
logger.warn(CONDUIT_TAG, e.getMessage());
return null;
}
Expand All @@ -115,6 +117,12 @@ logger, starter, baseCommit, diffID, 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).run();
if (failureResult != Task.Result.SUCCESS) {
// such failure, very broke
logger.warn("arcanist", "Unable to notify harbormaster of patch failure");
}
// Indicate failure
return null;
}

Expand Down
25 changes: 12 additions & 13 deletions src/main/java/com/uber/jenkins/phabricator/PhabricatorNotifier.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,15 @@

package com.uber.jenkins.phabricator;

import com.uber.jenkins.phabricator.conduit.*;
import com.uber.jenkins.phabricator.conduit.ConduitAPIClient;
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.credentials.ConduitCredentials;
import com.uber.jenkins.phabricator.tasks.NonDifferentialBuildTask;
import com.uber.jenkins.phabricator.tasks.PostCommentTask;
import com.uber.jenkins.phabricator.tasks.SendHarbormasterResultTask;
import com.uber.jenkins.phabricator.tasks.Task;
import com.uber.jenkins.phabricator.uberalls.UberallsClient;
import com.uber.jenkins.phabricator.utils.CommonUtils;
import com.uber.jenkins.phabricator.utils.Logger;
Expand All @@ -38,8 +43,6 @@
import hudson.plugins.cobertura.targets.CoverageResult;
import hudson.tasks.BuildStepMonitor;
import hudson.tasks.Notifier;
import net.sf.json.JSONNull;
import net.sf.json.JSONObject;
import org.kohsuke.stapler.DataBoundConstructor;

import java.io.IOException;
Expand Down Expand Up @@ -156,16 +159,12 @@ public final boolean perform(final AbstractBuild<?, ?> build, final Launcher lau
String commentAction = "none";
if (runHarbormaster) {
logger.info("harbormaster", "Sending build result to Harbormaster with PHID '" + phid + "', success: " + harbormasterSuccess);
try {
JSONObject result = diffClient.sendHarbormasterMessage(phid, harbormasterSuccess);
if (result.containsKey("error_info") && !(result.get("error_info") instanceof JSONNull)) {
logger.info("harbormaster",
String.format("Error from Harbormaster: %s", result.getString("error_info")));
return false;
}
} catch (ConduitAPIException e) {
logger.info(CONDUIT_TAG, "unable to post to harbormaster");
return true;

Task.Result result = new SendHarbormasterResultTask(logger, diffClient, phid, harbormasterSuccess).run();

if (result != Task.Result.SUCCESS) {
logger.info(CONDUIT_TAG, "Unable to post to harbormaster");
return false;
}
} else {
logger.info("uberalls", "Harbormaster integration not enabled for this build.");
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
package com.uber.jenkins.phabricator.tasks;

import com.uber.jenkins.phabricator.conduit.ConduitAPIException;
import com.uber.jenkins.phabricator.conduit.DifferentialClient;
import com.uber.jenkins.phabricator.utils.Logger;
import net.sf.json.JSONNull;
import net.sf.json.JSONObject;

import java.io.IOException;

public class SendHarbormasterResultTask extends Task {

private final DifferentialClient diffClient;
private final String phid;
private final boolean harbormasterSuccess;

public SendHarbormasterResultTask(Logger logger, DifferentialClient diffClient, String phid, boolean harbormasterSuccess) {
super(logger);
this.diffClient = diffClient;
this.phid = phid;
this.harbormasterSuccess = harbormasterSuccess;
}

/**
* {@inheritDoc}
*/
@Override
protected String getTag() {
return "send-harbormaster-result";
}

/**
* {@inheritDoc}
*/
@Override
protected void setup() {
// Do nothing
}

/**
* {@inheritDoc}
*/
@Override
protected void execute() {
try {
JSONObject result = diffClient.sendHarbormasterMessage(phid, harbormasterSuccess);

if (result.containsKey("error_info") && !(result.get("error_info") instanceof JSONNull)) {
info(String.format("Error from Harbormaster: %s", result.getString("error_info")));
this.result = Result.FAILURE;
} else {
this.result = Result.SUCCESS;
}
} catch (ConduitAPIException e) {
e.printStackTrace();
this.result = Result.FAILURE;
} catch (IOException e) {
e.printStackTrace();
this.result = Result.FAILURE;
}
}

/**
* {@inheritDoc}
*/
@Override
protected void tearDown() {
// Do nothing
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
package com.uber.jenkins.phabricator.tasks;

import com.uber.jenkins.phabricator.conduit.ConduitAPIException;
import com.uber.jenkins.phabricator.conduit.DifferentialClient;
import com.uber.jenkins.phabricator.utils.TestUtils;
import net.sf.json.JSONObject;
import org.junit.Before;
import org.junit.Test;

import java.io.IOException;

import static org.junit.Assert.assertEquals;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

public class SendHarbormasterResultTaskTest {
private final JSONObject validResponse = new JSONObject();
private DifferentialClient diffClient;

@Before
public void setUp() {
diffClient = mock(DifferentialClient.class);
}

@Test
public void testSuccessfulHarbormaster() throws IOException, ConduitAPIException {
when(diffClient.sendHarbormasterMessage(TestUtils.TEST_PHID, false)).thenReturn(validResponse);

assertEquals(Task.Result.SUCCESS, getResult());
}

@Test
public void testErrorInfoResponse() throws IOException, ConduitAPIException {
JSONObject errorResponse = new JSONObject();
errorResponse.put("error_info", "i'm having a bad day");
when(diffClient.sendHarbormasterMessage(TestUtils.TEST_PHID, false)).thenReturn(errorResponse);

assertEquals(Task.Result.FAILURE, getResult());
}

@Test
public void testConduitAPIFailure() throws IOException, ConduitAPIException {
when(diffClient.sendHarbormasterMessage(TestUtils.TEST_PHID, false)).thenThrow(ConduitAPIException.class);

assertEquals(Task.Result.FAILURE, getResult());
}

@Test
public void testIOExceptionFailure() throws IOException, ConduitAPIException {
when(diffClient.sendHarbormasterMessage(TestUtils.TEST_PHID, false)).thenThrow(IOException.class);

assertEquals(Task.Result.FAILURE, getResult());
}

private Task.Result getResult() {
return new SendHarbormasterResultTask(
TestUtils.getDefaultLogger(),
diffClient,
TestUtils.TEST_PHID,
false
).run();
}
}

0 comments on commit 2a089b2

Please sign in to comment.