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

Don't publish twice, while the build isn't finished yet #446

Merged
merged 1 commit into from
Dec 17, 2018
Merged

Don't publish twice, while the build isn't finished yet #446

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 2 additions & 2 deletions src/main/java/jenkins/plugins/slack/ActiveNotifier.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ public void finalized(AbstractBuild r) {
AbstractProject<?, ?> project = r.getProject();
Result result = r.getResult();
AbstractBuild<?, ?> previousBuild = project.getLastBuild();
if (null != previousBuild) {
if (null != previousBuild && !r.isBuilding()) {
Copy link
Member

Choose a reason for hiding this comment

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

when does this happen?
I tested master and this branch, (with a very simple job) and both had the same output

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm also troubled by this, I noticed it only now that I was working with #445 . My build job is a simple shell execution of:

sleep 2s
exit 0

And I keep getting 2 notifications when the build completes, which (I assume) are because finalized is fired very early, while jenkins hasn't marked up the build as finished. I looked into the jenkins source code, Run.java:
https://github.com/jenkinsci/jenkins/blob/bcc4e4fe9c3dccc5ca74299d24f8f992c9d5cd71/core/src/main/java/hudson/model/Run.java#L737-L745

    public @Nonnull String getDurationString() {
        if (hasntStartedYet()) {
            return Messages.Run_NotStartedYet();
        } else if (isBuilding()) {
            return Messages.Run_InProgressDuration(
                    Util.getTimeSpanString(System.currentTimeMillis()-startTime));
        }
        return Util.getTimeSpanString(duration);
    }

And Messages.properties:
https://github.com/jenkinsci/jenkins/blob/584c34cda584b98b71d82d038e7d5ebe38534e18/core/src/main/resources/hudson/model/Messages.properties#L233

Run.InProgressDuration={0} and counting

And confirmed that the output I get, test - #23 Success after 12 sec and counting (Open) comes up because the isBuilding() is still true.

My jenkins env is:

Virtual box machine
Debian GNU/Linux 9.6 (stretch)
jenkins: 2.155

I get the duplicate messages in our current master as well:

test - #41 Started by user admin (Open)
test - #41 Success after 4.1 sec and counting (Open)
test - #41 Success after 4.9 sec (Open)

Copy link

Choose a reason for hiding this comment

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

@KreAch3R I use JobDSL for creation job. -there is no duplicate.
Manually created : also only 2 messages in slack room;

test_misc » test_manuakl_slack - #1 Started by user Denis Shvedchenko (Open)
custom general mssage

test_misc » test_manuakl_slack - #1 Success after 0.3 sec and counting (Open)
custom success message

do {
previousBuild = previousBuild.getPreviousCompletedBuild();
} while (previousBuild != null && previousBuild.getResult() == Result.ABORTED);
Expand All @@ -123,7 +123,7 @@ public void completed(AbstractBuild r) {
AbstractProject<?, ?> project = r.getProject();
Result result = r.getResult();
AbstractBuild<?, ?> previousBuild = project.getLastBuild();
if (null != previousBuild) {
if (null != previousBuild && !r.isBuilding()) {
do {
previousBuild = previousBuild.getPreviousCompletedBuild();
} while (null != previousBuild && previousBuild.getResult() == Result.ABORTED);
Expand Down