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.

Sign up for GitHub

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

Jump to bottom

Coverage is returning all 0s #329

Closed
alangpierce opened this issue Nov 5, 2018 · 2 comments · Fixed by #332
Closed

Coverage is returning all 0s #329

alangpierce opened this issue Nov 5, 2018 · 2 comments · Fixed by #332

Comments

@alangpierce
Copy link
Owner

See the builds for these PRs: #327 #328

Looks like nyc is correctly running and producing a report where all files have 0 lines covered. The most recent master build and PR build worked fine, but now it appears broken even with only a README change.

cc @raylu in case you feel like taking a look. 😄
@alangpierce alangpierce mentioned this issue Nov 5, 2018
Merged
@raylu
Copy link
Contributor

raylu commented Nov 7, 2018

observations:

locally, node_modules/.bin/nyc yarn test-only works but yarn test does not
locally, setting "test": "env -i \"PWD=$PWD\" \"PATH=/bin:/usr/bin\" node_modules/.bin/nyc yarn test-only" works
on travis, raylu@0a04408 does not work (https://travis-ci.org/raylu/sucrase/jobs/451730806)

@alangpierce
Copy link
Owner Author

I figured it out! Comparing https://travis-ci.org/alangpierce/sucrase/jobs/448013956 and https://travis-ci.org/alangpierce/sucrase/jobs/450701078 , I noticed that the only change is in the yarn version, and indeed I can reproduce the failure on latest yarn. Looks like yarn changed something so that nyc yarn test-only no longer works with nyc's magical instrumentation. It's already filed on nyc as istanbuljs/nyc#921 . Looks like options are to either downgrade yarn or invoke nyc on node directly (or some similar approach).

alangpierce added a commit that referenced this issue Nov 11, 2018
@alangpierce
Fix coverage, update CI build in various ways
9837806 
Fixes #329

* Rather than using `nyc` to wrap `yarn` to wrap `mocha`, we now just have `nyc`
  wrap `mocha` directly. This fixes a regression where a yarn upgrade broken
  coverage.
* We now run node 11 by default, with extra builds for node 8 and 10.
* source-map-support didn't seem to be doing anything, so I removed it.
* I made plain `yarn test` no longer run coverage. It's now opt-in as
  `yarn run-with-coverage`. This means that builds that run tests but don't
  report coverage won't be needlessly computing coverage.
* I got rid of the `no-unused-variable` TSLint rule, which has a deprecation
  warning now.
@alangpierce alangpierce mentioned this issue Nov 11, 2018
Merged
alangpierce added a commit that referenced this issue Nov 11, 2018
@alangpierce
Fix coverage, update CI build in various ways
ee590a0 
Fixes #329

* Rather than using `nyc` to wrap `yarn` to wrap `mocha`, we now just have `nyc`
  wrap `mocha` directly. This fixes a regression where a yarn upgrade broken
  coverage.
* We now run node 11 by default, with extra builds for node 8 and 10.
* source-map-support didn't seem to be doing anything, so I removed it.
* I made plain `yarn test` no longer run coverage. It's now opt-in as
  `yarn run-with-coverage`. This means that builds that run tests but don't
  report coverage won't be needlessly computing coverage.
* I got rid of the `no-unused-variable` TSLint rule, which has a deprecation
  warning now.
alangpierce added a commit that referenced this issue Nov 11, 2018
@alangpierce
Fix coverage, update CI build in various ways
4e8167f 
Fixes #329

* Rather than using `nyc` to wrap `yarn` to wrap `mocha`, we now just have `nyc`
  wrap `mocha` directly. This fixes a regression where a yarn upgrade broken
  coverage.
* source-map-support didn't seem to be doing anything, so I removed it.
* I made plain `yarn test` no longer run coverage. It's now opt-in as
  `yarn run-with-coverage`. This means that builds that run tests but don't
  report coverage won't be needlessly computing coverage.
* I got rid of the `no-unused-variable` TSLint rule, which has a deprecation
  warning now.
* Some scripts weren't setting exit code properly, so this fixes that.
alangpierce added a commit that referenced this issue Nov 11, 2018
@alangpierce
Fix coverage, update CI build in various ways (#332)
3032f6b 
Fixes #329

* Rather than using `nyc` to wrap `yarn` to wrap `mocha`, we now just have `nyc`
  wrap `mocha` directly. This fixes a regression where a yarn upgrade broken
  coverage.
* source-map-support didn't seem to be doing anything, so I removed it.
* I made plain `yarn test` no longer run coverage. It's now opt-in as
  `yarn run-with-coverage`. This means that builds that run tests but don't
  report coverage won't be needlessly computing coverage.
* I got rid of the `no-unused-variable` TSLint rule, which has a deprecation
  warning now.
* Some scripts weren't setting exit code properly, so this fixes that.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix coverage, update CI build in various ways alangpierce/sucrase
2 participants
@raylu @alangpierce