-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
fix(cli): panic with deno coverage
#23353
fix(cli): panic with deno coverage
#23353
Conversation
… lol
deno coverage
deno coverage
Thanks for the PR, could you please add a test case that exercises this change? You can add an |
My apologies @bartlomieju — I missed your initial reply to this PR. I've gone ahead and added a test at |
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.
Looks good, thanks for the PR. Could you please use assert_contains!
macro?
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, thanks!
This PR directly addresses the issue raised in #23282 where Deno panics if `deno coverage` is called with `--include` regex that returns no matches. I've opted not to change the return value of `collect_summary` for simplicity and return an empty `HashMap` instead
This PR directly addresses the issue raised in #23282 where Deno panics if
deno coverage
is called with--include
regex that returns no matches.I've opted not to change the return value of
collect_summary
for simplicity and return an emptyHashMap
instead