Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[JENKINS-43292] Fix visualization of parallel steps with aborted and failed branches #24

Merged
merged 4 commits into from Apr 18, 2019

Conversation

@dwnusbaum
Copy link
Member

commented Apr 16, 2019

See JENKINS-43292.

Before this change, non-successful branches in parallel steps would always be visualized in Blue Ocean as if they failed, even if they were actually aborted (which can happen when using failFast: true). This is confusing, and it caused the aborted branches to be focused and expanded instead of the failed branches, making it hard to determine which branch failed and which branch was aborted.

The reason the visualization used to be incorrect is that for parallel branches, Blue Ocean calls computeChunkStatus2(before: parallelStartNode, firstNode: parallelBranchStartNode, lastNode: parallelBranchEndNode, after: parallelEndNode). In the event that both parallelBranchEndNode and parallelEndNode have an ErrorAction, the ErrorAction on the parallelEndNode was being used to determine the status, which is incorrect if the branch aborted but the overall parallel step failed.

The newly added test parallelFailFast fails before the change, and the basic test parallel passes before and after the change. Both tests use a new StandardChunkVisitor to visit parallel branches in a way similar to what is done by Blue Ocean.

For stages, Blue Ocean calls computeChunkStatus2(before: stageStartNode, firstNode: stageBodyStartNode, lastNode: stageBodyEndNode, after: stageEndNode). I am not aware of any way for there to be a different ErrorAction on the stageBodyEndNode than the stageEndNode, so the new code should be identical to the old code for stages. The newly added test catchOutsideFailingStage returns the same result before and after the change.

This PR also updates dependencies and incrementalifies the plugin, and requires a version of workflow-api including jenkinsci/workflow-api-plugin#89.

Example Pipeline:

parallel failFast: true,
  aborts: {
    sleep 5
  },
  fails: {
    sleep 1
    error('oops')
  },
    succeeds: {
    echo 'success'
  }

Before:
Screen Shot 2019-04-16 at 15 19 35

After:
Screen Shot 2019-04-16 at 15 17 15

@dwnusbaum dwnusbaum requested review from abayer, jglick and car-roll Apr 16, 2019

@jglick

This comment has been minimized.

Copy link
Member

commented Apr 16, 2019

Windows agent outage, trying to retrigger…

@jglick jglick closed this Apr 16, 2019

@jglick jglick reopened this Apr 16, 2019

@jglick
jglick approved these changes Apr 16, 2019
Copy link
Member

left a comment

Just glanced at it, but sounds OK.

pom.xml Outdated Show resolved Hide resolved
} else {
// If next node lacks an ErrorAction, the error was caught in a catch block

This comment has been minimized.

Copy link
@dwnusbaum

dwnusbaum Apr 16, 2019

Author Member

The comment in this else is not clear to me. catchOutsideFailingStage demonstrates the test case that I imagined corresponded to this code, but it passes with and without the change. As long as computeChunkStatus2 is being called for a stage or parallel branch in the ways described in the PR description, I don't think this branch will ever matter. (If you pass arbitrary arguments to computeChunkStatus2 then it might matter, but I think that would be an unintended use of the method)

@abayer
abayer approved these changes Apr 16, 2019

@dwnusbaum dwnusbaum merged commit e89a447 into jenkinsci:master Apr 18, 2019

2 checks passed

continuous-integration/jenkins/incrementals Deployed to Incrementals.
Details
continuous-integration/jenkins/pr-merge This commit looks good
Details

@dwnusbaum dwnusbaum deleted the dwnusbaum:JENKINS-43292 branch Apr 18, 2019

@batmat

This comment has been minimized.

Copy link
Member

commented May 2, 2019

OMG, please please, any plan to release this, that is awesome!

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.