Send one notification for a matrix job

[proski]

Stash notifier sends multiple notifications then used in a multi-configuration (matrix) job. There is a risk that the PR would appear to be "approved" by Jenkins before all jobs complete, creating a window of "opportunity" to merge broken code.

A simple fix is to require a minimal number of successful builds. But it's a not a reliable fix. New configurations can be added to the matrix build, making the check unreliable. Likewise, some configurations can be removed, making all PRs unmergeable until an admin changes the threshold. Both has happened to me.

I believe the best solution would be to send a single notification from the top-level process. That way, I could require one successful build for merging a PR, and that number is not likely to change.

My environment: Jenkins 2.150.2, Stash notifier 1.15.

[proski]

See also https://issues.jenkins-ci.org/browse/JENKINS-31978

[scaytrase]

Might be related to #74
There is a plenty of options for real:

• Implement inprogress notification on job queueing (has some theoretical problems though)
• Migrate to pipelines in order to control pre and post steps by yourself

I hope the problem is that matrix configuration project doesn't have any steps for parent job itself, in other case it would be easy to attach generic notification steps to them

[proski]

I'm actually interested in the notification about the build result, not about notification about queuing, so it's a separate issue.

It looks like the plugins that want to utilize the top-level job need to use a special mechanism (the MatrixAggregator class): https://issues.jenkins-ci.org/browse/JENKINS-20984

[scaytrase]

Looks like implementing matrix support is not really hard

https://github.com/jenkinsci/email-ext-plugin/blob/9012fcd1b9b2040f2d34c2723198a751eb81cce5/src/main/java/hudson/plugins/emailext/ExtendedEmailPublisher.java#L990-L1014

Only question is if it will create dependency bound to the matrix job plugin (but I hope most Jenkins installations have it by default)

[proski]

It turns out stash-pullrequest-builder-plugin can run code in the top-level job. Look for onCompleted. It posts comments to Stash and merges the PR on success if configured to do so.
https://github.com/jenkinsci/stash-pullrequest-builder-plugin/blob/master/src/main/java/stashpullrequestbuilder/stashpullrequestbuilder/StashBuilds.java#L46
https://github.com/jenkinsci/stash-pullrequest-builder-plugin/blob/master/src/main/java/stashpullrequestbuilder/stashpullrequestbuilder/StashBuildListener.java

[scaytrase]

Yes, implementing RunListener instead of implementing Notifier could do the trick, but this is a BC-break at the first look and requires a bunch of refactoring.

We've discussed some listener implementations in #74

[proski]

The top-level notification could use the listener, while the per-configuration notification could (and probably should) use the existing mechanism. As long as the new functionality is not enabled, the plugin should just work as before.

I hope it's possible to detect a matrix job for the purpose of hiding irrelevant checkboxes without introducing a hard dependency.