[JENKINS-39203] Create new action to mark a FlowNode with a result such as unstable

[dwnusbaum]

See JENKINS-39203. (I am intentionally not updating the status of that ticket or assigning it to myself until I am ready to merge/release all relevant changes to avoid giving users premature hope of a fix and so that I have clear answers for what will and won't work and when any fixes will be released.) Subsumes #63.

This PR adds a new action named WarningAction. This action can be stored on a FlowNode to mark that although it completed successfully, some kind of non-fatal problem occurred. For now, this action will be used by Blue Ocean to mark stages containing a FlowNode with a WarningAction as Result.UNSTABLE.

In the future, if we wanted to add the ability to track some new kind of non-fatal result, we could update WarningAction to store a custom Result (or similar) and use that in visualizations to modify how the action should be displayed.

Upstream PRs:

• [JENKINS-45579] Add steps to set unstable status and enhance catchError workflow-basic-steps-plugin#83
• [JENKINS-39203] Only show blocks containing a WarningAction as unstable pipeline-graph-analysis-plugin#25
• JENKINS-39203 - Only show blocks containing a WarningAction as unstable blueocean-plugin#1954
• [JENKINS-39203] Mark JUnit steps with failing tests with WarningAction junit-plugin#118
• [JENKINS-39203] Mark publishIssues steps with QualityGateStatus.WARNING with WarningAction warnings-ng-plugin#58

[dwnusbaum]

Closing and reopening in an attempt to get a CI build.

[dwnusbaum]

I think I'm running into JENKINS-54126, so I'll just deploy snapshots manually.

[dwnusbaum]

Maybe it would make more sense for this to be @Nullable so that something like unstable() with no message would work without having to create some kind of default message.

[jglick]

I do not see a use case for a null message, so would suggest keeping it @Nonnull unless and until one is found.

[jglick]

On second thought, perhaps it would be appropriate to have the constructor take no arguments, then use the idiom

public WarningAction withMessage(@Nonnull String message) {
    this.message = message;
    return this;
}

as I think it is likely we will at some point want an associated URL and perhaps even a badge icon.

[dwnusbaum]

Any particular reason to prefer withMessage, withUrl, etc. over adding new constructors that take additional arguments? With the former pattern it is easy to forget to call withMessage. We could use the builder pattern and validate that the message is non-null when constructing the WarningAction from the builder, but that seems like overkill.

[jglick]

The builder style makes it easier to introduce new features compatibly. If we decide that a message is mandatory, then keep it in the constructor.

[jglick]

Sounds fine in isolation.

[jglick]

Good catch!

[jglick]

I do not see a use case for a null message, so would suggest keeping it @Nonnull unless and until one is found.

[dwnusbaum]

Here is an example of what this change looks like for the following pipeline:

stage('unstable-one') {
    unstable('foobar')
}
stage('success') {
    echo('whatever')
}
stage('failure') {
    unstable('three')
    error('uncaught')
}

For now only the actual FlowNode that has the WarningAction attached will be shown as unstable (unlike Blue Ocean in jenkinsci/pipeline-graph-analysis-plugin#25 which searches all FlowNodes in a stage/branch for a WarningAction.). This seems like the most obvious behavior to me, but I could make it work like Blue Ocean if desired.

[dwnusbaum]

All of the upstream PRs are using the current version of the API and I don't anticipate any further changes to it right now, so I'm going to go ahead and merge this PR and release the plugin.