Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue 91: npm run posttest' fails when COVERALLS_REPO_TOKEN is unset #92

Conversation

farmisen
Copy link

#91

Testing correctly if the COVERALLS_REPO_TOKEN env variable is not empty.

@meeber
Copy link
Contributor

meeber commented May 14, 2016

@farmisen thanks for the PR!

The problem with this change is that Travis CI will no longer submit code coverage to Coveralls because $COVERALLS_REPO_TOKEN isn't set in that environment. In fact, I suspect that's why the test script was coded this way in the first place.

If my suspicion is correct, and it was originally coded this way on purpose, then I'd say there's room for improvement so that it continues submitting code coverage to Coveralls when run via Travis CI, but doesn't throw an error in a non-Travis environment where $COVERALLS_REPO_TOKEN isn't set.

@farmisen
Copy link
Author

got it - going to see what I can do.

@farmisen farmisen force-pushed the Issue-91_npm_run_posttest_fails_when_COVERALLS_REPO_TOKEN_is_unset branch 2 times, most recently from 0f3c4f3 to a35b64c Compare May 15, 2016 00:50
… order to be able use the coveralls service during the CI process without explicitly providing a coveralls token in the environment.
@farmisen farmisen force-pushed the Issue-91_npm_run_posttest_fails_when_COVERALLS_REPO_TOKEN_is_unset branch from a35b64c to 642c455 Compare May 15, 2016 00:58
@farmisen
Copy link
Author

@meeber, I did push some changes to make sure coveralls was invoked after a successful travis run and I have checked at https://coveralls.io/github/chaijs/chai-http and the coverage data for this PR are showing yet they still show as pending on this page. I am not familiar with coveralls but I am wondering if it could not be a config problem?

@meeber
Copy link
Contributor

meeber commented May 15, 2016

@farmisen let's wait for @keithamus before doing any more work on this.

My personal opinion is that code coverage should only be submitted to Coveralls when either:

  1. The test script is run from within a Travis CI environment
  2. A user manually launches a Coveralls-specific script

Your PR addresses the first point, but it also submits code coverage to Coveralls by default when $COVERALLS_REPO_TOKEN is present. I suspect this isn't desired functionality, but I'm just speculating, so let's wait for @keithamus :D

@keithamus
Copy link
Member

Thanks for the PR @farmisen! Thanks for pinging me on this one @meeber.

I was actually planning on going through all of the chai projects and standardising the build & test process using type-detect as a base (see https://github.com/chaijs/type-detect/blob/master/package.json#L25-L36). @farmisen how do you feel about taking what is in type-detect and reworking this PR to feature that?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants