Skip to content

Commit

Permalink
Merge 16489a6 into fd92537
Browse files Browse the repository at this point in the history
  • Loading branch information
@na-Itms
na-Itms committed Dec 17, 2018
2 parents fd92537 + 16489a6 commit 855d620
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 35 deletions.
6 changes: 4 additions & 2 deletions src/main/java/com/uber/jenkins/phabricator/BuildResultProcessor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,12 +81,14 @@ public BuildResultProcessor(
this.workspace = workspace;

this.commentAction = "none";
this.commenter = new CommentBuilder(logger, build.getResult(), coverageResult, buildUrl, preserveFormatting,
this.commenter = new CommentBuilder(logger, coverageResult, buildUrl, preserveFormatting,
coverageCheckSettings);
this.runHarbormaster = !CommonUtils.isBlank(phid);
}

public Result getBuildResult() {
// In Pipeline jobs, as long as no failure happens, the build status stays null.
// The PhabricatorNotifier needs to interpret null as "not failed (yet)".
if (this.build.getResult() == null) {
return Result.SUCCESS;
} else {
Expand Down Expand Up @@ -123,7 +125,7 @@ public boolean processParentCoverage(UberallsClient uberalls) {
* @param commentWithConsoleLinkOnFailure whether a failure should trigger a console link
*/
public void processBuildResult(boolean commentOnSuccess, boolean commentWithConsoleLinkOnFailure) {
commenter.processBuildResult(commentOnSuccess, commentWithConsoleLinkOnFailure, runHarbormaster);
commenter.processBuildResult(getBuildResult(), commentOnSuccess, commentWithConsoleLinkOnFailure, runHarbormaster);
}

/**
Expand Down
5 changes: 2 additions & 3 deletions src/main/java/com/uber/jenkins/phabricator/CommentBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,15 +33,13 @@ class CommentBuilder {
private final CodeCoverageMetrics currentCoverage;
private final StringBuilder comment;
private final String buildURL;
private final Result result;
private final boolean preserveFormatting;
private final CoverageCheckSettings coverageCheckSettings;

CommentBuilder(
Logger logger, Result result, CodeCoverageMetrics currentCoverage, String buildURL,
Logger logger, CodeCoverageMetrics currentCoverage, String buildURL,
boolean preserveFormatting, CoverageCheckSettings coverageCheckSettings) {
this.logger = logger;
this.result = result;
this.currentCoverage = currentCoverage;
this.buildURL = buildURL;
this.preserveFormatting = preserveFormatting;
Expand Down Expand Up @@ -130,6 +128,7 @@ private boolean isBuildFailingCoverageCheck(double lineCoveragePercent, double c
}

void processBuildResult(
Result result,
boolean commentOnSuccess,
boolean commentWithConsoleLinkOnFailure,
boolean runHarbormaster) {
Expand Down
10 changes: 9 additions & 1 deletion src/main/java/com/uber/jenkins/phabricator/PhabricatorNotifier.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -202,12 +202,20 @@ public final void perform(
final String buildUrl = whichBuildUrl;

if (!isDifferential) {
Result buildResult;
// In Pipeline jobs, as long as no failure happens, the build status stays null.
// The PhabricatorNotifier needs to interpret null as "not failed (yet)".
if (build.getResult() == null) {
buildResult = Result.SUCCESS;
} else {
buildResult = build.getResult();
}
// Process harbormaster for non-differential builds
Task.Result result = new NonDifferentialHarbormasterTask(
logger,
phid,
conduitClient,
build.getResult(),
buildResult,
buildUrl
).run();
if (result == Task.Result.SUCCESS) {
Expand Down
57 changes: 28 additions & 29 deletions src/test/java/com/uber/jenkins/phabricator/CommentBuilderTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ public class CommentBuilderTest {

@Before
public void setUp() {
commenter = createCommenter(Result.SUCCESS, TestUtils.getDefaultCodeCoverageMetrics());
commenter = createCommenter(TestUtils.getDefaultCodeCoverageMetrics());
}

@Test
Expand All @@ -55,13 +55,13 @@ public void testHasCoverageAvailable() {

@Test
public void testHasNoCoverageAvailable() {
CommentBuilder commenter = createCommenter(Result.SUCCESS, null);
CommentBuilder commenter = createCommenter(null);
assertFalse(commenter.hasCoverageAvailable());
}

@Test
public void testHasNoLineCoverage() {
CommentBuilder commenter = createCommenter(Result.SUCCESS, TestUtils.getEmptyCoverageMetrics());
CommentBuilder commenter = createCommenter(TestUtils.getEmptyCoverageMetrics());
assertFalse(commenter.hasCoverageAvailable());
}

Expand Down Expand Up @@ -105,7 +105,7 @@ public void testProcessParentWithIncreasedCoverage() {
@Test
public void testProcessWithDecreaseFailingTheBuild() {
CodeCoverageMetrics fiftyPercentDrop = TestUtils.getCoverageResult(100.0f, 100.0f, 100.0f, 100.0f, 50.0f, 50, 100);
CommentBuilder commenter = createCommenter(Result.SUCCESS, fiftyPercentDrop, false, -10.0f);
CommentBuilder commenter = createCommenter(fiftyPercentDrop, false, -10.0f);
boolean passCoverage = commenter.processParentCoverage(TestUtils.getDefaultCodeCoverageMetrics(),
TestUtils.TEST_SHA, FAKE_BRANCH_NAME);
String comment = commenter.getComment();
Expand All @@ -119,7 +119,7 @@ public void testProcessWithDecreaseFailingTheBuild() {
@Test
public void testProcessWithDecreaseNotFailingTheBuild() {
CodeCoverageMetrics fivePercentDrop = TestUtils.getCoverageResult(100.0f, 100.0f, 100.0f, 100.0f, 95.0f, 95, 100);
CommentBuilder commenter = createCommenter(Result.SUCCESS, fivePercentDrop, false, -10.0f);
CommentBuilder commenter = createCommenter(fivePercentDrop, false, -10.0f);
boolean passCoverage = commenter.processParentCoverage(TestUtils.getDefaultCodeCoverageMetrics(),
TestUtils.TEST_SHA, FAKE_BRANCH_NAME);
String comment = commenter.getComment();
Expand All @@ -133,7 +133,7 @@ public void testProcessWithDecreaseNotFailingTheBuild() {
@Test
public void testProcessWithDecreaseButHigherThanMinNotFailingTheBuild() {
CodeCoverageMetrics fifteenPercentDrop = TestUtils.getCoverageResult(100.0f, 100.0f, 100.0f, 100.0f, 85.0f, 85, 100);
CommentBuilder commenter = createCommenter(Result.SUCCESS, fifteenPercentDrop, false, -10.0f, 80.0f);
CommentBuilder commenter = createCommenter(fifteenPercentDrop, false, -10.0f, 80.0f);
boolean passCoverage = commenter.processParentCoverage(TestUtils.getDefaultCodeCoverageMetrics(),
TestUtils.TEST_SHA, FAKE_BRANCH_NAME);
String comment = commenter.getComment();
Expand All @@ -148,7 +148,6 @@ public void testProcessWithDecreaseButHigherThanMinNotFailingTheBuild() {
public void testProcessWithoutCoverageCheckSettings() {
CommentBuilder commenter = new CommentBuilder(
logger,
Result.SUCCESS,
TestUtils.getCoverageResult(100.0f, 100.0f, 100.0f, 100.0f, 50.0f, 50, 100), // 50% drop
FAKE_BUILD_URL,
false,
Expand All @@ -167,7 +166,7 @@ public void testProcessWithoutCoverageCheckSettings() {

@Test
public void testProcessBuildResultSuccess() {
commenter.processBuildResult(false, false, true);
commenter.processBuildResult(Result.SUCCESS, false, false, true);
assertTrue(
"no message expected for successful builds unless asked for",
commenter.getComment().length() == 0
Expand All @@ -176,42 +175,42 @@ public void testProcessBuildResultSuccess() {

@Test
public void testProcessBuildResultSuccessWithComment() {
commenter.processBuildResult(true, false, false);
commenter.processBuildResult(Result.SUCCESS, true, false, false);
assertEquals("Build is green", commenter.getComment());
}

@Test
public void testProcessBuildResultUnstable() {
CommentBuilder commenter = createCommenter(Result.UNSTABLE, null);
commenter.processBuildResult(true, false, false);
CommentBuilder commenter = createCommenter(null);
commenter.processBuildResult(Result.UNSTABLE, true, false, false);
assertEquals("Build is unstable", commenter.getComment());
}

@Test
public void testProcessBuildResultUnknownStatus() {
CommentBuilder commenter = createCommenter(Result.NOT_BUILT, null);
commenter.processBuildResult(true, false, false);
CommentBuilder commenter = createCommenter(null);
commenter.processBuildResult(Result.NOT_BUILT, true, false, false);
assertFalse(commenter.hasComment());
}

@Test
public void testProcessBuildResultWithFailureMessage() {
CommentBuilder commenter = createCommenter(Result.FAILURE, null);
commenter.processBuildResult(false, true, false);
CommentBuilder commenter = createCommenter(null);
commenter.processBuildResult(Result.FAILURE, false, true, false);
assertEquals("Build has FAILED", commenter.getComment());
}

@Test
public void testProcessBuildResultWithoutFailureMessage() {
CommentBuilder commenter = createCommenter(Result.FAILURE, null);
commenter.processBuildResult(false, false, true);
CommentBuilder commenter = createCommenter(null);
commenter.processBuildResult(Result.FAILURE, false, false, true);
assertEquals(0, commenter.getComment().length());
}

@Test
public void testProcessBuildResultAborted() {
CommentBuilder commenter = createCommenter(Result.ABORTED, null);
commenter.processBuildResult(false, false, false);
CommentBuilder commenter = createCommenter(null);
commenter.processBuildResult(Result.ABORTED, false, false, false);
assertEquals("Build was aborted", commenter.getComment());
}

Expand All @@ -223,14 +222,14 @@ public void testAddUserComment() {

@Test
public void testAddUserCommentWithPreservingFormatting() {
commenter = createCommenter(Result.SUCCESS, TestUtils.getDefaultCodeCoverageMetrics(), true);
commenter = createCommenter(TestUtils.getDefaultCodeCoverageMetrics(), true);
commenter.addUserComment("hello, world");
assertEquals("hello, world\n", commenter.getComment());
}

@Test
public void testAddUserCommentWithStatus() {
commenter.processBuildResult(false, false, false);
commenter.processBuildResult(Result.SUCCESS, false, false, false);
commenter.addUserComment("hello, world");
assertEquals("Build is green\n\n```\nhello, world\n```\n\n", commenter.getComment());
}
Expand All @@ -257,24 +256,24 @@ public void testAddBuildFailureMessage() {
assertTrue(comment.contains("Link to build"));
}

private CommentBuilder createCommenter(Result result, CodeCoverageMetrics coverage) {
return createCommenter(result, coverage, false);
private CommentBuilder createCommenter(CodeCoverageMetrics coverage) {
return createCommenter(coverage, false);
}

private CommentBuilder createCommenter(Result result, CodeCoverageMetrics coverage, boolean preserveFormatting) {
return createCommenter(result, coverage, preserveFormatting, 0.0);
private CommentBuilder createCommenter(CodeCoverageMetrics coverage, boolean preserveFormatting) {
return createCommenter(coverage, preserveFormatting, 0.0);
}

private CommentBuilder createCommenter(
Result result, CodeCoverageMetrics coverage, boolean preserveFormatting,
CodeCoverageMetrics coverage, boolean preserveFormatting,
double maxCoverageDecreaseInPercent) {
return createCommenter(result, coverage, preserveFormatting, maxCoverageDecreaseInPercent, 100.0);
return createCommenter(coverage, preserveFormatting, maxCoverageDecreaseInPercent, 100.0);
}

private CommentBuilder createCommenter(
Result result, CodeCoverageMetrics coverage, boolean preserveFormatting,
CodeCoverageMetrics coverage, boolean preserveFormatting,
double maxCoverageDecreaseInPercent, double minCoverageInPercent) {
return new CommentBuilder(logger, result, coverage, FAKE_BUILD_URL, preserveFormatting,
return new CommentBuilder(logger, coverage, FAKE_BUILD_URL, preserveFormatting,
new CoverageCheckSettings(true, maxCoverageDecreaseInPercent, minCoverageInPercent));
}
}

0 comments on commit 855d620

Please sign in to comment.