From 9701c60d52e6589b861e707f07891db67df8ab48 Mon Sep 17 00:00:00 2001 From: Niki Guldbrand Date: Tue, 24 Oct 2023 01:51:48 +0200 Subject: [PATCH] Refactor and integrate with --fail-under Now the feature is integrated with --fail-under, so that it works like this: * If --fail-under is used by it self, it should behave like before * If --fail-decreasing is used by it self, it will return an error if the coverage percentage is decreasing. * If --fail-under and --fail-decreasing is used together is works as follows, if the coverage is below the threshold, it error out if the coverage is decreasing, if the coverage is above the threshold, the coverage can increase and decrease freely. Summary: * updated README and CHANGELOG * Updated the command line help text * Reverted some canges to src/report/mod.rs and reimplemented them in src/lib.rs instead, made get_previous_result public * In src/lib.rs: * implemented get_delta, to get the coverage percentage from the previous run, with the help of get_previous_result * Implemented check_coverage, which now does the heavy lifting in this change * Refactored report_coverage_with_check to use check_coverage --- CHANGELOG.md | 3 +++ README.md | 2 +- src/args.rs | 4 ++- src/lib.rs | 63 +++++++++++++++++++++++++++++++++++++++-------- src/report/mod.rs | 25 +++---------------- 5 files changed, 64 insertions(+), 33 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9bca9b8679..559694db9b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,9 @@ file. - Don't disable ASLR if it's already disabled - Add the `--fail-decreasing` commandline option, which will make tarpaulin return an error if the test coverage has decreased. + If used in combination with `--fail-under`, and the coverage percentage is increasing, + pass the run, if we are above the threshold specified, allow the coverage percentage to + increase and decrease within that window. ## [0.27.1] 2023-10-02 ### Changed diff --git a/README.md b/README.md index f6634b82c9..495ee7b78f 100644 --- a/README.md +++ b/README.md @@ -69,7 +69,7 @@ Options: --skip-clean The opposite of --force-clean --force-clean Adds a clean stage to work around cargo bugs that may affect coverage results --fail-under Sets a percentage threshold for failure ranging from 0-100, if coverage is below exit with a non-zero code - --fail-decreasing Fail if the covarage percentage is decreasing + --fail-decreasing Fail if the covarage percentage is decreasing, if used in combination with `--fail-under`, then the tests passes under the threshold, as long as the covarage isn't decreasing, above the threshold it may increase or decrease -b, --branch Branch coverage: NOT IMPLEMENTED -f, --forward Forwards unexpected signals to test. This is now the default behaviour --coveralls Coveralls key, either the repo token, or if you're using travis use $TRAVIS_JOB_ID and specify travis-{ci|pro} in --ciserver diff --git a/src/args.rs b/src/args.rs index b4bf537470..9ba32ea07f 100644 --- a/src/args.rs +++ b/src/args.rs @@ -96,7 +96,9 @@ pub struct ConfigArgs { /// Sets a percentage threshold for failure ranging from 0-100, if coverage is below exit with a non-zero code #[arg(long, value_name = "PERCENTAGE")] pub fail_under: Option, - /// Fail if the covarage percentage is decreasing + /// Fail if the covarage percentage is decreasing, if used in combination with `--fail-under`, + /// then the tests passes under the threshold, as long as the covarage isn't decreasing, above + /// the threshold it may increase or decrease #[arg(long)] pub fail_decreasing: bool, /// Branch coverage: NOT IMPLEMENTED diff --git a/src/lib.rs b/src/lib.rs index a9a4f1f640..e75e3b053a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -4,7 +4,7 @@ use crate::errors::*; use crate::event_log::*; use crate::path_utils::*; use crate::process_handling::*; -use crate::report::report_coverage; +use crate::report::{get_previous_result, report_coverage}; use crate::source_analysis::{LineAnalysis, SourceAnalysis}; use crate::test_loader::*; use crate::traces::*; @@ -105,8 +105,9 @@ pub fn trace(configs: &[Config]) -> Result<(TraceMap, i32), RunError> { ret |= r; } if configs.len() > 1 { + let delta = get_delta(config, &t); // Otherwise threshold is a global one and we'll let the caller handle it - bad_threshold = check_fail_threshold(&t, config); + bad_threshold = check_coverage(delta, config, &t); } tracemap.merge(&t); } @@ -165,23 +166,33 @@ fn config_name(config: &Config) -> String { } } +// Returns the changed test coverage percentage, current vs. last run +fn get_delta(config: &Config, trace: &TraceMap) -> f64 { + let last = get_previous_result(config).unwrap_or(TraceMap::new()); + + let last = last.coverage_percentage() * 100.0f64; + let current = trace.coverage_percentage() * 100.0f64; + + let delta = current - last; + delta +} + fn check_fail_threshold(traces: &TraceMap, config: &Config) -> Result<(), RunError> { let percent = traces.coverage_percentage() * 100.0; match config.fail_under.as_ref() { Some(limit) if percent < *limit => { let error = RunError::BelowThreshold(percent, *limit); - error!("{}", error); Err(error) } _ => Ok(()), } } +// Check if the coverage percentage is decreasing fn check_fail_decreasing(delta: f64, config: &Config) -> Result<(), RunError> { if config.fail_decreasing { if delta < 0.0f64 { let error = RunError::CoverageDecreasing(delta); - error!("{}", error); return Err(error); } } @@ -240,18 +251,50 @@ pub fn report_tracemap(configs: &[Config], tracemap: TraceMap) -> Result<(), Run Ok(()) } -fn report_coverage_with_check(c: &Config, tracemap: &TraceMap) -> Result<(), RunError> { - let delta = report_coverage(c, tracemap)?.unwrap_or_else(|| 0.0f64); +fn check_coverage(delta: f64, config: &Config, trace: &TraceMap) -> Result<(), RunError> { + let threshold = check_fail_threshold(trace, config); + let decreasing = check_fail_decreasing(delta, config); - if let Some(err) = check_fail_threshold(tracemap, c).err() { - return Err(err); + // If both (--fail-under & --fail-decreasing) are used together, and we are below the threshold + // only allow the coverage to increase, or stay at the same level, else return an error. + if threshold.is_err() { + let threshold = threshold.unwrap_err(); + + if config.fail_decreasing { + if decreasing.is_ok() { + return Ok(()); + } + } + error!("{}", threshold); + return Err(threshold); } - if let Some(err) = check_fail_decreasing(delta, c).err() { - return Err(err); + + // If both (--fail-under & --fail-decreasing) are used together, allow the coverage to both + // increase and decrease, as long as we are above the threshold. + if threshold.is_ok() && config.fail_decreasing { + return Ok(()); } + + // If we are acting alone (no --fail-under) error out when then covage decreaces. + if decreasing.is_err() && config.fail_under.is_none() { + let decreasing = decreasing.unwrap_err(); + error!("{}", decreasing); + return Err(decreasing); + } + Ok(()) } +fn report_coverage_with_check(config: &Config, tracemap: &TraceMap) -> Result<(), RunError> { + // Needed for check_coverage, because after report_coverage below, the new data has been + // written, and the old data is lost. + let delta = get_delta(config, tracemap); + + report_coverage(config, tracemap)?; + + check_coverage(delta, config, tracemap) +} + /// Launches tarpaulin with the given configuration. pub fn launch_tarpaulin( config: &Config, diff --git a/src/report/mod.rs b/src/report/mod.rs index f603813850..f3ff440ff9 100644 --- a/src/report/mod.rs +++ b/src/report/mod.rs @@ -33,10 +33,8 @@ fn coverage_report_name(config: &Config) -> String { /// Reports the test coverage using the users preferred method. See config.rs /// or help text for details. -pub fn report_coverage(config: &Config, result: &TraceMap) -> Result, RunError> { +pub fn report_coverage(config: &Config, result: &TraceMap) -> Result<(), RunError> { if !result.is_empty() { - let delta = report_delta(config, result); - generate_requested_reports(config, result)?; let mut report_dir = config.target_dir(); report_dir.push("tarpaulin"); @@ -48,31 +46,16 @@ pub fn report_coverage(config: &Config, result: &TraceMap) -> Result .map_err(|_| RunError::CovReport("Failed to create run report".to_string()))?; serde_json::to_writer(&file, &result) .map_err(|_| RunError::CovReport("Failed to save run report".to_string()))?; - Ok(Some(delta)) + Ok(()) } else if !config.no_run { Err(RunError::CovReport( "No coverage results collected.".to_string(), )) } else { - Ok(None) + Ok(()) } } -/// Returns the changed test coverage perentage, used in combination with the --fail-decreasing -/// command line argument -fn report_delta(config: &Config, result: &TraceMap) -> f64 { - let last = match get_previous_result(config) { - Some(l) => l, - None => TraceMap::new(), - }; - - let last = last.coverage_percentage() * 100.0f64; - let current = result.coverage_percentage() * 100.0f64; - - let delta = current - last; - delta -} - fn generate_requested_reports(config: &Config, result: &TraceMap) -> Result<(), RunError> { if config.is_coveralls() { coveralls::export(result, config)?; @@ -146,7 +129,7 @@ fn print_missing_lines(config: &Config, result: &TraceMap) { } } -fn get_previous_result(config: &Config) -> Option { +pub fn get_previous_result(config: &Config) -> Option { // Check for previous report let mut report_dir = config.target_dir(); report_dir.push("tarpaulin");