This is surprisingly difficult to get right, especially with projects with lots of concurrent changes. Gitlab merge trains[1] really help here.
1. https://docs.gitlab.com/ee/ci/merge_request_pipelines/pipeli...
Yes. Anything else is bound to run into the issue described in the article.
I'm honestly surprised that anybody would operate a system that does not do this. Master has to be where all tests are green.
The answer to that is: yes. The final test cycle before landing code to master MUST be against the code that would be in master after merging.
This is what the better workflow tools do, among other things to prevent these weird error situations. We have Marge-bot for Gitlab. Uber open-sourced their tool for the same some time back.[0] Bors comes up often as a reference, too.[1]
0: Discussion at the time: https://news.ycombinator.com/item?id=19692820
The "integration" part of CI appears to have largely been lost - CI pipelines now tend to just build branch tips and confirm tests run. Performing the proposed merge and checking that combination still runs is still pretty rare IME.
Maybe if you have a workflow where it's always the same target branch (master) it works, but we often have long standing feature branches that get multiple smaller merges from other branches.
EDIT: Hang on a second, Tom Forbes! Fancy bumping into you here yet again. It's Dan from Marvel!
Master/Main changed between the tests running for that PR, and it merging. Why should those test results be considered a useful indicator for what happens after the PR merges anymore?
GitHub does have a setting you can enable to help with this, essentially, when any PR merges, it forces the developer to rebase any open PRs. A huge waste of effort, so nobody turns it in.
There are far better systems out there, e.g. zuul-ci.org which was designed to handle this exact problem at scale. Others have mentioned GitLab merge trains, though I've never used them - so hard to tell if they really solve this :)
git checkout --force refs/remotes/pull/119/merge
They generate it async behind the scenes, so it may not be immediately available after your PR is opened, and changes to master may be delayed.
Testing the merged branch at say 10am, then actually merging the PR at 11am says nothing about how that PR interacts with all the changes merged between those 2 points in time.
The only way to ensure always green master/main branch is to test every change as it goes in serially. (There are optimisations that allow you to test things serially but in parallel - which of course sounds like madness .. https://zuul-ci.org/docs/zuul/discussion/gating.html explains it better than I could :D)
I miss having Zuul CI on the projects I work on - was far better than any of the others I've used :)
But that brings a choke point into your workflow:
* Change A developed and good for master
* Change B developed and good for master
* Change A merged into master
* Change B not good for master any more, gets pushed back
* Change C developed and good for master
* ...
In principle, a change can be put back forever, depending on your practical situation.
If anything this is a reason to use merge rather than rebase and force push because it’s easier to debug this nuisance when your devs aren’t mutating implicitly their history