search

mohamicorp / stash-jenkins-postreceive-webhook

Webhook used to notify Jenkins when commits are made to Stash
Other
138 stars 98 forks source link

Build Success does not clear the overall Build Failure status #29

Closed ashnazg closed 10 years ago

ashnazg commented 10 years ago

When viewing the Build Status for a given PR overall, as well as for individual commits, a single Build Failure will result in the overall Build Status to be shown as Failure even when subsequent builds are successful.

Note how the overall PR build status shows failure (far right), but the popup build history shows the most recent build was successful: bug-overallbuild

Here again, note how the given commit (the top one) shows build failure (far right), but the popup build history for it shows the most recent build was successful: bug-commitbuild

I wanted to check with you, the plugin, to see what you think about there the icon choice bug might be, before filing a bug with Stash itself.

ashnazg commented 10 years ago

Note I'm using Stash v2.6.2 with plugin v1.1.

ashnazg commented 10 years ago

In scanning the code here, as well as reading through the "Updating Build Results", I would guess the logic for displaying that icon is actually in Stash itself.

mikesir87 commented 10 years ago

That is correct. This plugin only notifies Jenkins to trigger the build when a commit has been made. Your Jenkins plugin then notifies Stash about the build result. The final icon presentation is left up to Stash.

As far as my opinion of the matter, I would say that the last result wins. If the last result was a success, the icon should reflect that. But, I'm not sure what they're thinking is on it.

I'm going to go ahead and close this, as it isn't an issue with this plugin. But, feel free to continue discussing if you wish. :)

mikesir87 commented 10 years ago

It appears that Stash is expecting that re-builds is supposed to use the same key as the previous build, so more of providing an update to a build result, rather than a new build result. Kinda interesting approach as I would like to see the history, but that's up to them.

If that's the case, may want to contact the stashnotifier-plugin for Jenkins to have them resubmit the same key. Looking at their code base, it looks like if you were to remove the option to include the build number in the key, it might work? Who knows?

ashnazg commented 10 years ago

I opened a JIRA issue against Stash itself (https://jira.atlassian.com/browse/STASH-3905)... thanks for the review Mike.

ashnazg commented 10 years ago

Ah, someone else has noticed this behavior too -- https://github.com/jenkinsci/stashnotifier-plugin/issues/16

ashnazg commented 10 years ago

I did find a config option for stashnotifier to "keep build history", which I'd guess is supposed to toggle including the build number or not. However, on or off appears to still send the build number, so perhaps that is a bug in itself, although not the icon display choice bug that I'm concerned with overall.

ashnazg commented 10 years ago

Hmmm, if the "keep build history" does toggle that build number inclusion, then I bet the default "off" should have worked as a de facto workaround for the Stash bug... if Stash was only keeping one build status, and each update overwrote the previous one, then Stash would only have the most recent one to show at all.

ashnazg commented 10 years ago

Robert's comments on my Stash bug (https://jira.atlassian.com/browse/STASH-3905) confirm mikesir87's explanations. Apparently Stash's behavior is geared not for showing build "history", but only for allowing "peer" builds. So, in the case of one build server doing a second build for the same commit, Stash's expectation is that the build server second build record would overwrite the first build record. In the case of two different build servers running builds for the one commit, Stash would show the two separate builds and show an overall "status of the commit" based on the logic "All builds are green === GREEN overall; any single red build === RED overall".

Given all this, I think bug report here (https://github.com/jenkinsci/stashnotifier-plugin/issues/16) is no bug, but expected behavior -- if "keep repeated builds in stash" is toggled ON, then the old build records from the one build server do not get overwritten in Stash, and therefore Stash will not clear the overall build RED just because the most recent build was green. Stash's position is "any single RED record means overall is RED".

Back to square one -- the observed behavior I've seen is that whether or not the "keep repeated builds in stash" is toggled ON or OFF, the build number seems to always be included in the build record passed to Stash. Therefore, when toggled OFF, first build fails, then second build succeeds, Stash still sees two build records and shows "first Red, second Green, therefore overall Red".

As such, there may still be an actual bug with regard to the "keep repeated builds in stash" setting in the stashnotifier. Will continue to pursue the issue there instead.

mikesir87 commented 10 years ago

Great followup! I apparently don't have the ability to see the Stash bug issue you reported (tells me I don't have permission), so I wasn't able to follow along. Thanks for providing the update!