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.

Sign up for GitHub

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

Return root stages if they have child steps. #377

Merged
merged 3 commits into from
Apr 4, 2024
Merged

Return root stages if they have child steps. #377

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5a6b5d4
Handle no stage and Jenkins step error
timja Mar 12, 2024
ff5a4e4
Return root stages if they have steps
timbrown5 Apr 3, 2024
148edfd
Apply splotless
timbrown5 Apr 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 44 additions & 16 deletions src/main/java/io/jenkins/plugins/pipelinegraphview/treescanner/PipelineNodeTreeScanner.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@
private final boolean declarative;

private static final Logger logger = LoggerFactory.getLogger(PipelineNodeTreeScanner.class);
private boolean isDebugEnabled = logger.isDebugEnabled();
private final boolean isDebugEnabled = logger.isDebugEnabled();

public PipelineNodeTreeScanner(@NonNull WorkflowRun run) {
this.run = run;
Expand Down Expand Up @@ -126,6 +126,7 @@
for (String stageId : stageNodeMap.keySet()) {
stageNodeStepMap.put(stageId, getStageSteps(stageId));
}

return stageNodeStepMap;
}

Expand Down Expand Up @@ -207,16 +208,23 @@
return this.wrappedStageMap;
}
dump("Remapping stages");
// Find any root stages (ones without parents) that have steps - we want to return these.
// This should only be FlowNodeStart nodes, but I want to be flexible - if a Stage has steps then the
// step view should display it.
// NOTE: In instances where this isn't the FlowNodeStart, this might add an unexpected FlowNode to the graph
// view.
List<FlowNodeWrapper> rootStagesWithSteps = getStagesWithChildSteps().stream()
.filter(s -> s.getFirstParent() == null)
.collect(Collectors.toList());
Map<String, FlowNodeWrapper> stageMap = this.wrappedNodeMap.entrySet().stream()
.filter(e -> shouldBeInStageMap(e.getValue()))
.filter(e -> rootStagesWithSteps.contains(e.getValue()) || shouldBeInStageMap(e.getValue()))
.collect(Collectors.toMap(e -> e.getKey(), e -> e.getValue()));
List<FlowNodeWrapper> nodeList = new ArrayList<FlowNodeWrapper>(stageMap.values());
Collections.sort(nodeList, new FlowNodeWrapper.NodeComparator());
for (FlowNodeWrapper stage : nodeList) {
FlowNodeWrapper firstParent = stage.getFirstParent();
// Remap parentage of stages that aren't children of stages (e.g. allocate node
// step).
dump("Stages has %s parents", stage.getParents().size());
dump("First parent of stage %s: %s", stage.getId(), firstParent);
if (firstParent != null) {
dump("Parent exists in stage map: %s", stageMap.containsKey(firstParent.getId()));
Expand All @@ -229,6 +237,24 @@
return this.wrappedStageMap;
}

/* Filter wrappedNodes to get list of steps.
*/
private Map<String, FlowNodeWrapper> getSteps() {
return this.wrappedNodeMap.entrySet().stream()
.filter(e -> shouldBeInStepMap(e.getValue()))
.collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
}

/* Filter wrappedNodes to get list of steps.
*/
private List<FlowNodeWrapper> getStagesWithChildSteps() {
return getSteps().entrySet().stream()
.map(e -> e.getValue().getFirstParent())
.filter(p -> p != null)

Check warning on line 253 in src/main/java/io/jenkins/plugins/pipelinegraphview/treescanner/PipelineNodeTreeScanner.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line 

Line 253 is only partially covered, one branch is missing
.map(p -> wrappedNodeMap.get(p.getId()))
.collect(Collectors.toList());
}

private boolean shouldBeInStageMap(FlowNodeWrapper n) {
// We also want to drop steps blocks - as the front-end doesn't expect them.
// For the future: Adding Step Blocks as stages might be a good way to handle them in the
Expand Down Expand Up @@ -285,13 +311,10 @@
}

dump("Remapping steps");
Map<String, FlowNodeWrapper> stepMap = this.wrappedNodeMap.entrySet().stream()
.filter(e -> shouldBeInStepMap(e.getValue()))
.collect(Collectors.toMap(e -> e.getKey(), e -> e.getValue()));

Map<String, FlowNodeWrapper> stepMap = getSteps();
Map<String, FlowNodeWrapper> stageMap = this.getStageMapping();
List<FlowNodeWrapper> nodeList = new ArrayList<FlowNodeWrapper>(stepMap.values());
Collections.sort(nodeList, new FlowNodeWrapper.NodeComparator());
nodeList.sort(new FlowNodeWrapper.NodeComparator());
for (FlowNodeWrapper step : nodeList) {
FlowNodeWrapper firstParent = step.getFirstParent();
// Remap parentage of steps that aren't children of stages (e.g. are in Step
Expand Down Expand Up @@ -321,13 +344,15 @@
* Builds a graph from the list of nodes and relationships given to the class.
*/
private void buildGraph() {
List<FlowNode> nodeList = new ArrayList<FlowNode>(nodeMap.values());
Collections.sort(nodeList, new FlowNodeWrapper.FlowNodeComparator());
List<FlowNode> nodeList = new ArrayList<>(nodeMap.values());
nodeList.sort(new FlowNodeWrapper.FlowNodeComparator());
// If the Pipeline ended with an unhandled exception, then we want to catch the
// node which threw it.
BlockEndNode<?> nodeThatThrewException = null;
if (!nodeList.isEmpty()) {
nodeThatThrewException = getUnhandledException(nodeList.get(nodeList.size() - 1));
boolean hasStage = nodeList.stream().anyMatch(PipelineNodeUtil::isStage);

nodeThatThrewException = getUnhandledException(nodeList.get(nodeList.size() - 1), hasStage);
}
for (FlowNode node : nodeList) {
if (nodeThatThrewException == node) {
Expand All @@ -352,12 +377,15 @@
* Returns the origin of any unhandled exception for this node, or null if none
* found.
*/
private @CheckForNull BlockEndNode<?> getUnhandledException(@NonNull FlowNode node) {
private @CheckForNull BlockEndNode<?> getUnhandledException(@NonNull FlowNode node, boolean hasStage) {
// Check for an unhandled exception.
ErrorAction errorAction = node.getAction(ErrorAction.class);
// If this is a Jenkins failure exception, then we don't need to add a new node
// - it will come from an existing step.
if (errorAction != null && !PipelineNodeUtil.isJenkinsFailureException(errorAction.getError())) {
if (errorAction != null) {
// If this is a Jenkins failure exception, then we don't need to add a new node
// - it will come from an existing step if there is a stage for the step to be part of.
if (hasStage && PipelineNodeUtil.isJenkinsFailureException(errorAction.getError())) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this will fix it for no stages, but does this fix withCredentials in a stage? I think that's where I got stuck because its a block scoped step, it wasn't handled.

return null;
}
dump(
"getUnhandledException => Found unhandled exception: %s",
errorAction.getError().getMessage());
Expand All @@ -372,7 +400,7 @@
/*
* This is a corner case for trivial graphs - ones that only have one action. In
* this case the error can be thrown by a the FlowStartNode, which would mean a
* single nodes needs to be a stage and a step. Rather than adding a fake node
* single node needs to be a stage and a step. Rather than adding a fake node
* to the graph, we use the end node that we were given to act as the step
* - this might need additional logic when getting the log for the exception.
*/
Expand Down