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
base: main
Are you sure you want to change the base?
Conversation
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?
assert!( | ||
actual.contains(expected_message), | ||
"Expected output to contain '{}', but found:\n{}", | ||
expected_message, | ||
actual | ||
); |
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.
You can use assert_contains!
macro here to skip manually creating a message
assert!( | ||
unexpected_contents.is_empty(), | ||
"Expected the coverage directory to be empty except for 'empty_dir', but found: {:?}", | ||
unexpected_contents | ||
); |
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 strange that rustfmt
didn't dedent this... oh well
unexpected_contents | ||
); | ||
|
||
println!("No unexpected coverage files found as expected, test concludes successfully."); |
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.
Maybe skip this line, it won't get printed by default anyway
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