ghprb icon indicating copy to clipboard operation
ghprb copied to clipboard

Testing the same pull request several times tests the wrong sha1

Open cristidumitru opened this issue 10 years ago • 8 comments

If I have a pull request with multiple commits, I can't really test all the commits, only the first one. Scenario:

  • I create a commit
  • I push it to a branch
  • I make a pull request from that branch to master
  • I give the comment to start the testing of the pull request (custom phrase)
  • The test fails
  • I commit a fix
  • I give again the comment to start the testing of the pull request
  • [NOT OK] the first commit id is tested instead of the second one. wrong sha1

My jenkins job receives the parameters sha1 /origin/pr/12/merge and the ghprbActualCommit a0a203.... every time, no matter what I try. You can see in the screenshot, the second commit does not get the failure status icon. The only workaround is to start a new pull request every time, but that is really annoying, and we loose all the review comments in the process. The payloads received are correct and are triggered by a custom phrase I set on the job (I have multiple jobs that I want to start from the comments list)

Seems like a bug in the triggering mechanism.

cristidumitru avatar May 14 '14 22:05 cristidumitru

Can you turn the logging levels up and attach those?

valdisrigdon avatar May 27 '14 14:05 valdisrigdon

This may or may not be related to what we (at work) have been experiencing. It seems to be related to rev-parse not coming up with the correct commit. Note that in our case we had force-pushed to the branch and the wrong commit was being used for the build on the push and on subsequent "retest this please" builds.

Truncated log output is as follows:

GitHub pull request #913 of commit 76ae68190e41be6dea6e67ea517248f9e2aadc86 automatically merged.

(...)

git rev-parse refs/remotes/origin/pr/913/merge^{commit}
# Checking out Revision 3cb8e182406890897b302a8939498e9dc758fe2f (refs/remotes/origin/pr/913/merge)
git config core.sparsecheckout
git checkout -f 3cb8e182406890897b302a8939498e9dc758fe2f

The first line shows the correct commit of 76ae68190e41be6dea6e67ea517248f9e2aadc86 which matches what git locally and on github shows as the last commit in the PR (and I think is also what is populated as the GIT_COMMIT env variable). The later sha of 3cb8e182406890897b302a8939498e9dc758fe2f is the result of the preceding rev-parse command and is incorrect. FWIW, I get the same result running the rev-parse locally so it's not a server-side issue.

We fixed this by following a comment in the README and changing the job config to use ${ghprbActualCommit} instead of ${sha1}.

tvon avatar Jul 23 '15 15:07 tvon

So this issue has to do with git then. the variable ${sha1} is going to only be one of two things, either the same as ${ghprbActualCommit} if the PR can't be merged automatically, or origin/pr/##/merge. All of the rev-parse things are handled during the build by the jenkins git plugin. So this appears to be a bug with how GitHub handles a rebase.

The sha you mention 3cb8e182406890897b302a8939498e9dc758fe2f is created solely for the purpose of merging pull requests in GitHub.

DavidTanner avatar Jul 23 '15 16:07 DavidTanner

Actually that would be for #284 I think

DavidTanner avatar Jul 23 '15 16:07 DavidTanner

We've been seeing this even when using ${ghprbActualCommit}. It seems to happen when the build fails (e.g., compilation failure) and then you push a new commit. The GitHub pull request line shows the right commit, but then the git plugin does a checkout of the last commit. Perhaps this is because of the git plugin's "choosing strategy"? Is it trying to keep track of which commits have been built and rebuild them if they haven't succeeded? I know that the gerrit trigger plugin installs its own choosing strategy: https://wiki.jenkins-ci.org/display/JENKINS/Gerrit+Trigger. Maybe ghprb needs to do the same.

thughes avatar Jul 27 '15 21:07 thughes

any update on this

devotox avatar Jan 14 '16 16:01 devotox

Same issue here, any update?

dooleyb1 avatar Dec 16 '19 15:12 dooleyb1

Any update on this issue ?

Tsingh315 avatar Feb 26 '20 22:02 Tsingh315