From 3d313bfd8035604e1d603e6585fe2b3ec81f063e Mon Sep 17 00:00:00 2001 From: Gabriel Grubba <70247653+Grubba27@users.noreply.github.com> Date: Wed, 2 Feb 2022 18:19:00 -0300 Subject: [PATCH 1/5] Created Draft PR --- src/file_filter.rs | 2 +- src/main.rs | 136 ++++++++++++++++++++++-------------------- src/path_rewriting.rs | 9 +-- 3 files changed, 76 insertions(+), 71 deletions(-) diff --git a/src/file_filter.rs b/src/file_filter.rs index 493d1b58b..58ebc87f5 100644 --- a/src/file_filter.rs +++ b/src/file_filter.rs @@ -7,7 +7,7 @@ pub enum FilterType { Both(u32), } -#[derive(Default)] +#[derive(Default, Clone)] pub struct FileFilter { excl_line: Option, excl_start: Option, diff --git a/src/main.rs b/src/main.rs index c3252f36f..0533d859c 100644 --- a/src/main.rs +++ b/src/main.rs @@ -15,6 +15,7 @@ use std::path::{Path, PathBuf}; use std::str::FromStr; use std::sync::{Arc, Mutex}; use std::{process, thread}; +use std::borrow::Borrow; use structopt::{clap::ArgGroup, StructOpt}; use grcov::*; @@ -107,8 +108,9 @@ struct Opt { "html", "cobertura", ], + multiple=true )] - output_type: OutputType, + output_type: Vec, /// Specifies the output path. #[structopt(short, long, value_name = "PATH", alias = "output-file")] output_path: Option, @@ -390,68 +392,74 @@ fn main() { } } - let result_map_mutex = Arc::try_unwrap(result_map).unwrap(); - let result_map = result_map_mutex.into_inner().unwrap(); - let path_mapping_mutex = Arc::try_unwrap(path_mapping).unwrap(); - let path_mapping = path_mapping_mutex.into_inner().unwrap(); - - let iterator = rewrite_paths( - result_map, - path_mapping, - source_root.as_deref(), - prefix_dir.as_deref(), - opt.ignore_not_existing, - &opt.ignore_dir, - &opt.keep_dir, - filter_option, - file_filter, - ); - - match opt.output_type { - OutputType::Ade => output_activedata_etl(iterator, opt.output_path.as_deref(), demangle), - OutputType::Lcov => output_lcov(iterator, opt.output_path.as_deref(), demangle), - OutputType::Coveralls => output_coveralls( - iterator, - opt.token.as_deref(), - opt.service_name.as_deref(), - &opt.service_number.unwrap_or_default(), - opt.service_job_id.as_deref(), - &opt.service_pull_request.unwrap_or_default(), - &opt.commit_sha.unwrap_or_default(), - false, - opt.output_path.as_deref(), - &opt.vcs_branch, - opt.parallel, - demangle, - ), - OutputType::CoverallsPlus => output_coveralls( - iterator, - opt.token.as_deref(), - opt.service_name.as_deref(), - &opt.service_number.unwrap_or_default(), - opt.service_job_id.as_deref(), - &opt.service_pull_request.unwrap_or_default(), - &opt.commit_sha.unwrap_or_default(), - true, - opt.output_path.as_deref(), - &opt.vcs_branch, - opt.parallel, - demangle, - ), - OutputType::Files => output_files(iterator, opt.output_path.as_deref()), - OutputType::Covdir => output_covdir(iterator, opt.output_path.as_deref()), - OutputType::Html => output_html( - iterator, - opt.output_path.as_deref(), - num_threads, - opt.branch, - ), - OutputType::Cobertura => output_cobertura( - source_root.as_deref(), - iterator, - opt.output_path.as_deref(), - demangle, - ), - }; + opt.output_type + .iter() + .for_each(|f| { + let result_map = Arc::clone(&result_map); + let path_mapping = Arc::clone(&path_mapping); + let result_map_mutex = Arc::try_unwrap(result_map).unwrap(); + let result_map = result_map_mutex.into_inner().unwrap(); + + let path_mapping_mutex = Arc::try_unwrap(path_mapping).unwrap(); + let path_mapping = path_mapping_mutex.into_inner().unwrap(); + let path = rewrite_paths( + result_map, + path_mapping, + source_root.as_deref(), + prefix_dir.as_deref(), + opt.ignore_not_existing, + &opt.ignore_dir, + &opt.keep_dir, + filter_option, + file_filter.clone(), + ); + let iterator = Box::new(path.into_iter()); + match f { + OutputType::Ade => output_activedata_etl(iterator, opt.output_path.as_deref(), demangle), + OutputType::Lcov => output_lcov(iterator, opt.output_path.as_deref(), demangle), + OutputType::Coveralls => output_coveralls( + iterator, + opt.token.as_deref(), + opt.service_name.as_deref(), + opt.service_number.unwrap_or_default(), + opt.service_job_id.as_deref(), + opt.service_pull_request.unwrap_or_default(), + opt.commit_sha.unwrap_or_default(), + false, + opt.output_path.as_deref(), + &opt.vcs_branch, + opt.parallel, + demangle, + ), + OutputType::CoverallsPlus => output_coveralls( + iterator, + opt.token.as_deref(), + opt.service_name.as_deref(), + &opt.service_number.unwrap_or_default(), + opt.service_job_id.as_deref(), + &opt.service_pull_request.unwrap_or_default(), + &opt.commit_sha.unwrap_or_default(), + true, + opt.output_path.as_deref(), + &opt.vcs_branch, + opt.parallel, + demangle, + ), + OutputType::Files => output_files(iterator, opt.output_path.as_deref()), + OutputType::Covdir => output_covdir(iterator, opt.output_path.as_deref()), + OutputType::Html => output_html( + iterator, + opt.output_path.as_deref(), + num_threads, + opt.branch, + ), + OutputType::Cobertura => output_cobertura( + source_root.as_deref(), + iterator, + opt.output_path.as_deref(), + demangle, + ), + }; + }); } diff --git a/src/path_rewriting.rs b/src/path_rewriting.rs index d942acfc3..69450d6ba 100644 --- a/src/path_rewriting.rs +++ b/src/path_rewriting.rs @@ -235,7 +235,7 @@ pub fn rewrite_paths( to_keep_dirs: &[impl AsRef], filter_option: Option, file_filter: crate::FileFilter, -) -> CovResultIter { +) -> Vec<(PathBuf, PathBuf, CovResult)> { let to_ignore_globset = to_globset(to_ignore_dirs); let to_keep_globset = to_globset(to_keep_dirs); @@ -341,11 +341,8 @@ pub fn rewrite_paths( Some((abs_path, rel_path, result)) }); - Box::new( - results - .collect::>() - .into_iter(), - ) + results.collect::>() + } #[cfg(test)] From f0a1917d8c729d44a2134038fba363312ef09b5a Mon Sep 17 00:00:00 2001 From: Gabriel Grubba <70247653+Grubba27@users.noreply.github.com> Date: Wed, 2 Feb 2022 18:26:36 -0300 Subject: [PATCH 2/5] started adrressing comments --- src/main.rs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/main.rs b/src/main.rs index 0533d859c..dc5c5a66d 100644 --- a/src/main.rs +++ b/src/main.rs @@ -392,6 +392,9 @@ fn main() { } } + let service_number = &opt.service_number.unwrap_or_default(); + let service_pull_request = &opt.service_pull_request.unwrap_or_default(); + let commit_sha = &opt.commit_sha.unwrap_or_default(); opt.output_type .iter() @@ -422,10 +425,10 @@ fn main() { iterator, opt.token.as_deref(), opt.service_name.as_deref(), - opt.service_number.unwrap_or_default(), + service_number, opt.service_job_id.as_deref(), - opt.service_pull_request.unwrap_or_default(), - opt.commit_sha.unwrap_or_default(), + service_pull_request, + commit_sha, false, opt.output_path.as_deref(), &opt.vcs_branch, @@ -436,10 +439,10 @@ fn main() { iterator, opt.token.as_deref(), opt.service_name.as_deref(), - &opt.service_number.unwrap_or_default(), + service_number, opt.service_job_id.as_deref(), - &opt.service_pull_request.unwrap_or_default(), - &opt.commit_sha.unwrap_or_default(), + service_pull_request, + commit_sha, true, opt.output_path.as_deref(), &opt.vcs_branch, From bc0d1bdbcb07a2a5c2d3de9b079ad0448ed9c67a Mon Sep 17 00:00:00 2001 From: Gabriel Grubba <70247653+Grubba27@users.noreply.github.com> Date: Thu, 3 Feb 2022 18:31:01 -0300 Subject: [PATCH 3/5] addressed comments --- src/main.rs | 79 ++++++++++++++++++++++++++++++++--------------------- 1 file changed, 48 insertions(+), 31 deletions(-) diff --git a/src/main.rs b/src/main.rs index dc5c5a66d..09d65f491 100644 --- a/src/main.rs +++ b/src/main.rs @@ -392,10 +392,27 @@ fn main() { } } - let service_number = &opt.service_number.unwrap_or_default(); - let service_pull_request = &opt.service_pull_request.unwrap_or_default(); - let commit_sha = &opt.commit_sha.unwrap_or_default(); + let Opt { + service_number, + service_pull_request, + commit_sha, + output_path, + branch, + parallel, + ignore_not_existing, + .. + } = opt; + let service_number = service_number.unwrap_or_default(); + let service_pull_request = service_pull_request.unwrap_or_default(); + let commit_sha = commit_sha.unwrap_or_default(); + let output_path = output_path.as_deref(); + let ignore_dir = &opt.ignore_dir; + let keep_dir = &opt.keep_dir; + let token = opt.token.as_deref(); + let service_name = opt.service_name.as_deref(); + let service_job = opt.service_job_id.as_deref(); + let vcs_branch = &opt.vcs_branch; opt.output_type .iter() .for_each(|f| { @@ -411,56 +428,56 @@ fn main() { path_mapping, source_root.as_deref(), prefix_dir.as_deref(), - opt.ignore_not_existing, - &opt.ignore_dir, - &opt.keep_dir, + ignore_not_existing, + ignore_dir, + keep_dir, filter_option, file_filter.clone(), ); let iterator = Box::new(path.into_iter()); match f { - OutputType::Ade => output_activedata_etl(iterator, opt.output_path.as_deref(), demangle), - OutputType::Lcov => output_lcov(iterator, opt.output_path.as_deref(), demangle), + OutputType::Ade => output_activedata_etl(iterator, output_path, demangle), + OutputType::Lcov => output_lcov(iterator, output_path, demangle), OutputType::Coveralls => output_coveralls( iterator, - opt.token.as_deref(), - opt.service_name.as_deref(), - service_number, - opt.service_job_id.as_deref(), - service_pull_request, - commit_sha, + token, + service_name, + &service_number, + service_job, + &service_pull_request, + &commit_sha, false, - opt.output_path.as_deref(), - &opt.vcs_branch, - opt.parallel, + output_path, + vcs_branch, + parallel, demangle, ), OutputType::CoverallsPlus => output_coveralls( iterator, - opt.token.as_deref(), - opt.service_name.as_deref(), - service_number, - opt.service_job_id.as_deref(), - service_pull_request, - commit_sha, + token, + service_name, + &service_number, + service_job, + &service_pull_request, + &commit_sha, true, - opt.output_path.as_deref(), - &opt.vcs_branch, - opt.parallel, + output_path, + vcs_branch, + parallel, demangle, ), - OutputType::Files => output_files(iterator, opt.output_path.as_deref()), - OutputType::Covdir => output_covdir(iterator, opt.output_path.as_deref()), + OutputType::Files => output_files(iterator, output_path), + OutputType::Covdir => output_covdir(iterator, output_path), OutputType::Html => output_html( iterator, - opt.output_path.as_deref(), + output_path, num_threads, - opt.branch, + branch, ), OutputType::Cobertura => output_cobertura( source_root.as_deref(), iterator, - opt.output_path.as_deref(), + output_path, demangle, ), }; From 1e1405fec26e5de47b0a7eae4a876775525331e1 Mon Sep 17 00:00:00 2001 From: Gabriel Grubba <70247653+Grubba27@users.noreply.github.com> Date: Thu, 3 Feb 2022 18:38:15 -0300 Subject: [PATCH 4/5] rewriting tests --- src/path_rewriting.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/path_rewriting.rs b/src/path_rewriting.rs index 2314108a5..71bfdbc9e 100644 --- a/src/path_rewriting.rs +++ b/src/path_rewriting.rs @@ -420,7 +420,7 @@ mod tests { fn test_rewrite_paths_basic() { let mut result_map: CovResultMap = FxHashMap::default(); result_map.insert("main.cpp".to_string(), empty_result!()); - let results = rewrite_paths( + let results = Box::new(rewrite_paths( result_map, None, None, @@ -430,7 +430,7 @@ mod tests { &[""; 0], None, Default::default(), - ); + ).into_iter()); let mut count = 0; for (abs_path, rel_path, result) in results { count += 1; @@ -919,7 +919,7 @@ mod tests { #[should_panic] fn test_rewrite_paths_rewrite_path_using_relative_source_directory() { let result_map: CovResultMap = FxHashMap::default(); - rewrite_paths( + Box::new(rewrite_paths( result_map, None, Some(Path::new("tests")), @@ -929,7 +929,7 @@ mod tests { &[""; 0], None, Default::default(), - ) + ).into_iter()) .any(|_| false); } @@ -1049,7 +1049,7 @@ mod tests { let mut result_map: CovResultMap = FxHashMap::default(); result_map.insert("java/main.java".to_string(), empty_result!()); result_map.insert("main.rs".to_string(), empty_result!()); - let results = rewrite_paths( + let results = Box::new(rewrite_paths( result_map, None, Some(&canonicalize_path(".").unwrap()), @@ -1059,7 +1059,7 @@ mod tests { &[""; 0], None, Default::default(), - ); + ).into_iter()); let mut results: Vec<(PathBuf, PathBuf, CovResult)> = results.collect(); assert!(results.len() == 1); From 78eda01e1b36ad047f27cf0e3cc89364f3bcc5c5 Mon Sep 17 00:00:00 2001 From: Gabriel Grubba <70247653+Grubba27@users.noreply.github.com> Date: Thu, 3 Feb 2022 18:46:08 -0300 Subject: [PATCH 5/5] ran linter --- src/main.rs | 129 +++++++++++++++++++----------------------- src/path_rewriting.rs | 76 ++++++++++++++----------- 2 files changed, 101 insertions(+), 104 deletions(-) diff --git a/src/main.rs b/src/main.rs index 09d65f491..6690a825c 100644 --- a/src/main.rs +++ b/src/main.rs @@ -8,6 +8,7 @@ use regex::Regex; use rustc_hash::FxHashMap; use serde_json::Value; use simplelog::{ColorChoice, Config, LevelFilter, TermLogger, TerminalMode, WriteLogger}; +use std::borrow::Borrow; use std::fs::{self, File}; use std::ops::Deref; use std::panic; @@ -15,7 +16,6 @@ use std::path::{Path, PathBuf}; use std::str::FromStr; use std::sync::{Arc, Mutex}; use std::{process, thread}; -use std::borrow::Borrow; use structopt::{clap::ArgGroup, StructOpt}; use grcov::*; @@ -392,7 +392,6 @@ fn main() { } } - let Opt { service_number, service_pull_request, @@ -413,73 +412,63 @@ fn main() { let service_name = opt.service_name.as_deref(); let service_job = opt.service_job_id.as_deref(); let vcs_branch = &opt.vcs_branch; - opt.output_type - .iter() - .for_each(|f| { - let result_map = Arc::clone(&result_map); - let path_mapping = Arc::clone(&path_mapping); - let result_map_mutex = Arc::try_unwrap(result_map).unwrap(); - let result_map = result_map_mutex.into_inner().unwrap(); + opt.output_type.iter().for_each(|f| { + let result_map = Arc::clone(&result_map); + let path_mapping = Arc::clone(&path_mapping); + let result_map_mutex = Arc::try_unwrap(result_map).unwrap(); + let result_map = result_map_mutex.into_inner().unwrap(); - let path_mapping_mutex = Arc::try_unwrap(path_mapping).unwrap(); - let path_mapping = path_mapping_mutex.into_inner().unwrap(); - let path = rewrite_paths( - result_map, - path_mapping, - source_root.as_deref(), - prefix_dir.as_deref(), - ignore_not_existing, - ignore_dir, - keep_dir, - filter_option, - file_filter.clone(), - ); - let iterator = Box::new(path.into_iter()); - match f { - OutputType::Ade => output_activedata_etl(iterator, output_path, demangle), - OutputType::Lcov => output_lcov(iterator, output_path, demangle), - OutputType::Coveralls => output_coveralls( - iterator, - token, - service_name, - &service_number, - service_job, - &service_pull_request, - &commit_sha, - false, - output_path, - vcs_branch, - parallel, - demangle, - ), - OutputType::CoverallsPlus => output_coveralls( - iterator, - token, - service_name, - &service_number, - service_job, - &service_pull_request, - &commit_sha, - true, - output_path, - vcs_branch, - parallel, - demangle, - ), - OutputType::Files => output_files(iterator, output_path), - OutputType::Covdir => output_covdir(iterator, output_path), - OutputType::Html => output_html( - iterator, - output_path, - num_threads, - branch, - ), - OutputType::Cobertura => output_cobertura( - source_root.as_deref(), - iterator, - output_path, - demangle, - ), - }; - }); + let path_mapping_mutex = Arc::try_unwrap(path_mapping).unwrap(); + let path_mapping = path_mapping_mutex.into_inner().unwrap(); + let path = rewrite_paths( + result_map, + path_mapping, + source_root.as_deref(), + prefix_dir.as_deref(), + ignore_not_existing, + ignore_dir, + keep_dir, + filter_option, + file_filter.clone(), + ); + let iterator = Box::new(path.into_iter()); + match f { + OutputType::Ade => output_activedata_etl(iterator, output_path, demangle), + OutputType::Lcov => output_lcov(iterator, output_path, demangle), + OutputType::Coveralls => output_coveralls( + iterator, + token, + service_name, + &service_number, + service_job, + &service_pull_request, + &commit_sha, + false, + output_path, + vcs_branch, + parallel, + demangle, + ), + OutputType::CoverallsPlus => output_coveralls( + iterator, + token, + service_name, + &service_number, + service_job, + &service_pull_request, + &commit_sha, + true, + output_path, + vcs_branch, + parallel, + demangle, + ), + OutputType::Files => output_files(iterator, output_path), + OutputType::Covdir => output_covdir(iterator, output_path), + OutputType::Html => output_html(iterator, output_path, num_threads, branch), + OutputType::Cobertura => { + output_cobertura(source_root.as_deref(), iterator, output_path, demangle) + } + }; + }); } diff --git a/src/path_rewriting.rs b/src/path_rewriting.rs index 71bfdbc9e..6ce5b45a4 100644 --- a/src/path_rewriting.rs +++ b/src/path_rewriting.rs @@ -342,7 +342,6 @@ pub fn rewrite_paths( }); results.collect::>() - } #[cfg(test)] @@ -420,17 +419,20 @@ mod tests { fn test_rewrite_paths_basic() { let mut result_map: CovResultMap = FxHashMap::default(); result_map.insert("main.cpp".to_string(), empty_result!()); - let results = Box::new(rewrite_paths( - result_map, - None, - None, - None, - false, - &[""; 0], - &[""; 0], - None, - Default::default(), - ).into_iter()); + let results = Box::new( + rewrite_paths( + result_map, + None, + None, + None, + false, + &[""; 0], + &[""; 0], + None, + Default::default(), + ) + .into_iter(), + ); let mut count = 0; for (abs_path, rel_path, result) in results { count += 1; @@ -919,17 +921,20 @@ mod tests { #[should_panic] fn test_rewrite_paths_rewrite_path_using_relative_source_directory() { let result_map: CovResultMap = FxHashMap::default(); - Box::new(rewrite_paths( - result_map, - None, - Some(Path::new("tests")), - None, - true, - &[""; 0], - &[""; 0], - None, - Default::default(), - ).into_iter()) + Box::new( + rewrite_paths( + result_map, + None, + Some(Path::new("tests")), + None, + true, + &[""; 0], + &[""; 0], + None, + Default::default(), + ) + .into_iter(), + ) .any(|_| false); } @@ -1049,17 +1054,20 @@ mod tests { let mut result_map: CovResultMap = FxHashMap::default(); result_map.insert("java/main.java".to_string(), empty_result!()); result_map.insert("main.rs".to_string(), empty_result!()); - let results = Box::new(rewrite_paths( - result_map, - None, - Some(&canonicalize_path(".").unwrap()), - None, - true, - &[""; 0], - &[""; 0], - None, - Default::default(), - ).into_iter()); + let results = Box::new( + rewrite_paths( + result_map, + None, + Some(&canonicalize_path(".").unwrap()), + None, + true, + &[""; 0], + &[""; 0], + None, + Default::default(), + ) + .into_iter(), + ); let mut results: Vec<(PathBuf, PathBuf, CovResult)> = results.collect(); assert!(results.len() == 1);