From 3078fcf55a8aa04d26316ab353d84f2c9512bd47 Mon Sep 17 00:00:00 2001 From: Casper Beyer Date: Mon, 21 Dec 2020 21:04:25 +0800 Subject: [PATCH] feat(unstable): record raw coverage into a directory (#8642) --- cli/flags.rs | 32 +++++-- cli/main.rs | 51 ++++++----- cli/program_state.rs | 7 ++ cli/tests/integration_tests.rs | 12 +++ cli/tests/test_coverage.out | 8 +- cli/tests/test_run_test_coverage.out | 32 +++++++ cli/tests/test_run_test_coverage.ts | 14 +++ cli/tools/coverage.rs | 132 ++++++++++++++++++++------- runtime/worker.rs | 1 + 9 files changed, 221 insertions(+), 68 deletions(-) create mode 100644 cli/tests/test_run_test_coverage.out create mode 100644 cli/tests/test_run_test_coverage.ts diff --git a/cli/flags.rs b/cli/flags.rs index d3fd18c32a..bc08d2ec7a 100644 --- a/cli/flags.rs +++ b/cli/flags.rs @@ -10,6 +10,7 @@ use log::Level; use std::net::SocketAddr; use std::path::PathBuf; use std::str::FromStr; +use tempfile::TempDir; #[derive(Clone, Debug, PartialEq)] pub enum DenoSubcommand { @@ -108,7 +109,7 @@ pub struct Flags { pub ca_file: Option, pub cached_only: bool, pub config_path: Option, - pub coverage: bool, + pub coverage_dir: Option, pub ignore: Vec, pub import_map_path: Option, pub inspect: Option, @@ -614,11 +615,23 @@ fn test_parse(flags: &mut Flags, matches: &clap::ArgMatches) { let allow_none = matches.is_present("allow-none"); let quiet = matches.is_present("quiet"); let filter = matches.value_of("filter").map(String::from); - let coverage = matches.is_present("coverage"); - if coverage { - flags.coverage = true; - } + flags.coverage_dir = if matches.is_present("coverage") { + if let Some(coverage_dir) = matches.value_of("coverage") { + Some(coverage_dir.to_string()) + } else { + Some( + TempDir::new() + .unwrap() + .into_path() + .to_str() + .unwrap() + .to_string(), + ) + } + } else { + None + }; if matches.is_present("script_arg") { let script_arg: Vec = matches @@ -1282,7 +1295,10 @@ fn test_subcommand<'a, 'b>() -> App<'a, 'b> { .arg( Arg::with_name("coverage") .long("coverage") - .takes_value(false) + .min_values(0) + .max_values(1) + .require_equals(true) + .takes_value(true) .requires("unstable") .conflicts_with("inspect") .conflicts_with("inspect-brk") @@ -3050,7 +3066,7 @@ mod tests { #[test] fn test_with_flags() { #[rustfmt::skip] - let r = flags_from_vec_safe(svec!["deno", "test", "--unstable", "--no-run", "--filter", "- foo", "--coverage", "--allow-net", "--allow-none", "dir1/", "dir2/", "--", "arg1", "arg2"]); + let r = flags_from_vec_safe(svec!["deno", "test", "--unstable", "--no-run", "--filter", "- foo", "--coverage=cov", "--allow-net", "--allow-none", "dir1/", "dir2/", "--", "arg1", "arg2"]); assert_eq!( r.unwrap(), Flags { @@ -3063,7 +3079,7 @@ mod tests { include: Some(svec!["dir1/", "dir2/"]), }, unstable: true, - coverage: true, + coverage_dir: Some("cov".to_string()), allow_net: true, argv: svec!["arg1", "arg2"], ..Flags::default() diff --git a/cli/main.rs b/cli/main.rs index b6b6b295b8..cd682498e8 100644 --- a/cli/main.rs +++ b/cli/main.rs @@ -116,7 +116,7 @@ fn create_web_worker_callback( }); let attach_inspector = program_state.maybe_inspector_server.is_some() - || program_state.flags.coverage; + || program_state.coverage_dir.is_some(); let maybe_inspector_server = program_state.maybe_inspector_server.clone(); let module_loader = CliModuleLoader::new_for_worker(program_state.clone()); @@ -192,7 +192,7 @@ pub fn create_main_worker( let attach_inspector = program_state.maybe_inspector_server.is_some() || program_state.flags.repl - || program_state.flags.coverage; + || program_state.coverage_dir.is_some(); let maybe_inspector_server = program_state.maybe_inspector_server.clone(); let should_break_on_first_statement = program_state.flags.inspect_brk.is_some(); @@ -1018,16 +1018,23 @@ async fn test_command( let mut worker = create_main_worker(&program_state, main_module.clone(), permissions); - let mut maybe_coverage_collector = if flags.coverage { - let session = worker.create_inspector_session(); - let mut coverage_collector = - tools::coverage::CoverageCollector::new(session); - coverage_collector.start_collecting().await?; + if let Some(ref coverage_dir) = flags.coverage_dir { + env::set_var("DENO_UNSTABLE_COVERAGE_DIR", coverage_dir); + } - Some(coverage_collector) - } else { - None - }; + let mut maybe_coverage_collector = + if let Some(ref coverage_dir) = program_state.coverage_dir { + let session = worker.create_inspector_session(); + + let coverage_dir = PathBuf::from(coverage_dir); + let mut coverage_collector = + tools::coverage::CoverageCollector::new(coverage_dir, session); + coverage_collector.start_collecting().await?; + + Some(coverage_collector) + } else { + None + }; let execute_result = worker.execute_module(&main_module).await; execute_result?; @@ -1037,19 +1044,19 @@ async fn test_command( worker.run_event_loop().await?; if let Some(coverage_collector) = maybe_coverage_collector.as_mut() { - let coverages = coverage_collector.collect().await?; coverage_collector.stop_collecting().await?; - let filtered_coverages = tools::coverage::filter_script_coverages( - coverages, - main_module.as_url().clone(), - test_modules, - ); - - let mut coverage_reporter = - tools::coverage::PrettyCoverageReporter::new(quiet); - for coverage in filtered_coverages { - coverage_reporter.visit_coverage(&coverage); + // TODO(caspervonb) extract reporting into it's own subcommand. + // For now, we'll only report for the command that passed --coverage as a flag. + if flags.coverage_dir.is_some() { + let mut exclude = test_modules.clone(); + let main_module_url = main_module.as_url().to_owned(); + exclude.push(main_module_url); + tools::coverage::report_coverages( + &coverage_collector.dir, + quiet, + exclude, + )?; } } diff --git a/cli/program_state.rs b/cli/program_state.rs index 008244b5f6..afae8c1255 100644 --- a/cli/program_state.rs +++ b/cli/program_state.rs @@ -45,6 +45,7 @@ pub struct ProgramState { /// Flags parsed from `argv` contents. pub flags: flags::Flags, pub dir: deno_dir::DenoDir, + pub coverage_dir: Option, pub file_fetcher: FileFetcher, pub modules: Arc>>>, @@ -105,8 +106,14 @@ impl ProgramState { None => None, }; + let coverage_dir = flags + .coverage_dir + .clone() + .or_else(|| env::var("DENO_UNSTABLE_COVERAGE_DIR").ok()); + let program_state = ProgramState { dir, + coverage_dir, flags, file_fetcher, modules: Default::default(), diff --git a/cli/tests/integration_tests.rs b/cli/tests/integration_tests.rs index 302e808faf..5696f1f5bc 100644 --- a/cli/tests/integration_tests.rs +++ b/cli/tests/integration_tests.rs @@ -3310,6 +3310,18 @@ itest!(deno_test_coverage { exit_code: 0, }); +itest!(deno_test_coverage_explicit { + args: "test --coverage=.test_coverage --unstable test_coverage.ts", + output: "test_coverage.out", + exit_code: 0, +}); + +itest!(deno_test_run_test_coverage { + args: "test --allow-all --coverage --unstable test_run_test_coverage.ts", + output: "test_run_test_coverage.out", + exit_code: 0, +}); + itest!(deno_lint { args: "lint --unstable lint/file1.js lint/file2.ts lint/ignored_file.ts", output: "lint/expected.out", diff --git a/cli/tests/test_coverage.out b/cli/tests/test_coverage.out index a4b37e4de4..b8423e7fd7 100644 --- a/cli/tests/test_coverage.out +++ b/cli/tests/test_coverage.out @@ -15,11 +15,11 @@ cover [WILDCARD]/cli/tests/subdir/mod1.ts ... 35.714% (5/14) 11 | export function throwsError() { 12 | throw Error("exception from mod1"); 13 | } -cover [WILDCARD]/cli/tests/subdir/subdir2/mod2.ts ... 62.500% (5/8) - 5 | export function printHello2() { - 6 | printHello(); - 7 | } cover [WILDCARD]/cli/tests/subdir/print_hello.ts ... 25.000% (1/4) 1 | export function printHello() { 2 | console.log("Hello"); 3 | } +cover [WILDCARD]/cli/tests/subdir/subdir2/mod2.ts ... 62.500% (5/8) + 5 | export function printHello2() { + 6 | printHello(); + 7 | } diff --git a/cli/tests/test_run_test_coverage.out b/cli/tests/test_run_test_coverage.out new file mode 100644 index 0000000000..bcaae01a12 --- /dev/null +++ b/cli/tests/test_run_test_coverage.out @@ -0,0 +1,32 @@ +Check [WILDCARD]/$deno$test.ts +running 1 tests +test spawn test ... Check [WILDCARD]/$deno$test.ts +running 1 tests +test returnsFooSuccess ... ok ([WILDCARD]) + +test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out ([WILDCARD]) + +ok ([WILDCARD]) + +test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out ([WILDCARD]) + +cover [WILDCARD]/subdir/mod1.ts ... 35.714% (5/14) + 2 | export function returnsHi() { + 3 | return "Hi"; + 4 | } +-----|----- + 8 | export function printHello3() { + 9 | printHello2(); + 10 | } + 11 | export function throwsError() { + 12 | throw Error("exception from mod1"); + 13 | } +cover [WILDCARD]/subdir/print_hello.ts ... 25.000% (1/4) + 1 | export function printHello() { + 2 | console.log("Hello"); + 3 | } +cover [WILDCARD]/subdir/subdir2/mod2.ts ... 62.500% (5/8) + 5 | export function printHello2() { + 6 | printHello(); + 7 | } +cover [WILDCARD]/test_coverage.ts ... 100.000% (5/5) diff --git a/cli/tests/test_run_test_coverage.ts b/cli/tests/test_run_test_coverage.ts new file mode 100644 index 0000000000..e3f0e47ce9 --- /dev/null +++ b/cli/tests/test_run_test_coverage.ts @@ -0,0 +1,14 @@ +Deno.test("spawn test", async function () { + const process = Deno.run({ + cmd: [ + Deno.execPath(), + "test", + "--allow-all", + "--unstable", + "test_coverage.ts", + ], + }); + + await process.status(); + process.close(); +}); diff --git a/cli/tools/coverage.rs b/cli/tools/coverage.rs index 229cb8020f..53385b482b 100644 --- a/cli/tools/coverage.rs +++ b/cli/tools/coverage.rs @@ -7,14 +7,19 @@ use deno_core::serde_json::json; use deno_core::url::Url; use deno_runtime::inspector::InspectorSession; use serde::Deserialize; +use serde::Serialize; +use std::fs; +use std::path::PathBuf; +use uuid::Uuid; pub struct CoverageCollector { + pub dir: PathBuf, session: Box, } impl CoverageCollector { - pub fn new(session: Box) -> Self { - Self { session } + pub fn new(dir: PathBuf, session: Box) -> Self { + Self { dir, session } } pub async fn start_collecting(&mut self) -> Result<(), AnyError> { @@ -33,7 +38,7 @@ impl CoverageCollector { Ok(()) } - pub async fn collect(&mut self) -> Result, AnyError> { + pub async fn stop_collecting(&mut self) -> Result<(), AnyError> { let result = self .session .post_message("Profiler.takePreciseCoverage", None) @@ -42,9 +47,11 @@ impl CoverageCollector { let take_coverage_result: TakePreciseCoverageResult = serde_json::from_value(result)?; - let mut coverages: Vec = Vec::new(); - for script_coverage in take_coverage_result.result { - let result = self + fs::create_dir_all(&self.dir)?; + + let script_coverages = take_coverage_result.result; + for script_coverage in script_coverages { + let get_script_source_value = self .session .post_message( "Debugger.getScriptSource", @@ -55,22 +62,28 @@ impl CoverageCollector { .await?; let get_script_source_result: GetScriptSourceResult = - serde_json::from_value(result)?; + serde_json::from_value(get_script_source_value)?; - coverages.push(Coverage { + let script_source = get_script_source_result.script_source.clone(); + + let coverage = Coverage { script_coverage, - script_source: get_script_source_result.script_source, - }) + script_source, + }; + + // TODO(caspervonb) Would be much better to look up the source during the reporting stage + // instead of storing it here. + // Long term, that's what we should be doing. + let filename = format!("{}.json", Uuid::new_v4()); + let json = serde_json::to_string(&coverage)?; + fs::write(self.dir.join(filename), &json)?; } - Ok(coverages) - } - - pub async fn stop_collecting(&mut self) -> Result<(), AnyError> { self .session .post_message("Profiler.stopPreciseCoverage", None) .await?; + self.session.post_message("Profiler.disable", None).await?; self.session.post_message("Debugger.disable", None).await?; @@ -78,7 +91,9 @@ impl CoverageCollector { } } -#[derive(Debug, Deserialize)] +// TODO(caspervonb) all of these structs can and should be made private, possibly moved to +// inspector::protocol. +#[derive(Debug, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] pub struct CoverageRange { pub start_offset: usize, @@ -86,7 +101,7 @@ pub struct CoverageRange { pub count: usize, } -#[derive(Debug, Deserialize)] +#[derive(Debug, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] pub struct FunctionCoverage { pub function_name: String, @@ -94,7 +109,7 @@ pub struct FunctionCoverage { pub is_block_coverage: bool, } -#[derive(Debug, Deserialize)] +#[derive(Debug, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] pub struct ScriptCoverage { pub script_id: String, @@ -102,7 +117,7 @@ pub struct ScriptCoverage { pub functions: Vec, } -#[derive(Debug, Deserialize)] +#[derive(Debug, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] pub struct Coverage { pub script_coverage: ScriptCoverage, @@ -132,8 +147,12 @@ impl PrettyCoverageReporter { PrettyCoverageReporter { quiet } } - pub fn visit_coverage(&mut self, coverage: &Coverage) { - let lines = coverage.script_source.lines().collect::>(); + pub fn visit_coverage( + &mut self, + script_coverage: &ScriptCoverage, + script_source: &str, + ) { + let lines = script_source.lines().collect::>(); let mut covered_lines: Vec = Vec::new(); let mut uncovered_lines: Vec = Vec::new(); @@ -143,7 +162,7 @@ impl PrettyCoverageReporter { let line_end_offset = line_start_offset + line.len(); let mut count = 0; - for function in &coverage.script_coverage.functions { + for function in &script_coverage.functions { for range in &function.ranges { if range.start_offset <= line_start_offset && range.end_offset >= line_end_offset @@ -167,7 +186,7 @@ impl PrettyCoverageReporter { } if !self.quiet { - print!("cover {} ... ", coverage.script_coverage.url); + print!("cover {} ... ", script_coverage.url); let line_coverage_ratio = covered_lines.len() as f32 / lines.len() as f32; let line_coverage = format!( @@ -212,10 +231,41 @@ impl PrettyCoverageReporter { } } -pub fn filter_script_coverages( +fn collect_coverages(dir: &PathBuf) -> Result, AnyError> { + let mut coverages: Vec = Vec::new(); + + let entries = fs::read_dir(dir)?; + for entry in entries { + let json = fs::read_to_string(entry.unwrap().path())?; + let coverage: Coverage = serde_json::from_str(&json)?; + + coverages.push(coverage); + } + + // TODO(caspervonb) drain_filter would make this cleaner, its nightly at the moment. + if coverages.len() > 1 { + coverages.sort_by_key(|k| k.script_coverage.url.clone()); + + for i in (1..coverages.len() - 1).rev() { + if coverages[i].script_coverage.url + == coverages[i - 1].script_coverage.url + { + let current = coverages.remove(i); + let previous = &mut coverages[i - 1]; + + for function in current.script_coverage.functions { + previous.script_coverage.functions.push(function); + } + } + } + } + + Ok(coverages) +} + +fn filter_coverages( coverages: Vec, - test_file_url: Url, - test_modules: Vec, + exclude: Vec, ) -> Vec { coverages .into_iter() @@ -225,20 +275,16 @@ pub fn filter_script_coverages( return false; } - if url == test_file_url { - return false; - } - - for test_module_url in &test_modules { - if &url == test_module_url { + for module_url in &exclude { + if &url == module_url { return false; } } if let Ok(path) = url.to_file_path() { - for test_module_url in &test_modules { - if let Ok(test_module_path) = test_module_url.to_file_path() { - if path.starts_with(test_module_path.parent().unwrap()) { + for module_url in &exclude { + if let Ok(module_path) = module_url.to_file_path() { + if path.starts_with(module_path.parent().unwrap()) { return true; } } @@ -250,3 +296,21 @@ pub fn filter_script_coverages( }) .collect::>() } + +pub fn report_coverages( + dir: &PathBuf, + quiet: bool, + exclude: Vec, +) -> Result<(), AnyError> { + let coverages = collect_coverages(dir)?; + let coverages = filter_coverages(coverages, exclude); + + let mut coverage_reporter = PrettyCoverageReporter::new(quiet); + for coverage in coverages { + let script_coverage = coverage.script_coverage; + let script_source = coverage.script_source; + coverage_reporter.visit_coverage(&script_coverage, &script_source); + } + + Ok(()) +} diff --git a/runtime/worker.rs b/runtime/worker.rs index adb525c4c9..58a35cc950 100644 --- a/runtime/worker.rs +++ b/runtime/worker.rs @@ -87,6 +87,7 @@ impl MainWorker { } else { None }; + let should_break_on_first_statement = inspector.is_some() && options.should_break_on_first_statement;