Skip to content
This repository was archived by the owner on Dec 2, 2024. It is now read-only.

Add nyc and browser code coverage #169

Merged
merged 6 commits into from
Jul 4, 2018
Merged

Add nyc and browser code coverage #169

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
70592c6
remove old coverage setup
vweevers Jul 1, 2018
720a9ac
add nyc and coveralls
vweevers Jul 1, 2018
2b83f8e
temporarily depend on airtap/airtap#coverage
vweevers Jul 1, 2018
aed8e83
coverage is opt-in now
vweevers Jul 1, 2018
4cc2334
remove faucet
vweevers Jul 1, 2018
923de0f
Switch to airtap@0.0.9
Jul 4, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
node_modules
coverage
npm-debug.log
.nyc_output
4 changes: 2 additions & 2 deletions .travis.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@ matrix:
env: CMD=test
- node_js: 10
env: CMD=test
- node_js: stable
env: CMD=report-coverage
- node_js: stable
env: CMD=test-browsers
addons:
Expand All @@ -24,5 +22,7 @@ matrix:
script:
- DEBUG=airtap:* npm run $CMD

after_success: npm run coverage

notifications:
email: false
15 changes: 7 additions & 8 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,10 @@
"safe-buffer": "~5.1.1"
},
"devDependencies": {
"airtap": "^0.0.8",
"airtap": "github:airtap/airtap#coverage",
"coveralls": "^3.0.2",
"faucet": "^0.0.1",
"istanbul": "^0.4.2",
"istanbul-coveralls": "^1.0.3",
"nyc": "^12.0.2",
"rimraf": "^2.6.2",
"standard": "^11.0.0",
"tape": "^4.8.0"
Expand All @@ -38,11 +38,10 @@
"./immediate.js": "./immediate-browser.js"
},
"scripts": {
"test": "standard && node test.js | faucet",
"test-browsers": "airtap --sauce-connect --loopback airtap.local --no-coverage test.js",
"test-browser-local": "airtap --no-coverage --local 9000 test.js",
"coverage": "istanbul cover -i memdown.js ./node_modules/.bin/tape ./test.js && istanbul check-coverage --lines 90 --function 80 --statements 90 --branches 80",
"report-coverage": "npm run coverage && istanbul-coveralls"
"test": "standard && nyc --silent node test.js | faucet",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI @ralphtheninja --silent disables the coverage summary at the end of test output

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should enable it and remove faucet.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm okay with either. On the one hand, faucet reduces the output, on the other, the coverage summary is kinda nice (though when coverage matters, I prefer using nyc report --reporter=html).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ralphtheninja your call

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be optimal if we could have both faucet and the coverage report (maybe we can figure out a solution for this somehow?).

I just think it's really useful to have that report while testing locally. "Did I fuck up coverage? Did coverage change?"

Copy link
Member

@ralphtheninja ralphtheninja Jul 1, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maye we can just hack faucet to let stuff pass through. I'm guessing it's faucet that prevents the nyc output?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

level-faucet :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably difficult because tap-parser considers the test to be complete after

1..735     
# tests 735
# pass  735
           
# ok

Perhaps when tap-parser emits "complete", we can switch stdin to pipe to stdout.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ralphtheninja it's a rabbit hole, mind if open an issue for later?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ralphtheninja it's a rabbit hole, mind if open an issue for later?

Oh definitely. Lets solve this later. I'm happy as long as I can get the coverage report. Let's just remove faucet now and bring it back later.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does --silent do?

Copy link
Member Author

@vweevers vweevers Jul 1, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See above ;) #169 (comment)

"test-browsers": "airtap --sauce-connect --loopback airtap.local --coverage test.js",
"test-browser-local": "airtap --coverage --local 9000 test.js",
"coverage": "nyc report --reporter=text-lcov | coveralls"
},
"license": "MIT",
"engines": {
Expand Down