archived-codacy / node-codacy-coverage

Code Coverage reporter for Codacy
MIT License
63 stars 45 forks source link

Pull Requests Fail When Testing GetGitData to Retrieve Commit Id #4

Closed DavidTPate closed 9 years ago

DavidTPate commented 9 years ago

What's happening is that Travis is setting the TRAVIS_COMMIT environment variable to the commit associated with the build but since this test is for a merge it actually has a different commit id which causes the test case to fail. The reason why this is just now showing itself (as opposed to previously) is because Travis just recently added the reporting of Pull Request merged builds to their Github integration.

I'm not sure yet how to address this 100% and still keep the accuracy of the test. But the Environment variables Travis provides can be found here.

DavidTPate commented 9 years ago

@rtfpessoa The code in question is located here. I mention the cause in the description.

I'm thinking the correct fix here is to do the following:

if (actualTravisCommit && process.env.TRAVIS_PULL_REQUEST !== 'true') {
  return expect(getGitData.getCommitId()).to.eventually.equal(actualTravisCommit);
}
return expect(getGitData.getCommitId()).to.eventually.be.fulfilled;

I'd rather test the value during a Pull Request but I don't think we have any way to get the commit id for the merge that isn't through our CLI interface (and can be considered trustworthy). What do you think?

rtfpessoa commented 9 years ago

@DavidTPate I don't have much experience with travis, but for what I saw that seems to be a good solution.

DavidTPate commented 9 years ago

Looks like I missed a piece of Travis's documentation. The environment variable will never be "true" but instead will be "false" for pull requests. Here's what the docs say about it: The pull request number if the current job is a pull request, "false" if it's not a pull request.

Going to close this issue as we are taking care of it in #5