From 4ca77ad84c46ba4afef95f0b6775377d350b7339 Mon Sep 17 00:00:00 2001 From: Casper Beyer Date: Tue, 5 Jan 2021 00:01:21 +0800 Subject: [PATCH] fix(coverage): merge duplicate reports (#8942) Merging multiple runs isn't quite right because we rely on a 0 count to signal that a block hasn't been called. Other tools like c8 expect this to be true as-well so we need to do our best to merge coverage files rather than duplicating them. --- cli/tests/integration_tests.rs | 6 +++ cli/tests/run_coverage.ts | 4 +- cli/tests/test_run_combined_coverage.out | 31 +++++++++++++ cli/tests/test_run_run_coverage.out | 16 ++++--- cli/tools/coverage.rs | 55 ++++++++++++++++-------- 5 files changed, 85 insertions(+), 27 deletions(-) create mode 100644 cli/tests/test_run_combined_coverage.out diff --git a/cli/tests/integration_tests.rs b/cli/tests/integration_tests.rs index 9dc4df7cb0..199172e2c2 100644 --- a/cli/tests/integration_tests.rs +++ b/cli/tests/integration_tests.rs @@ -3330,6 +3330,12 @@ itest!(deno_test_run_run_coverage { exit_code: 0, }); +itest!(deno_test_run_combined_coverage { + args: "test --allow-all --coverage --unstable test_run_run_coverage.ts test_run_test_coverage.ts", + output: "test_run_combined_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/run_coverage.ts b/cli/tests/run_coverage.ts index 098d463e8d..d1443dfadf 100644 --- a/cli/tests/run_coverage.ts +++ b/cli/tests/run_coverage.ts @@ -1,3 +1,3 @@ -import { returnsFoo2 } from "./subdir/mod1.ts"; +import { returnsHi } from "./subdir/mod1.ts"; -returnsFoo2(); +returnsHi(); diff --git a/cli/tests/test_run_combined_coverage.out b/cli/tests/test_run_combined_coverage.out new file mode 100644 index 0000000000..476e9adcc7 --- /dev/null +++ b/cli/tests/test_run_combined_coverage.out @@ -0,0 +1,31 @@ +Check [WILDCARD]/tests/$deno$test.ts +running 2 tests +test spawn test ... Check [WILDCARD]/tests/run_coverage.ts +ok ([WILDCARD]) +test spawn test ... Check [WILDCARD]/tests/$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. 2 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out ([WILDCARD]) + +cover [WILDCARD]/tests/run_coverage.ts ... 100.000% (3/3) +cover [WILDCARD]/tests/subdir/mod1.ts ... 57.143% (8/14) + 8 | export function printHello3() { + 9 | printHello2(); + 10 | } + 11 | export function throwsError() { + 12 | throw Error("exception from mod1"); + 13 | } +cover [WILDCARD]/tests/subdir/print_hello.ts ... 25.000% (1/4) + 1 | export function printHello() { + 2 | console.log("Hello"); + 3 | } +cover [WILDCARD]/tests/subdir/subdir2/mod2.ts ... 62.500% (5/8) + 5 | export function printHello2() { + 6 | printHello(); + 7 | } +cover [WILDCARD]/tests/test_coverage.ts ... 100.000% (5/5) diff --git a/cli/tests/test_run_run_coverage.out b/cli/tests/test_run_run_coverage.out index 15ea230ef3..2f18020d3f 100644 --- a/cli/tests/test_run_run_coverage.out +++ b/cli/tests/test_run_run_coverage.out @@ -1,16 +1,15 @@ -Check [WILDCARD]/$deno$test.ts +Check [WILDCARD]/tests/$deno$test.ts running 1 tests -test spawn test ... Check [WILDCARD]/run_coverage.ts +test spawn test ... Check [WILDCARD]/tests/run_coverage.ts ok ([WILDCARD]) test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out ([WILDCARD]) cover [WILDCARD]/tests/run_coverage.ts ... 100.000% (3/3) cover [WILDCARD]/tests/subdir/mod1.ts ... 35.714% (5/14) - 2 | export function returnsHi() { - 3 | return "Hi"; - 4 | } ------|----- + 5 | export function returnsFoo2() { + 6 | return returnsFoo(); + 7 | } 8 | export function printHello3() { 9 | printHello2(); 10 | } @@ -21,7 +20,10 @@ cover [WILDCARD]/tests/subdir/print_hello.ts ... 25.000% (1/4) 1 | export function printHello() { 2 | console.log("Hello"); 3 | } -cover [WILDCARD]/tests/subdir/subdir2/mod2.ts ... 62.500% (5/8) +cover [WILDCARD]/tests/subdir/subdir2/mod2.ts ... 25.000% (2/8) + 2 | export function returnsFoo() { + 3 | return "Foo"; + 4 | } 5 | export function printHello2() { 6 | printHello(); 7 | } diff --git a/cli/tools/coverage.rs b/cli/tools/coverage.rs index 53385b482b..7cf98cab2b 100644 --- a/cli/tools/coverage.rs +++ b/cli/tools/coverage.rs @@ -93,7 +93,7 @@ impl CoverageCollector { // TODO(caspervonb) all of these structs can and should be made private, possibly moved to // inspector::protocol. -#[derive(Debug, Serialize, Deserialize)] +#[derive(Debug, Serialize, Deserialize, Clone)] #[serde(rename_all = "camelCase")] pub struct CoverageRange { pub start_offset: usize, @@ -101,7 +101,7 @@ pub struct CoverageRange { pub count: usize, } -#[derive(Debug, Serialize, Deserialize)] +#[derive(Debug, Serialize, Deserialize, Clone)] #[serde(rename_all = "camelCase")] pub struct FunctionCoverage { pub function_name: String, @@ -109,7 +109,7 @@ pub struct FunctionCoverage { pub is_block_coverage: bool, } -#[derive(Debug, Serialize, Deserialize)] +#[derive(Debug, Serialize, Deserialize, Clone)] #[serde(rename_all = "camelCase")] pub struct ScriptCoverage { pub script_id: String, @@ -117,7 +117,7 @@ pub struct ScriptCoverage { pub functions: Vec, } -#[derive(Debug, Serialize, Deserialize)] +#[derive(Debug, Serialize, Deserialize, Clone)] #[serde(rename_all = "camelCase")] pub struct Coverage { pub script_coverage: ScriptCoverage, @@ -237,29 +237,48 @@ fn collect_coverages(dir: &PathBuf) -> Result, AnyError> { 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)?; + let new_coverage: Coverage = serde_json::from_str(&json)?; - coverages.push(coverage); - } + let existing_coverage = coverages + .iter_mut() + .find(|x| x.script_coverage.url == new_coverage.script_coverage.url); - // 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()); + if let Some(existing_coverage) = existing_coverage { + for new_function in new_coverage.script_coverage.functions { + let existing_function = existing_coverage + .script_coverage + .functions + .iter_mut() + .find(|x| x.function_name == new_function.function_name); - 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]; + if let Some(existing_function) = existing_function { + for new_range in new_function.ranges { + let existing_range = + existing_function.ranges.iter_mut().find(|x| { + x.start_offset == new_range.start_offset + && x.end_offset == new_range.end_offset + }); - for function in current.script_coverage.functions { - previous.script_coverage.functions.push(function); + if let Some(existing_range) = existing_range { + existing_range.count += new_range.count; + } else { + existing_function.ranges.push(new_range); + } + } + } else { + existing_coverage + .script_coverage + .functions + .push(new_function); } } + } else { + coverages.push(new_coverage); } } + coverages.sort_by_key(|k| k.script_coverage.url.clone()); + Ok(coverages) }