GitHub: AppVeyor does not test the right commit

Ralf's Avatar

Ralf

08 Nov, 2018 09:00 AM

If the PR branch gets pushed to between AppVeyor being first notified and AppVeyor actually starting to run the build job, it does not actually build the commit it says it builds.

Take https://ci.appveyor.com/project/solson63299/miri/builds/20139267/job/pu5vr3ouura9sg6r for example: The header says this tests commit c29ec9b9, but the build log shows that it installed version nightly-2018-11-08 of rust whereas that commit of miri still was based on nightly-2018-11-07 as can be seen at https://github.com/solson/miri/blob/c29ec9b9d3cc4c82e1d9e1ff8f330ad0d6544948/rust-version. The reason for this is likely that I pushed a new commit changing the Rust version to that branch before the job got started. But a job shouldn't pick up commits that were not present when it got queued!

The build log also clearly shows why this is happening:
> git fetch -q origin +refs/pull/513/merge:
> git checkout -qf FETCH_HEAD

Even though there is a commit ID shown in the job description, the job makes no effort at actually testing that commit. I think this is a rather serious problem, as it is crucial that CI actually tests the things we think it tests.

  1. Support Staff 1 Posted by Owen McDonnell on 09 Nov, 2018 05:40 PM

    Owen McDonnell's Avatar

    On a PR AppVeyor builds a virtual merge commit of the two commits shown. The head commit of the feature branch being merged into the head commit of the base branch.

  2. 2 Posted by Ralf Jung on 09 Nov, 2018 05:49 PM

    Ralf Jung's Avatar

    I see, I was not aware of the merging.
    Still, it should then merge the actually shown commit into master. But at the
    point the job I showed ran, neither master nor the given commit had the new Rust
    version. Instead, it picked up commits that were added to the feature branch
    *after* the commit it shows in the job page. Effectively, the commit shown on
    the job page is useless information.
    FWIW, other CI services do not have this problem (Travis, for example).

    On 09.11.18 18:40, Owen McDonnell wrote:

  3. 3 Posted by Ralf Jung on 16 Apr, 2019 07:25 PM

    Ralf Jung's Avatar

    > AppVeyor builds a virtual merge commit of the two commits shown.

    In fact I am pretty sure now that this is just not the case. Looking at https://ci.appveyor.com/project/rust-lang-libs/miri/builds/23897129/job/xt5ixv0893cgtv2e, neither of the two commits shown at the top contained the change that lead to the build failure later.

    This matches what one can see in the build script:
    > git fetch -q origin +refs/pull/692/merge:
    > git checkout -qf FETCH_HEAD

    It just picks whatever is the current state of the PR, entirely disregarding which commit it should be testing.

    This can, obviously, lead to wrong results being reported, so it seems like quite a severe problem to me.

  4. Support Staff 4 Posted by Owen McDonnell on 23 Apr, 2019 01:33 PM

    Owen McDonnell's Avatar

    Sorry for the delay in getting back to you.
    An issue has been created.

  5. Ilya Finkelshteyn closed this discussion on 23 Jun, 2019 09:01 PM.

Comments are currently closed for this discussion. You can start a new one.

Keyboard shortcuts

Generic

? Show this help
ESC Blurs the current field

Comment Form

r Focus the comment reply box
^ + ↩ Submit the comment

You can use Command ⌘ instead of Control ^ on Mac