[JENKINS-37812] No notification re. failed job if agent goes offline during the build

[fcojfernandez]

See JENKINS-37812

Class Mailer is implementing the interface SimpleBuildStep. Since the workspace is only used to format the email sent, it's not entirely needed to fail if the workspace is not retrieved. With this PR the email notifying that the build failed will be sent even if the agent goes offline.

Added a test that checks that the email is sent and fails with the precious code.

@reviewbybees @alecharp

[jeffret-b]

Makes sense to me.

[varyvol]

It makes sense to me, but it feels weird that implementing such an important interface is done in a "arbitrary" way. Have you checked the git history to review when this was added and why?

[oleg-nenashev]

SimpleBuildStep exists here to provide Pipeline support. This change effectively removes Pipeline Support AFAICT, e.g. AbstractBuild is not Pipeline-compatible. Has it been trsted?

[batmat]

I agree with @varyvol.
To make this acceptable IMO, we should double-check we're not breaking compatibility.

I think we should proceed this way:

• Add a Pipeline test (adding necessary pipeline-plugins in test scope) to check the snippet we find on some places over the web still works:

step([$class: 'Mailer',
     notifyEveryUnstableBuild: true,
     recipients: "some-email@gmail.com",
     sendToIndividuals: true])

E.g.

• https://code.i-harness.com/fr/q/23727cc
• https://baptiste-wicht.com/posts/2017/06/jenkins-tip-send-notifications-fixed-builds-declarative-pipeline.html

Ideally: we should probably also have a pipeline test using the more standard new mail step introduced in https://github.com/jenkinsci/workflow-basic-steps-plugin/blob/master/src/main/java/org/jenkinsci/plugins/workflow/steps/MailStep.java and check it still works.

If I'm not clear, please reach out to me, I'm happy to explain this more thoroughly.

[fcojfernandez]

@oleg-nenashev @batmat I thought email-ext was the plugin responsible for the Pipeline integration and not this one (I didn't know the correct use of SimpleBuildStep) and according the official documentation of the plugin the way to use it is by adding a Post-build Action. I have to re-write the PR :(

[fcojfernandez]

@oleg-nenashev @batmat @varyvol feedback addressed

• The jenkins.version property has been updated to be aligned with all the test dependencies
• I've used the workflow.version 2.6 since it's the version of workflow-plugin which that plugin updated its dependency on mailer-plugin

[alecharp]

I do also prefer the current state of the PR. Thank you @fcojfernandez

[batmat]

Looks better to me. 👍 thanks for the test. This is good and critical investment to avoid breaking thing in the future.

[batmat]

NIT: given you're already using assertThat below, using hasSize here would yield a clearer message in case of failure.

[batmat]

👍

[oleg-nenashev]

lgtm, and thanks for the tests!

[oleg-nenashev]

This property does not make much sense, because Pipeline plugin releases are not synchronized starting from 2.0 versions

[alecharp]

can be change is a following PR, I'll merge in the current state. Thank you for reviewing this @oleg-nenashev.