-
Notifications
You must be signed in to change notification settings - Fork 934
Instrument Repo for Code Coverage #268
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
Conversation
Approved for RTDB. Got the coverage numbers that I needed. Thanks! |
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.
LGTM
@@ -27,9 +27,18 @@ module.exports = { | |||
use: 'ts-loader' | |||
}, | |||
{ | |||
test: /\.js$/, | |||
use: ['source-map-loader'], | |||
test: /\.[tj]sx?$/, |
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.
Is this just a standard configuration now or are we really doing jsx and tsx stuff? In contrast in other locations we're still using just straight .ts
file extensions.
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.
It's just a standard thing, we have no need for jsx/tsx so it isn't that big of a deal I don't think.
FYI- It seems like you upgraded prettier in this PR, which caused a bunch of extra formatting changes, which is causing me merge conflicts in a different branch. Going forward it would be helpful if:
|
This is going to probably take a little extra time (we have to get the extra CI integration approved) but having this instrumented will give us a path to improving our coverage.
Instrumenting files
Browser
All of our browser builds running through
karma-webpack
are instrumented viaistanbul-instrumenter-loader
. We then dump this info for each component into the componentcoverage/browser
dir by browser.Node
All of our node builds are instrumented via
nyc
which is a tool provided by the istanbul folks. Unfortunately, karma is not supported in the same way so we needed two tools. Coverage for this tool is dumped in thecoverage/node
directory.Merging
lcov.info
FilesAfter collecting
lcov.info
files (i.e. the coverage reports) for each file we leveragelcov-result-merger
to consolidate to a single file that we can then pipe to CI.I am going to leave the CI integration (i.e.
coveralls
) in as @vikrum is currently evaluating if we can get access here.