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
Use c8's --merge-async option when generating coverage from the root #18910
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.
Tangent to this PR: I question a bit if we should have c8 as a devDependency on every package? The way we do test coverage is by running with c8 from the root package.json and just running test:mocha
or similar on each package. Some packages do have a test:coverage
script that uses c8 but AFAIK we don't leverage them?
⯅ @fluid-example/bundle-size-tests: +253 Bytes
Baseline commit: 837c4ba |
yeah. it seems weird to me too. |
Description
This should resolve the intermittent OOM issues we've been seeing on CI. The cause of them appears was this issue, as by default c8 attempts to merge coverage reports by loading them all into memory at the same time. Our root c8 config includes many files, so this can trigger OOM issues.
I've updated the repo to c8@8.0.1, since we needed to bump the c8 dependency anyway to use the merge-async option and the only breaking change between 7 and 8 was dropping node 10 support.