From 84cb5983cc23e5ac318dcd950987b416231b2f5e Mon Sep 17 00:00:00 2001 From: ddoktorski <45050160+ddoktorski@users.noreply.github.com> Date: Wed, 4 Sep 2024 11:46:16 +0200 Subject: [PATCH] Add `--coverage` flag (#2369) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes https://github.com/software-mansion/cairo-coverage/issues/5 ## Introduced changes - Added a flag `--coverage` to `test` command that creates coverage report of all non-fuzz tests that passed, using the `cairo-coverage` binary ## Checklist - [X] Linked relevant issue - [X] Updated relevant documentation - [X] Added relevant tests - [X] Performed self-review of the code - [ ] Added changes to `CHANGELOG.md` --------- Co-authored-by: Karol Sewiło <95349104+ksew1@users.noreply.github.com> Co-authored-by: Arcticae Co-authored-by: Karol Sewilo Co-authored-by: Tomasz Rejowski <34059895+Arcticae@users.noreply.github.com> --- .github/workflows/ci.yml | 4 ++ .gitignore | 3 +- CHANGELOG.md | 1 + crates/forge-runner/src/coverage_api.rs | 40 +++++++++++++++++++ crates/forge-runner/src/forge_config.rs | 6 ++- crates/forge-runner/src/lib.rs | 23 ++++++++++- crates/forge/src/combine_configs.rs | 11 +++++ crates/forge/src/lib.rs | 6 ++- crates/forge/src/run_tests/package.rs | 1 + crates/forge/src/run_tests/test_target.rs | 18 ++++++++- crates/forge/src/scarb.rs | 6 ++- crates/forge/src/scarb/config.rs | 10 ++++- .../tests/data/coverage_project/Scarb.toml | 19 +++++++++ .../tests/data/coverage_project/src/lib.cairo | 9 +++++ .../data/coverage_project/tests/lib.cairo | 8 ++++ crates/forge/tests/e2e/coverage.rs | 15 +++++++ crates/forge/tests/e2e/mod.rs | 1 + docs/src/SUMMARY.md | 1 + docs/src/appendix/snforge/test.md | 5 +++ docs/src/testing/coverage.md | 32 +++++++++++++++ 20 files changed, 207 insertions(+), 12 deletions(-) create mode 100644 crates/forge-runner/src/coverage_api.rs create mode 100644 crates/forge/tests/data/coverage_project/Scarb.toml create mode 100644 crates/forge/tests/data/coverage_project/src/lib.cairo create mode 100644 crates/forge/tests/data/coverage_project/tests/lib.cairo create mode 100644 crates/forge/tests/e2e/coverage.rs create mode 100644 docs/src/testing/coverage.md diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index bec7cec2bd..7e569c61d7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -72,6 +72,10 @@ jobs: run: | curl -L https://raw.githubusercontent.com/software-mansion/cairo-profiler/main/scripts/install.sh | sh + - name: Install cairo-coverage + run: | + curl -L https://raw.githubusercontent.com/software-mansion/cairo-coverage/main/scripts/install.sh | sh + - uses: actions/checkout@v4 - uses: dtolnay/rust-toolchain@stable - uses: Swatinem/rust-cache@23bce251a8cd2ffc3c1075eaa2367cf899916d84 diff --git a/.gitignore b/.gitignore index 6c958611f0..8801014fc0 100644 --- a/.gitignore +++ b/.gitignore @@ -11,6 +11,7 @@ Scarb.lock */node_modules .spr.yml .snfoundry_cache/ -snfoundry_trace/ .snfoundry_versioned_programs/ +snfoundry_trace/ +coverage/ **/.forge_e2e_cache/ diff --git a/CHANGELOG.md b/CHANGELOG.md index 2654d58b05..7b0df363b6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 #### Added - Derived `Debug` and `Clone` on `trace.cairo` items +- `--coverage` flag to the `test` command. Saves trace data and then generates coverage report of test cases which pass and are not fuzz tests. You need [cairo-coverage](https://github.com/software-mansion/cairo-coverage) installed on your system. ## [0.29.0] - 2024-08-28 diff --git a/crates/forge-runner/src/coverage_api.rs b/crates/forge-runner/src/coverage_api.rs new file mode 100644 index 0000000000..4d87422ee3 --- /dev/null +++ b/crates/forge-runner/src/coverage_api.rs @@ -0,0 +1,40 @@ +use anyhow::{Context, Result}; +use shared::command::CommandExt; +use std::process::Stdio; +use std::{env, fs, path::PathBuf, process::Command}; + +pub const COVERAGE_DIR: &str = "coverage"; +pub const OUTPUT_FILE_NAME: &str = "coverage.lcov"; + +pub fn run_coverage(saved_trace_data_paths: &[PathBuf]) -> Result<()> { + let coverage = env::var("CAIRO_COVERAGE") + .map(PathBuf::from) + .ok() + .unwrap_or_else(|| PathBuf::from("cairo-coverage")); + + let dir_to_save_coverage = PathBuf::from(COVERAGE_DIR); + fs::create_dir_all(&dir_to_save_coverage).context("Failed to create a coverage dir")?; + let path_to_save_coverage = dir_to_save_coverage.join(OUTPUT_FILE_NAME); + + let trace_files: Vec<&str> = saved_trace_data_paths + .iter() + .map(|trace_data_path| { + trace_data_path + .to_str() + .expect("Failed to convert trace data path to string") + }) + .collect(); + + Command::new(coverage) + .arg("--output-path") + .arg(&path_to_save_coverage) + .args(trace_files) + .stdout(Stdio::inherit()) + .stderr(Stdio::inherit()) + .output_checked() + .with_context(|| { + "cairo-coverage failed to generate coverage - inspect the errors above for more info" + })?; + + Ok(()) +} diff --git a/crates/forge-runner/src/forge_config.rs b/crates/forge-runner/src/forge_config.rs index 7920e4087d..07ea04381f 100644 --- a/crates/forge-runner/src/forge_config.rs +++ b/crates/forge-runner/src/forge_config.rs @@ -33,19 +33,21 @@ pub struct OutputConfig { pub struct ExecutionDataToSave { pub trace: bool, pub profile: bool, + pub coverage: bool, } impl ExecutionDataToSave { #[must_use] - pub fn from_flags(save_trace_data: bool, build_profile: bool) -> Self { + pub fn from_flags(save_trace_data: bool, build_profile: bool, coverage: bool) -> Self { Self { trace: save_trace_data, profile: build_profile, + coverage, } } #[must_use] pub fn is_vm_trace_needed(&self) -> bool { - self.trace || self.profile + self.trace || self.profile || self.coverage } } diff --git a/crates/forge-runner/src/lib.rs b/crates/forge-runner/src/lib.rs index fbca93c621..ba2905853c 100644 --- a/crates/forge-runner/src/lib.rs +++ b/crates/forge-runner/src/lib.rs @@ -1,9 +1,10 @@ use crate::build_trace_data::test_sierra_program_path::VersionedProgramPath; +use crate::coverage_api::run_coverage; use crate::forge_config::{ExecutionDataToSave, ForgeConfig, TestRunnerConfig}; use crate::fuzzer::RandomFuzzer; use crate::running::{run_fuzz_test, run_test}; use crate::test_case_summary::TestCaseSummary; -use anyhow::Result; +use anyhow::{anyhow, Result}; use build_trace_data::save_trace_data; use cairo_lang_sierra::program::{ConcreteTypeLongId, Function, TypeDeclaration}; use camino::Utf8Path; @@ -14,7 +15,9 @@ use package_tests::with_config_resolved::{ TestCaseWithResolvedConfig, TestTargetWithResolvedConfig, }; use profiler_api::run_profiler; +use shared::print::print_as_warning; use std::collections::HashMap; +use std::path::PathBuf; use std::sync::Arc; use test_case_summary::{AnyTestCaseSummary, Fuzzing}; use tokio::sync::mpsc::{channel, Sender}; @@ -22,6 +25,7 @@ use tokio::task::JoinHandle; use universal_sierra_compiler_api::AssembledProgramWithDebugInfo; pub mod build_trace_data; +pub mod coverage_api; pub mod expected_result; pub mod forge_config; pub mod package_tests; @@ -57,7 +61,7 @@ pub trait TestCaseFilter { pub fn maybe_save_trace_and_profile( result: &AnyTestCaseSummary, execution_data_to_save: ExecutionDataToSave, -) -> Result<()> { +) -> Result> { if let AnyTestCaseSummary::Single(TestCaseSummary::Passed { name, trace_data, .. }) = result @@ -67,6 +71,21 @@ pub fn maybe_save_trace_and_profile( if execution_data_to_save.profile { run_profiler(name, &trace_path)?; } + return Ok(Some(trace_path)); + } + } + Ok(None) +} + +pub fn maybe_generate_coverage( + execution_data_to_save: ExecutionDataToSave, + saved_trace_data_paths: &[PathBuf], +) -> Result<()> { + if execution_data_to_save.coverage { + if saved_trace_data_paths.is_empty() { + print_as_warning(&anyhow!("No trace data to generate coverage from")); + } else { + run_coverage(saved_trace_data_paths)?; } } Ok(()) diff --git a/crates/forge/src/combine_configs.rs b/crates/forge/src/combine_configs.rs index a7fa45c963..ebd9269444 100644 --- a/crates/forge/src/combine_configs.rs +++ b/crates/forge/src/combine_configs.rs @@ -18,6 +18,7 @@ pub fn combine_configs( detailed_resources: bool, save_trace_data: bool, build_profile: bool, + coverage: bool, max_n_steps: Option, contracts_data: ContractsData, cache_dir: Utf8PathBuf, @@ -27,6 +28,7 @@ pub fn combine_configs( let execution_data_to_save = ExecutionDataToSave::from_flags( save_trace_data || forge_config_from_scarb.save_trace_data, build_profile || forge_config_from_scarb.build_profile, + coverage || forge_config_from_scarb.coverage, ); ForgeConfig { @@ -65,6 +67,7 @@ mod tests { false, false, false, + false, None, Default::default(), Default::default(), @@ -78,6 +81,7 @@ mod tests { false, false, false, + false, None, Default::default(), Default::default(), @@ -102,6 +106,7 @@ mod tests { false, false, false, + false, None, Default::default(), Default::default(), @@ -140,6 +145,7 @@ mod tests { detailed_resources: true, save_trace_data: true, build_profile: true, + coverage: true, max_n_steps: Some(1_000_000), }; @@ -150,6 +156,7 @@ mod tests { false, false, false, + false, None, Default::default(), Default::default(), @@ -174,6 +181,7 @@ mod tests { execution_data_to_save: ExecutionDataToSave { trace: true, profile: true, + coverage: true, }, versioned_programs_dir: Default::default(), }), @@ -191,6 +199,7 @@ mod tests { detailed_resources: false, save_trace_data: false, build_profile: false, + coverage: false, max_n_steps: Some(1234), }; let config = combine_configs( @@ -200,6 +209,7 @@ mod tests { true, true, true, + true, Some(1_000_000), Default::default(), Default::default(), @@ -225,6 +235,7 @@ mod tests { execution_data_to_save: ExecutionDataToSave { trace: true, profile: true, + coverage: true, }, versioned_programs_dir: Default::default(), }), diff --git a/crates/forge/src/lib.rs b/crates/forge/src/lib.rs index 04a8dc8642..7947af1e90 100644 --- a/crates/forge/src/lib.rs +++ b/crates/forge/src/lib.rs @@ -126,10 +126,14 @@ pub struct TestArgs { #[arg(long)] save_trace_data: bool, - /// Build profiles of all test which have passed and are not fuzz tests using the cairo-profiler + /// Build profiles of all tests which have passed and are not fuzz tests using the cairo-profiler #[arg(long)] build_profile: bool, + /// Generate a coverage report for the executed tests which have passed and are not fuzz tests using the cairo-coverage + #[arg(long)] + coverage: bool, + /// Number of maximum steps during a single test. For fuzz tests this value is applied to each subtest separately. #[arg(long)] max_n_steps: Option, diff --git a/crates/forge/src/run_tests/package.rs b/crates/forge/src/run_tests/package.rs index a0c2cc7781..aa1ee6f93c 100644 --- a/crates/forge/src/run_tests/package.rs +++ b/crates/forge/src/run_tests/package.rs @@ -65,6 +65,7 @@ impl RunForPackageArgs { args.detailed_resources, args.save_trace_data, args.build_profile, + args.coverage, args.max_n_steps, contracts_data, cache_dir.clone(), diff --git a/crates/forge/src/run_tests/test_target.rs b/crates/forge/src/run_tests/test_target.rs index 35963561b8..509a4ec1d5 100644 --- a/crates/forge/src/run_tests/test_target.rs +++ b/crates/forge/src/run_tests/test_target.rs @@ -2,7 +2,8 @@ use anyhow::Result; use cairo_lang_runner::RunnerError; use forge_runner::{ forge_config::ForgeConfig, - function_args, maybe_save_trace_and_profile, maybe_save_versioned_program, + function_args, maybe_generate_coverage, maybe_save_trace_and_profile, + maybe_save_versioned_program, package_tests::with_config_resolved::TestTargetWithResolvedConfig, printing::print_test_result, run_for_test_case, @@ -84,13 +85,21 @@ pub async fn run_for_test_target( } let mut results = vec![]; + let mut saved_trace_data_paths = vec![]; let mut interrupted = false; while let Some(task) = tasks.next().await { let result = task??; print_test_result(&result, forge_config.output_config.detailed_resources); - maybe_save_trace_and_profile(&result, forge_config.output_config.execution_data_to_save)?; + + let trace_path = maybe_save_trace_and_profile( + &result, + forge_config.output_config.execution_data_to_save, + )?; + if let Some(path) = trace_path { + saved_trace_data_paths.push(path); + } if result.is_failed() && forge_config.test_runner_config.exit_first { interrupted = true; @@ -100,6 +109,11 @@ pub async fn run_for_test_target( results.push(result); } + maybe_generate_coverage( + forge_config.output_config.execution_data_to_save, + &saved_trace_data_paths, + )?; + let summary = TestTargetSummary { test_case_summaries: results, }; diff --git a/crates/forge/src/scarb.rs b/crates/forge/src/scarb.rs index 3f41c24905..5018494b8e 100644 --- a/crates/forge/src/scarb.rs +++ b/crates/forge/src/scarb.rs @@ -226,7 +226,8 @@ mod tests { max_n_steps: None, detailed_resources: false, save_trace_data: false, - build_profile: false + build_profile: false, + coverage: false } ); } @@ -454,7 +455,8 @@ mod tests { max_n_steps: None, detailed_resources: false, save_trace_data: false, - build_profile: false + build_profile: false, + coverage: false } ); } diff --git a/crates/forge/src/scarb/config.rs b/crates/forge/src/scarb/config.rs index 222270d41d..0940c3d582 100644 --- a/crates/forge/src/scarb/config.rs +++ b/crates/forge/src/scarb/config.rs @@ -20,8 +20,10 @@ pub struct ForgeConfigFromScarb { pub detailed_resources: bool, /// Save execution traces of all test which have passed and are not fuzz tests pub save_trace_data: bool, - /// Builds profile of all test which have passed and are not fuzz tests + /// Build profiles of all tests which have passed and are not fuzz tests pub build_profile: bool, + /// Generate a coverage report for the executed tests which have passed and are not fuzz tests + pub coverage: bool, /// Fork configuration profiles pub fork: Vec, /// Limit of steps @@ -67,9 +69,12 @@ pub(crate) struct RawForgeConfig { /// Save execution traces of all test which have passed and are not fuzz tests pub save_trace_data: bool, #[serde(default)] - /// Builds profiles of all test which have passed and are not fuzz tests + /// Build profiles of all tests which have passed and are not fuzz tests pub build_profile: bool, #[serde(default)] + /// Generate a coverage report for the executed tests which have passed and are not fuzz tests + pub coverage: bool, + #[serde(default)] /// Fork configuration profiles pub fork: Vec, /// Limit of steps @@ -138,6 +143,7 @@ impl TryFrom for ForgeConfigFromScarb { detailed_resources: value.detailed_resources, save_trace_data: value.save_trace_data, build_profile: value.build_profile, + coverage: value.coverage, fork: fork_targets, max_n_steps: value.max_n_steps, }) diff --git a/crates/forge/tests/data/coverage_project/Scarb.toml b/crates/forge/tests/data/coverage_project/Scarb.toml new file mode 100644 index 0000000000..6aa7a7cb04 --- /dev/null +++ b/crates/forge/tests/data/coverage_project/Scarb.toml @@ -0,0 +1,19 @@ +[package] +name = "coverage_project" +version = "0.1.0" + +# See more keys and their definitions at https://docs.swmansion.com/scarb/docs/reference/manifest.html + +[dependencies] +starknet = "2.4.0" + +[dev-dependencies] +snforge_std = { path = "../../../../../snforge_std" } + +[[target.starknet-contract]] +sierra = true + +[cairo] +unstable-add-statements-functions-debug-info = true +unstable-add-statements-code-locations-debug-info = true +inlining-strategy= "avoid" diff --git a/crates/forge/tests/data/coverage_project/src/lib.cairo b/crates/forge/tests/data/coverage_project/src/lib.cairo new file mode 100644 index 0000000000..04569b8445 --- /dev/null +++ b/crates/forge/tests/data/coverage_project/src/lib.cairo @@ -0,0 +1,9 @@ +pub fn increase_by_two(arg: u8) -> u8 { + assert(2 == 2, ''); + increase_by_one(arg + 1) +} + +pub fn increase_by_one(arg: u8) -> u8 { + assert(1 == 1, ''); + arg + 1 +} diff --git a/crates/forge/tests/data/coverage_project/tests/lib.cairo b/crates/forge/tests/data/coverage_project/tests/lib.cairo new file mode 100644 index 0000000000..343d703d0f --- /dev/null +++ b/crates/forge/tests/data/coverage_project/tests/lib.cairo @@ -0,0 +1,8 @@ +use coverage_project::{increase_by_one, increase_by_two}; + + +#[test] +fn my_test() { + assert(increase_by_two(1) == 3, ''); // inlines + assert(increase_by_one(1) == 2, ''); // inlines +} diff --git a/crates/forge/tests/e2e/coverage.rs b/crates/forge/tests/e2e/coverage.rs new file mode 100644 index 0000000000..40ac64136c --- /dev/null +++ b/crates/forge/tests/e2e/coverage.rs @@ -0,0 +1,15 @@ +use super::common::runner::{setup_package, test_runner}; +use forge_runner::coverage_api::{COVERAGE_DIR, OUTPUT_FILE_NAME}; + +#[test] +#[ignore] // TODO(#2426) this will only work for scarb 2.8.0 or above +fn test_coverage_project() { + let temp = setup_package("coverage_project"); + + test_runner(&temp).arg("--coverage").assert().success(); + + assert!(temp.join(COVERAGE_DIR).join(OUTPUT_FILE_NAME).is_file()); + + // Check if it doesn't crash in case some data already exists + test_runner(&temp).arg("--coverage").assert().success(); +} diff --git a/crates/forge/tests/e2e/mod.rs b/crates/forge/tests/e2e/mod.rs index fe4af24947..8f4ffe7db1 100644 --- a/crates/forge/tests/e2e/mod.rs +++ b/crates/forge/tests/e2e/mod.rs @@ -5,6 +5,7 @@ mod build_trace_data; mod collection; mod color; mod components; +mod coverage; mod env; mod features; mod fork_warning; diff --git a/docs/src/SUMMARY.md b/docs/src/SUMMARY.md index 15139b1db1..1de0c4777f 100644 --- a/docs/src/SUMMARY.md +++ b/docs/src/SUMMARY.md @@ -24,6 +24,7 @@ * [Test Collection](testing/test-collection.md) * [Contract Collection](testing/contracts-collection.md) * [Gas and VM Resources Estimation](testing/gas-and-resource-estimation.md) +* [Coverage](testing/coverage.md) # `snforge` Advanced Features * [Fork Testing](snforge-advanced-features/fork-testing.md) diff --git a/docs/src/appendix/snforge/test.md b/docs/src/appendix/snforge/test.md index 71484c0ac2..851d380b00 100644 --- a/docs/src/appendix/snforge/test.md +++ b/docs/src/appendix/snforge/test.md @@ -65,6 +65,11 @@ Saves execution traces of test cases which pass and are not fuzz tests. You can Saves trace data and then builds profiles of test cases which pass and are not fuzz tests. You need [cairo-profiler](https://github.com/software-mansion/cairo-profiler) installed on your system. You can set a custom path to cairo-profiler with `CAIRO_PROFILER` env variable. Profile can be read with pprof, more information: [cairo-profiler](https://github.com/software-mansion/cairo-profiler), [pprof](https://github.com/google/pprof?tab=readme-ov-file#building-pprof) +## `--coverage` + +Saves trace data and then generates coverage report of test cases which pass and are not fuzz tests. +You need [cairo-coverage](https://github.com/software-mansion/cairo-coverage) installed on your system. You can set a custom path to cairo-coverage with `CAIRO_COVERAGE` env variable. + ## `--max-n-steps` `` Number of maximum steps during a single test. For fuzz tests this value is applied to each subtest separately. diff --git a/docs/src/testing/coverage.md b/docs/src/testing/coverage.md new file mode 100644 index 0000000000..02d4c6b925 --- /dev/null +++ b/docs/src/testing/coverage.md @@ -0,0 +1,32 @@ +# Coverage + +Coverage reporting allows developers to gain comprehensive insights into how their code is executed. +With `cairo-coverage`, you can generate a coverage report that can later be analyzed for detailed coverage statistics. + +## Integration with [cairo-coverage](https://github.com/software-mansion/cairo-coverage) + +`snforge` is able to produce a file with a trace for each passing test (excluding fuzz tests). +All you have to do is use the [`--save-trace-data`](../appendix/snforge/test.md#--save-trace-data) flag: + +```shell +$ snforge test --save-trace-data +``` + +The files with traces will be saved to `snfoundry_trace` directory. Each one of these files can then be used as an input +for the [cairo-coverage](https://github.com/software-mansion/cairo-coverage). + +If you want `snforge` to call `cairo-coverage` on generated files automatically, use [`--coverage`](../appendix/snforge/test.md#--coverage) flag: + +```shell +$ snforge test --coverage +``` + +> 📝 **Note** +> To generate trace data files, it is required to use [Scarb](https://github.com/software-mansion/scarb) version `2.8.0` or higher and include the following in your `Scarb.toml` file: +> ```toml +> [cairo] +> unstable-add-statements-code-locations-debug-info = true +> unstable-add-statements-functions-debug-info = true +> inlining-strategy = "avoid" +> ``` +> For more information about these sections, please refer to the [Scarb documentation](https://docs.swmansion.com/scarb/docs/reference/manifest.html#cairo). \ No newline at end of file