-
Notifications
You must be signed in to change notification settings - Fork 84
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
feat: rework .nycrc to support --all #226
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good start! Just flagging this as do not merge
so we make sure the updates to all other libs go out first.
"src/*{/*,/**/*}.js", | ||
"src/*/v*/*.js", | ||
"test/**/*.js", | ||
"build/test" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I notice this doesn't do anything funky with TypeScript. Are we getting coverage from what's in build/src
, or from src/
? How do we know this isn't being counted twice?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Coverage uses source-maps, so in reports it will point the user to the files in src/ as the files missing coverage; in practice it’s always the files in build/src that are executed, but will not show up in reports due to them including source-maps.
So this LGTM. Just please submit a hammer PR first, then lets land this one. |
what is this?
This reworks our
.nycrc
to better support the flag--all
; this lets us know we've missed writing coverage for files:Note: this update requires nyc@14.
oddities
I think the rules:
were an attempt to not collect coverage in the
src/v1
,src/v3beta1
directory, etc."src/*{/*,/**/*}.js",
feels a little heavy handed, and was stopping us from collecting coverage for some libraries.src/v1
? seems like in a perfect world we'd eventually have tests that exercise these APIs?