-
Notifications
You must be signed in to change notification settings - Fork 6.8k
Correct PR branch detection in code coverage #12615
Conversation
Closing to trigger branch job |
I have verified that this change d724c8c works for PRs that have been submitted from the main repo as well as from a user fork. Branch validation is pending at http://jenkins.mxnet-ci.amazon-ml.com/blue/organizations/jenkins/incubator-mxnet/detail/fix-testcoverage-report/2/pipeline |
Branch validation successful. Everything's working as expected now :) |
It seems like it just broke again. Please don't merge |
Ah! I found something: These two runs reference the same commit d724c8c. But the SCM output is different:
The key difference here is that during the first run, my PRs base was the same as the HEAD of the master branch: 7797584 and my branch was 2 commits ahead and 0 behind. Thus, no merge was necessary but a simple fast-forward was applied. During the second run, 3401e6e was pushed to the master branch and thus my branch was 2 commits ahead and 1 behind. This requires a merge commit. This means that I don't have to make this logic conditional on whether the current run is part of a PR or a regular branch but rather parse the current contexts' HEAD and check whether it matches "Merge commit '$HASH$' into HEAD". |
077abb1
to
b5545b1
Compare
b5545b1
to
64f55cf
Compare
368593a
to
0ed6a10
Compare
The branch detection in the code coverage script was broken because a non-existing environment variable in groovy is considered null opposed to other documentation I've found that states it as being an empty string.
A CI run to test the branch detection in a non-PR job is available below (that's why I created the branch in the main repository instead of my own fork).
A CI run to test the branch detection in a PR job is the regular PR validation.
Accept criteria is both reports referencing the same commit hash.
Background for the change was that during my testing I used
git log
while I switched torev-parse
during last minute. Since CI automatically moves the currently checked out commit to the right one,rev-parse
is actually working for PR-merge as well as for Branch validation.