codeclimate / ruby-test-reporter

DEPRECATED Uploads Ruby test coverage data to Code Climate
https://codeclimate.com
Other
92 stars 92 forks source link

Pass thru Travis PR SHA #153

Closed maxjacobson closed 7 years ago

maxjacobson commented 7 years ago

When Travis builds a PR, as opposed to a push, it checks out the merge commit that GitHub exposes and runs the tests at that SHA.

When we report the results, however, we'd like to report them for the HEAD SHA from the PR, because otherwise the status won't show up.

it's possible the coverage results will differ slightly between the merge SHA and the HEAD sha, but I'm not sure if anyone will be offended by this fiction. We've had one report that a user is surprised not to receive a status check when they only have Pull Request builds enabled, and not Push builds.

:warning: This is slightly WIP, as I do some testing on a test repo to confirm it works

dblandin commented 7 years ago

Wouldn't we want to use TRAVIS_COMMIT which should be available for both pushes and PRs?

TRAVIS_COMMIT: The commit that the current build is testing.

https://docs.travis-ci.com/user/environment-variables/

maxjacobson commented 7 years ago

When a user has their Travis CI configured to build PRs but not pushes, the TRAVIS_COMMIT is going to be different from the HEAD commit on the PR. The idea is that it will try to answer the question of "will this still be green when I merge it?". That is a helpful question to answer. But, it's not really helpful for us, because when we receive a payload that the TRAVIS_COMMIT sha has 97% coverage, and we try to update the status of that commit on GitHub to have a status check, we're updating for a SHA that's not on the PR, and so the PR doesn't receive a coverage status check.

maxjacobson commented 7 years ago

@dblandin I think I'm misrepresenting a bit, actually, and this change won't fix the problem. Going to dig a little deeper

maxjacobson commented 7 years ago

OK after talking about this IRL we agreed it's not a desired behavior.