Skip to content
Permalink
Browse files

[FIXED JENKINS-48752] Drastically simplify and fix changed calculation

The previous was...complicated. In some contexts, we do need to check
the execution result, not just the run result, but that led to me
writing some crappy code that meant that changed always triggered for
any status but SUCCESS. Sigh. So instead, specifically get the worst
status from the run and the execution and use that for
comparison. Suddenly everything's simpler. =)
  • Loading branch information...
abayer committed Jan 29, 2018
1 parent a247e5d commit 08be448a802e6b5bbcddf1e50ebc230011976bb3
@@ -41,24 +41,17 @@ class Changed extends BuildCondition {
Result execResult = getExecutionResult(r)
// Only look at the previous completed build.
WorkflowRun prev = r.getPreviousCompletedBuild()

// Get the *worst* result of either the execution or the run. If the run's result is null, that's effectively
// SUCCESS.
Result runResult = execResult.combine(r.getResult() ?: Result.SUCCESS)

// If there's no previous build, we're inherently changed.
if (prev == null) {
return true
}
// If the current build's result isn't null (i.e., it's got a specified status), and it's different than the
// previous build's result, we're changed.
else if ((execResult != null && prev.getResult() != execResult) ||
(r.getResult() != null && prev.getResult() != r.getResult())) {
return true
}
// If the current build's result is null and the previous build's result is not SUCCESS, we're changed.
else if ((execResult == Result.SUCCESS && prev.getResult() != Result.SUCCESS) ||
(r.getResult() == null && prev.getResult() != Result.SUCCESS)) {
return true
}
// And in any other condition, we're not changed, so return false.
else {
return false
} else {
// Otherwise, compare the combined execution/run result to the previous result.
return runResult != prev.getResult()
}
}

@@ -207,4 +207,28 @@ public void contextResultOverridesRunResult() throws Exception {
.logNotContains("MOST DEFINITELY FINISHED")
.go();
}

@Issue("JENKINS-48752")
@Test
public void changedAndNotSuccess() throws Exception {
WorkflowRun b = getAndStartNonRepoBuild("postOnChangeFailed");
j.assertBuildStatus(Result.FAILURE, j.waitForCompletion(b));
j.assertLogContains("[Pipeline] { (foo)", b);
j.assertLogContains("I FAILED", b);

WorkflowJob job = b.getParent();
job.setDefinition(new CpsFlowDefinition(pipelineSourceFromResources("postOnChangeUnstable"), true));
WorkflowRun b2 = j.assertBuildStatus(Result.UNSTABLE, j.waitForCompletion(job.scheduleBuild2(0).get()));
j.assertLogContains("[Pipeline] { (foo)", b2);
j.assertLogContains("hello", b2);
j.assertLogContains("I CHANGED", b2);

// Now make sure we don't get any alert this time.
WorkflowRun b3 = j.assertBuildStatus(Result.UNSTABLE, j.waitForCompletion(job.scheduleBuild2(0).get()));
j.assertLogContains("[Pipeline] { (foo)", b3);
j.assertLogContains("hello", b3);
j.assertLogNotContains("I CHANGED", b3);
j.assertLogNotContains("I FAILED", b3);
}

}
@@ -0,0 +1,48 @@
/*
* The MIT License
*
* Copyright (c) 2018, CloudBees, Inc.
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
* in the Software without restriction, including without limitation the rights
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
* copies of the Software, and to permit persons to whom the Software is
* furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in
* all copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
* THE SOFTWARE.
*/

pipeline {
agent none
stages {
stage("foo") {
steps {
echo "hello"
script {
currentBuild.result = "UNSTABLE"
}
}
}
}
post {
changed {
echo "I CHANGED"
}
failure {
echo "I FAILED"
}
}
}



0 comments on commit 08be448

Please sign in to comment.
You can’t perform that action at this time.