Currently, the pull request handler figures out which commit to test by retrieving payload['pull_request']['head']['sha']
. However, this is the latest commit on the head branch (as opposed to the base branch, which is the one changed once the PR is merged).
The behavior on Travis CI is different, per their Building Pull Requests doc:
Rather than test the commits that have been pushed to the branch the pull request is from, we test the merge between the origin and the upstream branch. To only build on push events, you can disable Build on Pull Requests from your repository settings.
I believe the other online CI systems are similar, but I'm just using Travis CI as the example as that's what I'm familiar with from working on VOC.
GitHub provides this commit sha in the ['pull_request']['merge_commit_sha']
field, and as explained a note box in their Pull Requests api docs:
Each time the pull request receives new commits, GitHub creates a merge commit to test whether the pull request can be automatically merged into the base branch. (This test commit is not added to the base branch or the head branch.)
The value of the merge_commit_sha
attribute changes depending on the state of the pull request. Before a pull request is merged, the merge_commit_sha
attribute holds the SHA of the test merge commit. After a pull request is merged, the attribute changes depending on how the pull request was merged: [...]
I think that we should test this merge-commit-of-head-into-base-branch instead of the latest commit on the head branch, because there are circumstances where the head branch may pass CI (due to it being mergeable without file conflicts), but the way git happily does the merge leaves the base branch (master) in a failing state. I vaguely remember that Travis has caught this case in at least one VOC PR before?
For discussion perhaps? I am not too sure if there is further impact on the Beekeeper/Beefore/etc as a result of making this change.