-
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
feat(test): create coverage reports when --coverage specified in deno test #28260
base: main
Are you sure you want to change the base?
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.
I think this is a good idea 👍
) -> Result<(), AnyError> { | ||
if coverage_flags.files.include.is_empty() { | ||
if files_include.is_empty() { |
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 believe this function should remove previously generated coverage before dumping new reports to the directory. Otherwise the results might be skewed from previous runs. WDYT?
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.
Updated. Now deno test --coverage
clears the reports & raw data from the previous runs. It still keeps the contents of coverage_dir when --coverage-raw-data-only
specified (and --clean
not specified) to support the advanced scenarios.
cli/tools/coverage/reporter.rs
Outdated
_coverage_root: &Path, | ||
file_reports: &[(CoverageReport, String)], | ||
) { | ||
file_reports.iter().for_each(|(report, file_text)| { | ||
self.report(report, file_text).unwrap(); | ||
}); | ||
if let Some((report, _)) = file_reports.first() { | ||
if let Some(ref output) = report.output { | ||
let path = output.canonicalize().unwrap().to_string_lossy().to_string(); |
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.
This unwrap is sus here 😬 maybe consider failing gracefully with an error message? Also you don't need to do the conversion to string first - Url::from_file_path
will accept a PathBuf
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.
Updated. Now it prints error message when .canonicalize
failed
part of #21325
closes #28218
This PR updates the behavior of
deno test --coverage
option. Now if--coverage
option is specified,deno test
command automatically shows summary report in the terminal, and generates the lcov report in$coverage_dir/lcov.info
and html report in$coverage_dir/html/
This change also adds
--coverage-raw-data-only
flag, which prevents the above reports generated, instead only generates the raw json coverage data (which is the same as current behavior)An example of execution looks like:
Note:
jest --coverage
outputs 'summary' and 'lcov' reports by default. This outputs 'html' reports in addition to them.Planning to land for Deno 2.3