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.

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

Fix concurrent build back to normal notification missed #576

Merged
merged 2 commits into from
May 30, 2019

Conversation

EricBartusch
Copy link
Contributor

Fixes #544

Added a check to make sure that the same builds aren't being compared with each other.

It seems like when two concurrent builds are happening, the context for the project variable gets changed so the project.getLastBuild() ends up returning the newest build even though r is still the earlier of the two. This makes r and previousBuild refer to the same build and break the OnBackToNormal check.

Fixing issue where previous build could also be current build if multiple builds were running concurrently
@@ -134,7 +134,7 @@ public void completed(AbstractBuild r) {
if (null != previousBuild) {
do {
previousBuild = previousBuild.getPreviousCompletedBuild();
} while (null != previousBuild && previousBuild.getResult() == Result.ABORTED);
} while ((null != previousBuild && previousBuild.getResult() == Result.ABORTED) || previousBuild.getNumber() == r.getNumber());
Copy link
Member

Choose a reason for hiding this comment

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

seems ok, have you tested it out?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, I was able to reproduce the error on the local Jenkins instance created by running the plugin. I confirmed that when I made my change the notification properly showed up as well as logging the last build properly, fixing #568 too.

I'll check out the failing build and push more changes if need be.

@timja timja added the fix label May 29, 2019
@timja timja changed the title Update ActiveNotifier.java Fix concurrent build back to normal notification missed May 30, 2019
@timja timja merged commit 06b17f2 into jenkinsci:master May 30, 2019
@timja timja mentioned this pull request Jun 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Back to normal notifications missed
2 participants