Show test results for prospective merge of a Github PR

Github PRs show if the PR branch has conflicts with the base branch.

CircleCI shows test results on the PR branch.

What is missing is the ability to see whether test results fail when the PR branch is merged into the base branch.

Admittedly, it’s not everyday that PRs break tests of other PRs, but it does happen - for example when there’s a change in UI text:

  1. A feature developer issues PR A testing for some specific text in the UI: “The status is FOO”. PR A contains a view test checking for the existence of “The status is FOO”.
  2. A another developer issues PR B that implements a product decision to change all uses of “FOO” to “BAR”. Github says branch is clear to merge
  3. PR B is merged (Github says there are no conflicts; CircleCI says tests pass)
  4. PR A is merged, without problem (Github says there are no conflicts; CircleCI says tests pass)
  5. The base branch now has a test which fails because the application produces a view with the text “The status is BAR”, but the test in PR A expects “The status is FOO”.

Fail.

All of the ways of dealing with this currently seem bad:

  1. Constantly merge into the PR branch, making a mess of the commit history
  2. Rebase the PR branch onto the base and force push onto Github (a no no for obvious reasons)
  3. Catch the test failures later in the base branch (inconsistent with gitflow and other branching strategies)

A feature would be nice, but a work around would be great too.

 

Source: https://discuss.circleci.com/t/show-test-results-for-prospective-merge-of-a-github-pr/1662

  • Guest
  • Mar 5 2019
  • Taking votes
  • Attach files
  • Some User commented
    5 May 03:43pm

    Bumping this after 361 days.

    All other CI systems have this.

  • Celestine Kao commented
    13 Jan 10:54pm

    If you're using GitHub as your VCS, the ghpr orb has a command that builds a branch of the prospective merge.

  • Breno Ribeiro commented
    8 Jan 09:48pm

    https://ideas.circleci.com/ideas/CCI-I-894 would solve this for me.
    Voted on both ideas. :)

  • Amir Arad commented
    29 Oct, 2019 06:56am

    This is a fundamental feature ...

  • Patrick Senti commented
    23 Oct, 2019 09:17am

    @CircleCI-staff Seeing that this has been an open discussion for almost 4 years now (the referenced source is of January 16), is there some means to speed up the process? 

  • David Chen commented
    26 Sep, 2019 09:45pm

    I think I just got 3 notifications from the last post. wat.

  • Ben Hartshorne commented
    26 Sep, 2019 08:49pm

    An enhancement to the feature request to combat the "but what if master changes after I open the PR" comments it would be neat to opt a PR in to "re-run merge tests on every master update". As a PR gets closer to merge time, you could elevate it to get retested every time master moves, thereby having in the PR test results from "this code against current master" rather than "this code against master from whenever the last commit happened on this branch" which ... as pointed out, still yields an answer of "probably" when asked "will this merge work".

    The downside of re-running merge tests when master moves is load (and thereby cost), which may be an acceptable trade off for certain customers and branches.

  • Aditya Chandra commented
    13 Sep, 2019 01:00pm

    This is a very important feature

  • Mathias Lang commented
    2 Sep, 2019 03:10am

    Yes, it is as crucial as the comments describe. It's the default on Travis, and never have I seen someone complaining about it. On the other hand, this enhancement have been asked for for at least 3 years.

    We do the following in our configuration:
    ```
    - run:
        name: Checkout merge commit
        command: |
            set -ex
            if [[ -n "${CIRCLE_PR_NUMBER}" ]]
            then
                FETCH_REFS="${FETCH_REFS}
                +refs/pull/${CIRCLE_PR_NUMBER}/merge:pr/${CIRCLE_PR_NUMBER}/merge"
                git fetch -u origin ${FETCH_REFS}
                git checkout "pr/${CIRCLE_PR_NUMBER}/merge"
            fi
    ```

    But I don't see any reason why it shouldn't be the default. Imagine merging a PR just to find out it goes red because someone added a test in between that fails with the new code. It's not a perfect solution (e.g. Travis does not trigger rebuild automatically on every merge, otherwise it'd overload the CI), but it's a lot better than the current default on CircleCI.

  • Rob Cresswell commented
    13 Aug, 2019 08:41am

    Is this as crucial as the comments describe? I'm fairly naive to CI/CD so apologies if I'm misunderstanding, but the merge commit would still be "the merge commit at the point of testing", which is still no guarantee of it breaking the target branch or not at the point of merge; you'd still have to retest the target to be certain.

    So this seems like extra security, but still the answer of "will it work" is still "probably"?

  • David Chen commented
    1 Jul, 2019 04:47pm

    We implemented this feature ourselves but it adds on a bit of a burden to the code. And then the same practice has to be applied to every repository out there. And along with that, the functionality is not perfect. This issue is also coupled with the burden of not being able to trigger workflows properly (I believe the API for triggering workflows is flaky at best and im not sure it exists at the moment). 

    What we mainly wanted was to trigger builds for PRs as well as for the master branch + tags, so to cover that, we went with setting CircleCI such that the trigger would be via commit. But we're missing one case which is when you first make the PR, the build does not happen again when you have this setting.  

  • Rohi Bagaria commented
    17 May, 2019 08:01pm

    We have found a work around where we pull the `master` branch into the current branch before doing the build step. It works for now, but its quite brittle. This workaround won't work when the base branch isn't the `master` branch. It is a very basic feature I would expect in a CI/CD tool.

  • David Chen commented
    9 May, 2019 08:22pm

    For now, isn't this something we can implement ourselves using the features available CircleCI? Some people have also mentioned TravisCI offering that merge test but then they don't do the non-merged test. Doesn't that itself run into issues?

  • Ralf Gommers commented
    9 Mar, 2019 06:00am

    This is a pretty major pain point.The problem is this:

    1. CI for master branch starts failing, for example because a dependency had a new release which causes a problem. (all CI's fail at this point)

    2. A fix is merged into master.

    3. All existing PRs that are updated, as well as all new PRs that are made from a not fully up-to-date master, still fail on CircleCI, but work on TravisCI, Appveyor, Azure Devops, etc.

     

    This is *very* common. CircleCI's default behavior is not useful; every other CI system builds the merge commit.

     

    Note that some people in the original thread suggested re-running the merge commit on every change to either master or the PR. That is not necessary in most cases, and would result in a massive increase in resource usage.

  • Charles Lowell commented
    5 Mar, 2019 07:37pm

    For reference, here is the description of the feature on Travis

     

    Rather than build the commits that have been pushed to the branch the pull request is from, we build the merge between the source branch and the upstream branch.

    https://docs.travis-ci.com/user/pull-requests/#how-pull-requests-are-built

  • Charles Lowell commented
    5 Mar, 2019 07:33pm

    The reason this feature is so important can be summed up with a single question we want to answer with every pull request:

     

    "If I merge this pull request onto master, will it work or will it break?"

     

    The answer a passing CircleCI build gives you today by just using the `checkout` command is "probably"

     

    Maybe a parameter to the "checkout" command? e.g.

     

    steps:
    - checkout:
    merge: true

     

    Either way, seems like answering that fundamental question in a simple yes or no fashion is foundational to the entire concept of CI

  • and 189 more