-
Bug
-
Resolution: Unresolved
-
Major
-
None
Some time back, declarative pipelines were marking a stage as unstable, and I was able to have something like:
pipeline {
stage {
steps {
sh 'runuittests'
}
post {
always {
junit 'junit.xml'
}
}
}
stage {
when {
expression {
currentBuild.resultIsBetterOrEqualTo('SUCCESS')
}
}
steps {
// Only runs when tests are stable
}
}
}
However, this stopped working at some point in the recent past.
currentBuild.result does not seem to be populated when junit marks the build unstable.
- is duplicated by
-
-
- Resolved
-
- links to
[JENKINS-48178] Unstable build from junit not picked up in currentBuild.result
Thanks, therealwaldo
So this is the same underlying issue as JENKINS-37663: junit doesn't call Run#setResult(Result) any more, it calls getContext().setResult(Result), which eventually flows down to CpsFlowExecution#setResult(). I thought I was smart to do it that way, because hey, avoiding actually explicitly setting the build result as a whole is a good thing! Once we eventually have proper per-stage/step statuses, we definitely want to avoid doing that. But now currentBuild.result and friends are no longer meaningful, which is nonideal. The JENKINS-37663 fix was to make Declarative's post conditions smart enough to look at the CpsFlowExecution#getResult(), but that's a lot harder to pull off here because RunWrapper (which is what currentBuild is an instance of, more or less) is in workflow-support, so doesn't know anything about CpsFlowExecution#getResult(). That's not a method on FlowExecution, just CpsFlowExecution.
And the more I think about it, the more I wonder if getContext().setResult(Result) was really that much smarter to use. It's true that we can call that again in the future with a better result than it currently has, which we can't do with Run#setResult(Result), but to take advantage of that, we'd need all kinds of special logic for tracking the status before we enter a try/catch or catchError and resetting back to that afterwards, which we don't have at this point. So I'd say there are two possible options here:
- Add FlowExecution#setResult(Result) and FlowExecution#getResult() abstract methods, and change CpsFlowExecution to override them. Finally, update RunWrapper in workflow-support to pick up FlowExecution#getResult() and use similar logic to what we have in Declarative to decide which of FlowExecution#getResult() and Run#getResult() is the appropriate one to use. This doesn't help for any other cases of something expecting Run#getResult() to be accurate and current, though, and this whole approach is feeling like premature optimization.
- Roll junit back to using Run#setResult(Result). Declarative's fine with that - its logic is smart enough to know when to ignore CpsFlowExecution#getResult() in favor of Run#getResult() anyway, so we won't break anything by changing that. It does mean that down the road, we'll have to make changes to junit to support better status granularity, but like I mentioned above, I think we were probably going to have to do that anyway.
As you may have guessed, I'm leaning towards option #2.
cc svanoort for his thoughts.
Ben Cooksley
added a comment - Would this issue apply equally to regular Jenkins Pipelines (non-declarative ones) as well?
We've been hit by an issue which is basically the same as the above (currentBuild.result no longer being changed by JUnit to unstable), only we don't use declarative pipelines.
Ben Cooksley
added a comment - Would this issue apply equally to regular Jenkins Pipelines (non-declarative ones) as well?
We've been hit by an issue which is basically the same as the above (currentBuild.result no longer being changed by JUnit to unstable), only we don't use declarative pipelines. bcooksley - yup, currentBuild.result isn't picking up the junit UNSTABLE result regardless of whether you're using Declarative.
PR up for junit at https://github.com/jenkinsci/junit-plugin/pull/91 to switch to setting the Run result, getting currentBuild.result and friends back to behaving like expected.
Code changed in jenkins
User: Andrew Bayer
Path:
src/main/java/hudson/tasks/junit/pipeline/JUnitResultsStepExecution.java
src/test/java/hudson/tasks/junit/pipeline/JUnitResultsStepTest.java
http://jenkins-ci.org/commit/junit-plugin/acd8f4843602a4629344f4cb404cec2dfb7a54d2
Log:
[FIXED JENKINS-48178] Set run result rather than context result
This fixes currentBuild.result to be UNSTABLE after test failures are
encountered. We'll want to revisit this in the future once we have
more granular stage/step status, but going with
getContext().setResult(...) was, in retrospect, premature optimization.
I use JUnit 1.24 and still failed tests captured by JUnit don't get the pipline to unstable state. A failed test causes the pipline to go to failed status.
My Pipeline:
pipeline {
agent {
docker {
image 'qnib/pytest'
args '-v /home/user/git/:/test-reports'
}
}
stages {
stage('Test') {
steps {
sh 'pytest --junitxml result.xml testing/'
}
post {
always {
junit 'result.xml'
}
}
}
}
}
IKO BH
added a comment - - edited abayer
I use JUnit 1.24 and still failed tests captured by JUnit don't get the pipline to unstable state. A failed test causes the pipline to go to failed status.
My Pipeline:
pipeline {
agent {
docker {
image 'qnib/pytest'
args '-v /home/user/git/:/test-reports'
}
}
stages {
stage('Test') {
steps {
sh 'pytest --junitxml result.xml testing/'
}
post {
always {
junit 'result.xml'
}
}
}
}
} 
Attn abayer