Modify computeChunkTiming to always use the start time of lastNode as the endTime for parallel branches

[stuartrowe]

Fix for JENKINS-75640 after discussion on jenkinsci/pipeline-graph-view-plugin#368.

The current implementation of StatusAndTiming#computeChunkTiming always computes the timing of parallel branches using the start time of closing parallel step node - the start time of the current branch's start node for completed builds. This isn't correct as each branch may take a different amount of time.

This change removes the conditions around using the the start time of branch's end node so that it will be used for both active and complete builds.

Testing done

The StatusAndTimingTest#testBasicParallelFail test already has basic validation that branch timings are correct. I've just updated the assert logic so that comparison timing is also computed using the branch's end node instead of the after node.

Submitter checklist

• Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
• Ensure that the pull request title represents the desired changelog entry
• Please describe what you did
• Link to relevant issues in GitHub or Jira
• Link to relevant pull requests, esp. upstream and downstream changes
• Ensure you have provided tests - that demonstrates feature works or fixes the issue

[timja]

Thanks for looking in to this and tracking it down!

[felipecrs]

It would be awesome to get this shipped.

[stuartrowe]

@jglick @dwnusbaum @car-roll could this get reviewed please?

[jglick]

@timja you seem to have write permission here (I am not sure how), so you should just be the maintainer, since pipeline-graph-view is the only actively maintained plugin using this API plugin.

[timja]

@timja you seem to have write permission here (I am not sure how), so you should just be the maintainer, since pipeline-graph-view is the only actively maintained plugin using this API plugin.

Org owner but sure will do

[dwnusbaum]

IMO this plugin should just be deprecated, and you should copy all relevant functionality into pipeline-graph-view as desired and refactor it for your use case. The ChunkFinder API was very broken in edge cases last I checked (e.g. nested parallelism, especially when the parallel steps are still running), in that the detected "chunks" can be no good, and as a result all of the StatusAndTiming parameters that take before/after nodes are awkward to use and can seem broken depending on what arguments you pass.

[felipecrs]

Pipeline: REST API depends on it, and for one, I think Pipeline Stage View depends on it too.

If we decide to deprecate, we got to first get feature-parity with the Pipeline Stage View so that it can be deprecated too.

[dwnusbaum]

I just meant to deprecate it so that no one tries to use it in new code. Blue Ocean and Stage View will exist probably forever and use this code. If you are making a new Pipeline visualization plugin, I would recommend copying what you need or ideally using it as inspiration for your own DepthFirstScanner-based traversal, ignoring the entire "chunk" concept and related APIs, but do not try to reuse it. Fixing bugs here carries real risk of regressing those other plugins and the use cases can be slightly different in ways that are not always obvious.