From 4ac946a45486ed4bbcc8d7dff165a5f1fd77790f Mon Sep 17 00:00:00 2001 From: Hugo Caillard <911307+hugocaillard@users.noreply.github.com> Date: Fri, 4 Oct 2024 19:22:28 +0200 Subject: [PATCH 1/7] refactor: better eval hooks handling in contract calls --- .../clarinet-cli/src/generate/contract.rs | 5 -- components/clarinet-deployments/src/lib.rs | 24 ++------ components/clarinet-sdk-wasm/src/core.rs | 24 ++++++-- components/clarity-lsp/src/common/state.rs | 1 - components/clarity-repl/src/analysis/mod.rs | 3 +- .../clarity-repl/src/repl/interpreter.rs | 28 ++++----- components/clarity-repl/src/repl/session.rs | 60 +++++-------------- 7 files changed, 56 insertions(+), 89 deletions(-) diff --git a/components/clarinet-cli/src/generate/contract.rs b/components/clarinet-cli/src/generate/contract.rs index 0d896afbd..055a4e88e 100644 --- a/components/clarinet-cli/src/generate/contract.rs +++ b/components/clarinet-cli/src/generate/contract.rs @@ -31,11 +31,6 @@ impl GetChangesForRmContract { f.append_path("tests")?; f.append_path(&name)?; if !f.exists() { - format!( - "{} tests/{} doesn't exist. Skipping removal", - red!("Warning"), - name - ); return Ok(()); } let change = FileDeletion { diff --git a/components/clarinet-deployments/src/lib.rs b/components/clarinet-deployments/src/lib.rs index ba4f8ecf6..6e5165dd3 100644 --- a/components/clarinet-deployments/src/lib.rs +++ b/components/clarinet-deployments/src/lib.rs @@ -56,7 +56,7 @@ pub fn setup_session_with_deployment( ) -> DeploymentGenerationArtifacts { let mut session = initiate_session_from_manifest(manifest); let UpdateSessionExecutionResult { contracts, .. } = - update_session_with_deployment_plan(&mut session, deployment, contracts_asts, false, None); + update_session_with_deployment_plan(&mut session, deployment, contracts_asts, None); let deps = BTreeMap::new(); let mut diags = HashMap::new(); @@ -122,7 +122,6 @@ pub fn update_session_with_deployment_plan( session: &mut Session, deployment: &DeploymentSpecification, contracts_asts: Option<&BTreeMap>, - code_coverage_enabled: bool, forced_min_epoch: Option, ) -> UpdateSessionExecutionResult { update_session_with_genesis_accounts(session, deployment); @@ -160,13 +159,7 @@ pub fn update_session_with_deployment_plan( tx.contract_name.clone(), ); let contract_ast = contracts_asts.as_ref().and_then(|m| m.get(&contract_id)); - let result = handle_emulated_contract_publish( - session, - tx, - contract_ast, - epoch, - code_coverage_enabled, - ); + let result = handle_emulated_contract_publish(session, tx, contract_ast, epoch); contracts.insert(contract_id, result); } TransactionSpecification::EmulatedContractCall(tx) => { @@ -198,7 +191,6 @@ fn handle_emulated_contract_publish( tx: &EmulatedContractPublishSpecification, contract_ast: Option<&ContractAST>, epoch: StacksEpochId, - code_coverage_enabled: bool, ) -> Result> { let default_tx_sender = session.get_tx_sender(); session.set_tx_sender(&tx.emulated_sender.to_string()); @@ -210,12 +202,8 @@ fn handle_emulated_contract_publish( clarity_version: tx.clarity_version, epoch, }; - let test_name = if code_coverage_enabled { - Some("__analysis__".to_string()) - } else { - None - }; - let result = session.deploy_contract(&contract, None, false, test_name, contract_ast); + + let result = session.deploy_contract(&contract, None, false, contract_ast); session.set_tx_sender(&default_tx_sender); result @@ -250,7 +238,7 @@ fn handle_emulated_contract_call( &tx.emulated_sender.to_string(), true, false, - false, + vec![], "deployment".to_string(), ); if let Err(errors) = &result { @@ -930,7 +918,7 @@ mod tests { location: FileLocation::from_path_string("/contracts/contract_1.clar").unwrap(), }; - handle_emulated_contract_publish(session, &emulated_publish_spec, None, epoch, false) + handle_emulated_contract_publish(session, &emulated_publish_spec, None, epoch) } #[test] diff --git a/components/clarinet-sdk-wasm/src/core.rs b/components/clarinet-sdk-wasm/src/core.rs index b042a84fd..f45377aae 100644 --- a/components/clarinet-sdk-wasm/src/core.rs +++ b/components/clarinet-sdk-wasm/src/core.rs @@ -9,7 +9,7 @@ use clarinet_deployments::{ }; use clarinet_files::chainhook_types::StacksNetwork; use clarinet_files::{FileAccessor, FileLocation, ProjectManifest, WASMFileSystemAccessor}; -use clarity_repl::analysis::coverage::CoverageReporter; +use clarity_repl::analysis::coverage::{CoverageReporter, TestCoverageReport}; use clarity_repl::clarity::analysis::contract_interface_builder::{ ContractInterface, ContractInterfaceFunction, ContractInterfaceFunctionAccess, }; @@ -19,7 +19,8 @@ use clarity_repl::clarity::vm::types::{ PrincipalData, QualifiedContractIdentifier, StandardPrincipalData, }; use clarity_repl::clarity::{ - Address, ClarityVersion, EvaluationResult, ExecutionResult, StacksEpochId, SymbolicExpression, + Address, ClarityVersion, EvalHook, EvaluationResult, ExecutionResult, StacksEpochId, + SymbolicExpression, }; use clarity_repl::repl::clarity_values::{uint8_to_string, uint8_to_value}; use clarity_repl::repl::session::BOOT_CONTRACTS_DATA; @@ -449,7 +450,6 @@ impl SDK { &mut session, &deployment, Some(&artifacts.asts), - false, Some(DEFAULT_EPOCH), ); @@ -719,6 +719,16 @@ impl SDK { track_coverage, } = self.options; + let mut hooks: Vec<&mut dyn EvalHook> = Vec::new(); + let mut coverage_hook = if track_coverage { + Some(TestCoverageReport::new(test_name.clone())) + } else { + None + }; + if let Some(ref mut hook) = coverage_hook { + hooks.push(hook) + } + let parsed_args = args .iter() .map(|a| SymbolicExpression::atom_value(uint8_to_value(a))) @@ -733,7 +743,7 @@ impl SDK { sender, allow_private, track_costs, - track_coverage, + hooks, test_name, ) .map_err(|diagnostics| { @@ -753,6 +763,10 @@ impl SDK { message })?; + if let Some(coverage_hook) = coverage_hook { + session.coverage_reports.push(coverage_hook) + } + Ok(execution_result_to_transaction_res(&execution)) } @@ -844,7 +858,7 @@ impl SDK { epoch: session.current_epoch, }; - match session.deploy_contract(&contract, None, false, Some(args.name.clone()), None) { + match session.deploy_contract(&contract, None, false, None) { Ok(res) => res, Err(diagnostics) => { let mut message = format!( diff --git a/components/clarity-lsp/src/common/state.rs b/components/clarity-lsp/src/common/state.rs index e2735740b..aa2bad669 100644 --- a/components/clarity-lsp/src/common/state.rs +++ b/components/clarity-lsp/src/common/state.rs @@ -654,7 +654,6 @@ pub async fn build_state( &mut session, &deployment, Some(&artifacts.asts), - false, Some(StacksEpochId::Epoch21), ); for (contract_id, mut result) in contracts.into_iter() { diff --git a/components/clarity-repl/src/analysis/mod.rs b/components/clarity-repl/src/analysis/mod.rs index 6627befb1..7757e86e7 100644 --- a/components/clarity-repl/src/analysis/mod.rs +++ b/components/clarity-repl/src/analysis/mod.rs @@ -154,9 +154,8 @@ where F: FnOnce(&mut AnalysisDatabase) -> std::result::Result, { conn.begin(); - let result = f(conn).map_err(|e| { + let result = f(conn).inspect_err(|_| { conn.roll_back().expect("Failed to roll back"); - e })?; conn.commit().expect("Failed to commit"); Ok(result) diff --git a/components/clarity-repl/src/repl/interpreter.rs b/components/clarity-repl/src/repl/interpreter.rs index 22df7047d..423218072 100644 --- a/components/clarity-repl/src/repl/interpreter.rs +++ b/components/clarity-repl/src/repl/interpreter.rs @@ -832,7 +832,7 @@ impl ClarityInterpreter { clarity_version: ClarityVersion, track_costs: bool, allow_private: bool, - eval_hooks: Option>, + mut eval_hooks: Vec<&mut dyn EvalHook>, ) -> Result { let mut conn = ClarityDatabase::new( &mut self.clarity_datastore, @@ -860,11 +860,11 @@ impl ClarityInterpreter { let mut global_context = GlobalContext::new(false, CHAIN_ID_TESTNET, conn, cost_tracker, epoch); - if let Some(mut in_hooks) = eval_hooks { - let mut hooks: Vec<&mut dyn EvalHook> = Vec::new(); - for hook in in_hooks.drain(..) { - hooks.push(hook); - } + let mut hooks: Vec<&mut dyn EvalHook> = Vec::new(); + for hook in eval_hooks.drain(..) { + hooks.push(hook); + } + if !hooks.is_empty() { global_context.eval_hooks = Some(hooks); } @@ -1807,7 +1807,7 @@ mod tests { ClarityVersion::Clarity2, false, false, - None, + vec![], ); assert_execution_result_value( result, @@ -1826,7 +1826,7 @@ mod tests { ClarityVersion::Clarity2, false, false, - None, + vec![], ); assert_execution_result_value( result, @@ -1879,7 +1879,7 @@ mod tests { ClarityVersion::Clarity2, false, false, - None, + vec![], ); assert_execution_result_value( result, @@ -1941,7 +1941,7 @@ mod tests { ClarityVersion::Clarity3, false, false, - None, + vec![], ); assert_execution_result_value( result, @@ -1964,7 +1964,7 @@ mod tests { ClarityVersion::Clarity3, false, false, - None, + vec![], ); assert_execution_result_value( result, @@ -2153,7 +2153,7 @@ mod tests { ClarityVersion::Clarity2, false, allow_private, - None, + vec![], ); assert!(result.is_ok()); @@ -2184,7 +2184,7 @@ mod tests { ClarityVersion::Clarity2, false, allow_private, - None, + vec![], ); assert!(result.is_ok()); @@ -2215,7 +2215,7 @@ mod tests { ClarityVersion::Clarity2, false, allow_private, - None, + vec![], ); assert!(result.is_err()); diff --git a/components/clarity-repl/src/repl/session.rs b/components/clarity-repl/src/repl/session.rs index 30c910b71..7066f6985 100644 --- a/components/clarity-repl/src/repl/session.rs +++ b/components/clarity-repl/src/repl/session.rs @@ -183,7 +183,7 @@ impl Session { }; // Result ignored, boot contracts are trusted to be valid - let _ = self.deploy_contract(&contract, None, false, None, None); + let _ = self.deploy_contract(&contract, None, false, None); } } } @@ -528,7 +528,6 @@ impl Session { contract: &ClarityContract, eval_hooks: Option>, cost_track: bool, - test_name: Option, ast: Option<&ContractAST>, ) -> Result> { if contract.epoch != self.current_epoch { @@ -556,32 +555,17 @@ impl Session { }; return Err(vec![diagnostic]); } - let mut hooks: Vec<&mut dyn EvalHook> = Vec::new(); - let mut coverage = test_name.map(TestCoverageReport::new); - if let Some(coverage) = &mut coverage { - hooks.push(coverage); - }; - - if let Some(mut in_hooks) = eval_hooks { - for hook in in_hooks.drain(..) { - hooks.push(hook); - } - } let contract_id = contract.expect_resolved_contract_identifier(Some(&self.interpreter.get_tx_sender())); - let result = self.interpreter.run(contract, ast, cost_track, Some(hooks)); + let result = self.interpreter.run(contract, ast, cost_track, eval_hooks); - result.map(|result| { + result.inspect(|result| { if let EvaluationResult::Contract(contract_result) = &result.result { self.contracts .insert(contract_id.clone(), contract_result.contract.clone()); } - if let Some(coverage) = coverage { - self.coverage_reports.push(coverage); - } - result }) } @@ -593,36 +577,28 @@ impl Session { sender: &str, allow_private: bool, track_costs: bool, - track_coverage: bool, + eval_hooks: Vec<&mut dyn EvalHook>, test_name: String, ) -> Result> { let initial_tx_sender = self.get_tx_sender(); + // Handle fully qualified contract_id and sugared syntax let contract_id_str = if contract.starts_with('S') { contract.to_string() } else { format!("{}.{}", initial_tx_sender, contract) }; - let contract_id = QualifiedContractIdentifier::parse(&contract_id_str).unwrap(); - - let mut hooks: Vec<&mut dyn EvalHook> = vec![]; - let mut coverage = TestCoverageReport::new(test_name.clone()); - if track_coverage { - hooks.push(&mut coverage); - } - - let clarity_version = ClarityVersion::default_for_epoch(self.current_epoch); self.set_tx_sender(sender); let execution = match self.interpreter.call_contract_fn( - &contract_id, + &QualifiedContractIdentifier::parse(&contract_id_str).unwrap(), method, args, self.current_epoch, - clarity_version, + ClarityVersion::default_for_epoch(self.current_epoch), track_costs, allow_private, - Some(hooks), + eval_hooks, ) { Ok(result) => result, Err(e) => { @@ -637,10 +613,6 @@ impl Session { }; self.set_tx_sender(&initial_tx_sender); - if track_coverage { - self.coverage_reports.push(coverage); - } - if let Some(ref cost) = execution.cost { self.costs_reports.push(CostsReport { test_name, @@ -1543,7 +1515,7 @@ mod tests { epoch: StacksEpochId::Epoch2_05, }; - let result = session.deploy_contract(&contract, None, false, None, None); + let result = session.deploy_contract(&contract, None, false, None); assert!(result.is_err(), "Expected error for clarity mismatch"); } @@ -1561,7 +1533,7 @@ mod tests { .clarity_version(ClarityVersion::Clarity2) .build(); - let result = session.deploy_contract(&contract, None, false, None, None); + let result = session.deploy_contract(&contract, None, false, None); assert!(result.is_err()); let err = result.unwrap_err(); assert!(err.len() == 1); @@ -1601,7 +1573,7 @@ mod tests { epoch: StacksEpochId::Epoch25, }; - let _ = session.deploy_contract(&contract, None, false, None, None); + let _ = session.deploy_contract(&contract, None, false, None); // assert data-var is set to 0 assert_eq!( @@ -1659,7 +1631,7 @@ mod tests { // deploy default contract let contract = ClarityContractBuilder::default().build(); - let result = session.deploy_contract(&contract, None, false, None, None); + let result = session.deploy_contract(&contract, None, false, None); assert!(result.is_ok()); } @@ -1681,7 +1653,7 @@ mod tests { BOOT_TESTNET_ADDRESS, false, false, - false, + vec![], "test".to_string(), ); assert_execution_result_value( @@ -1710,7 +1682,7 @@ mod tests { // deploy default contract let contract = ClarityContractBuilder::default().build(); - let _ = session.deploy_contract(&contract, None, false, None, None); + let _ = session.deploy_contract(&contract, None, false, None); dbg!(&contract); @@ -1721,7 +1693,7 @@ mod tests { &session.get_tx_sender(), false, false, - false, + vec![], "test".to_owned(), ); assert_execution_result_value(&result, Value::okay(Value::UInt(1)).unwrap()); @@ -1733,7 +1705,7 @@ mod tests { &session.get_tx_sender(), false, false, - false, + vec![], "test".to_owned(), ); assert_execution_result_value(&result, Value::UInt(1)); From 681a60c73072bc55bc08e1c73891bc89240c84af Mon Sep 17 00:00:00 2001 From: Hugo Caillard <911307+hugocaillard@users.noreply.github.com> Date: Tue, 8 Oct 2024 16:09:48 +0200 Subject: [PATCH 2/7] refactor: code coverage hook --- components/clarinet-deployments/src/lib.rs | 1 - components/clarinet-sdk-wasm/src/core.rs | 69 +-- .../clarinet-sdk/node/tests/reports.test.ts | 6 +- .../clarity-repl/src/analysis/coverage.rs | 414 +++++++++--------- .../src/analysis/coverage_tests.rs | 99 ++--- components/clarity-repl/src/repl/session.rs | 19 - 6 files changed, 301 insertions(+), 307 deletions(-) diff --git a/components/clarinet-deployments/src/lib.rs b/components/clarinet-deployments/src/lib.rs index 6e5165dd3..3f6062322 100644 --- a/components/clarinet-deployments/src/lib.rs +++ b/components/clarinet-deployments/src/lib.rs @@ -239,7 +239,6 @@ fn handle_emulated_contract_call( true, false, vec![], - "deployment".to_string(), ); if let Err(errors) = &result { println!("error: {:?}", errors.first().unwrap().message); diff --git a/components/clarinet-sdk-wasm/src/core.rs b/components/clarinet-sdk-wasm/src/core.rs index f45377aae..7d93f5612 100644 --- a/components/clarinet-sdk-wasm/src/core.rs +++ b/components/clarinet-sdk-wasm/src/core.rs @@ -9,11 +9,10 @@ use clarinet_deployments::{ }; use clarinet_files::chainhook_types::StacksNetwork; use clarinet_files::{FileAccessor, FileLocation, ProjectManifest, WASMFileSystemAccessor}; -use clarity_repl::analysis::coverage::{CoverageReporter, TestCoverageReport}; +use clarity_repl::analysis::coverage::{CoverageHook, CoverageReports}; use clarity_repl::clarity::analysis::contract_interface_builder::{ ContractInterface, ContractInterfaceFunction, ContractInterfaceFunctionAccess, }; -use clarity_repl::clarity::ast::ContractAST; use clarity_repl::clarity::chainstate::StacksAddress; use clarity_repl::clarity::vm::types::{ PrincipalData, QualifiedContractIdentifier, StandardPrincipalData, @@ -23,7 +22,7 @@ use clarity_repl::clarity::{ SymbolicExpression, }; use clarity_repl::repl::clarity_values::{uint8_to_string, uint8_to_value}; -use clarity_repl::repl::session::BOOT_CONTRACTS_DATA; +use clarity_repl::repl::session::{CostsReport, BOOT_CONTRACTS_DATA}; use clarity_repl::repl::{ clarity_values, ClarityCodeSource, ClarityContract, ContractDeployer, Session, SessionSettings, DEFAULT_CLARITY_VERSION, DEFAULT_EPOCH, @@ -287,6 +286,9 @@ pub struct SDK { file_accessor: Box, options: SDKOptions, current_test_name: String, + + coverage_reports: CoverageReports, + costs_reports: Vec, } #[wasm_bindgen] @@ -303,16 +305,21 @@ impl SDK { Self { deployer: String::new(), cache: HashMap::new(), + accounts: HashMap::new(), contracts_interfaces: HashMap::new(), contracts_locations: HashMap::new(), session: None, + file_accessor: fs, options: SDKOptions { track_coverage, track_costs, }, current_test_name: String::new(), + + coverage_reports: CoverageReports::new(), + costs_reports: vec![], } } @@ -713,6 +720,12 @@ impl SDK { }: &CallFnArgs, allow_private: bool, ) -> Result { + let contract_id = if contract.starts_with('S') { + contract.to_string() + } else { + format!("{}.{}", self.deployer, contract) + }; + let test_name = self.current_test_name.clone(); let SDKOptions { track_costs, @@ -720,13 +733,14 @@ impl SDK { } = self.options; let mut hooks: Vec<&mut dyn EvalHook> = Vec::new(); + let mut coverage_hook = if track_coverage { - Some(TestCoverageReport::new(test_name.clone())) + Some(CoverageHook::new()) } else { None }; - if let Some(ref mut hook) = coverage_hook { - hooks.push(hook) + if let Some(hook) = coverage_hook.as_mut() { + hooks.push(hook); } let parsed_args = args @@ -737,14 +751,13 @@ impl SDK { let session = self.get_session_mut(); let execution = session .call_contract_fn( - contract, + &contract_id, method, &parsed_args, sender, allow_private, track_costs, hooks, - test_name, ) .map_err(|diagnostics| { let mut message = format!( @@ -763,8 +776,19 @@ impl SDK { message })?; + if track_costs { + if let Some(ref cost) = execution.cost { + self.costs_reports.push(CostsReport { + test_name, + contract_id, + method: method.to_string(), + args: parsed_args.iter().map(|a| a.to_string()).collect(), + cost_result: cost.clone(), + }); + } + } if let Some(coverage_hook) = coverage_hook { - session.coverage_reports.push(coverage_hook) + self.coverage_reports.merge(coverage_hook.reports); } Ok(execution_result_to_transaction_res(&execution)) @@ -1044,38 +1068,33 @@ impl SDK { ) -> Result { let contracts_locations = self.contracts_locations.clone(); let session = self.get_session_mut(); - let mut coverage_reporter = CoverageReporter::new(); - let mut asts: BTreeMap = BTreeMap::new(); + + let mut asts = BTreeMap::new(); + let mut contract_paths = BTreeMap::new(); + for (contract_id, contract) in session.contracts.iter() { asts.insert(contract_id.clone(), contract.ast.clone()); } - coverage_reporter.asts.append(&mut asts); - for (contract_id, contract_location) in contracts_locations.iter() { - coverage_reporter - .contract_paths - .insert(contract_id.name.to_string(), contract_location.to_string()); + contract_paths.insert(contract_id.name.to_string(), contract_location.to_string()); } if include_boot_contracts { for (contract_id, (_, ast)) in BOOT_CONTRACTS_DATA.iter() { - coverage_reporter - .asts - .insert(contract_id.clone(), ast.clone()); - coverage_reporter.contract_paths.insert( + asts.insert(contract_id.clone(), ast.clone()); + contract_paths.insert( contract_id.name.to_string(), format!("{boot_contracts_path}/{}.clar", contract_id.name), ); } } - coverage_reporter - .reports - .append(&mut session.coverage_reports); - let coverage = coverage_reporter.build_lcov_content(); + let coverage = self + .coverage_reports + .build_lcov_content(&asts, &contract_paths); let mut costs_reports = Vec::new(); - costs_reports.append(&mut session.costs_reports); + costs_reports.append(&mut self.costs_reports); let costs_reports: Vec = costs_reports .iter() .map(SerializableCostsReport::from_vm_costs_report) diff --git a/components/clarinet-sdk/node/tests/reports.test.ts b/components/clarinet-sdk/node/tests/reports.test.ts index 5e6e9eebe..df6ebb870 100644 --- a/components/clarinet-sdk/node/tests/reports.test.ts +++ b/components/clarinet-sdk/node/tests/reports.test.ts @@ -37,7 +37,7 @@ describe("simnet can get code coverage", () => { expect(reports.coverage.length).toBe(0); }); - it("reports coverage if enabled", async () => { + it.only("reports coverage if enabled", async () => { const simnet = await initSimnet("tests/fixtures/Clarinet.toml", true, { trackCoverage: true, trackCosts: false, @@ -45,11 +45,12 @@ describe("simnet can get code coverage", () => { simnet.callPublicFn("counter", "increment", [], address1); simnet.callPublicFn("counter", "increment", [], address1); - simnet.callPrivateFn("counter", "inner-increment", [], address1); + // simnet.callPrivateFn("counter", "inner-increment", [], address1); const reports = simnet.collectReport(false, ""); // increment is called twice + console.log("reports.coverage", reports.coverage); expect(reports.coverage.includes("FNDA:2,increment")).toBe(true); // inner-increment is called one time directly and twice by `increment` expect(reports.coverage.includes("FNDA:3,inner-increment")).toBe(true); @@ -85,6 +86,7 @@ describe("simnet can get costs reports", () => { expect(parsedReports).toHaveLength(1); const report = parsedReports[0]; + console.log("report", report); expect(report.contract_id).toBe(`${simnet.deployer}.counter`); expect(report.method).toBe("increment"); expect(report.cost_result.total.write_count).toBe(3); diff --git a/components/clarity-repl/src/analysis/coverage.rs b/components/clarity-repl/src/analysis/coverage.rs index 5839a3182..5401f2921 100644 --- a/components/clarity-repl/src/analysis/coverage.rs +++ b/components/clarity-repl/src/analysis/coverage.rs @@ -1,8 +1,4 @@ -use std::{ - collections::{BTreeMap, BTreeSet, HashMap, HashSet}, - fs::{create_dir_all, File}, - io::{Error, ErrorKind, Write}, -}; +use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet}; use clarity::vm::{ ast::ContractAST, @@ -14,25 +10,191 @@ use clarity::vm::{ EvalHook, SymbolicExpression, }; -#[derive(Serialize, Deserialize, Debug, Default, Clone)] -pub struct CoverageReporter { - pub reports: Vec, - pub asts: BTreeMap, - pub contract_paths: BTreeMap, -} - type ExprCoverage = HashMap; type ExecutableLines = HashMap>; type ExecutableBranches = HashMap>; -/// One `TestCoverageReport` per test file. #[derive(Serialize, Deserialize, Debug, Default, Clone)] -pub struct TestCoverageReport { - pub test_name: String, +pub struct CoverageReports(HashMap>); + +#[derive(Serialize, Deserialize, Debug, Default, Clone)] +pub struct CoverageHook { + pub reports: CoverageReports, + pub current_test_name: Option, pub contracts_coverage: HashMap, } +impl CoverageHook { + pub fn new() -> Self { + Self { + reports: CoverageReports::default(), + current_test_name: None, + contracts_coverage: HashMap::new(), + } + } +} + +fn retrieve_functions(exprs: &[SymbolicExpression]) -> Vec<(String, u32, u32)> { + let mut functions = vec![]; + for cur_expr in exprs.iter() { + if let Some(define_expr) = DefineFunctionsParsed::try_parse(cur_expr).ok().flatten() { + match define_expr { + DefineFunctionsParsed::PrivateFunction { signature, body: _ } + | DefineFunctionsParsed::PublicFunction { signature, body: _ } + | DefineFunctionsParsed::ReadOnlyFunction { signature, body: _ } => { + let expr = signature.first().expect("Invalid function signature"); + let function_name = expr.match_atom().expect("Invalid function signature"); + + functions.push(( + function_name.to_string(), + cur_expr.span.start_line, + cur_expr.span.end_line, + )); + } + _ => {} + } + continue; + } + } + functions +} + +fn retrieve_executable_lines_and_branches( + exprs: &[SymbolicExpression], +) -> (ExecutableLines, ExecutableBranches) { + let mut lines: ExecutableLines = HashMap::new(); + let mut branches: ExecutableBranches = HashMap::new(); + + for expression in exprs.iter() { + let mut frontier = vec![expression]; + + while let Some(cur_expr) = frontier.pop() { + // Only consider functions declaration and body (ignore arguments) + if let Some(define_expr) = DefineFunctionsParsed::try_parse(cur_expr).ok().flatten() { + match define_expr { + DefineFunctionsParsed::PrivateFunction { signature, body } + | DefineFunctionsParsed::PublicFunction { signature, body } + | DefineFunctionsParsed::ReadOnlyFunction { signature, body } => { + if let Some(function_name) = signature.first() { + frontier.push(function_name); + } + frontier.push(body); + } + _ => {} + } + + continue; + } + + if let Some(children) = cur_expr.match_list() { + if let Some((func, args)) = try_parse_native_func(children) { + // handle codes branches + // (if, asserts!, and, or, match) + match func { + NativeFunctions::If | NativeFunctions::Asserts => { + let (_cond, args) = args.split_first().unwrap(); + branches.insert( + cur_expr.id, + args.iter() + .map(|a| { + let expr = extract_expr_from_list(a); + (expr.span.start_line, expr.id) + }) + .collect(), + ); + } + NativeFunctions::And | NativeFunctions::Or => { + branches.insert( + cur_expr.id, + args.iter() + .map(|a| { + let expr = extract_expr_from_list(a); + (expr.span.start_line, expr.id) + }) + .collect(), + ); + } + NativeFunctions::Match => { + // for match ignore bindings children - some, ok, err + if args.len() == 4 || args.len() == 5 { + let input = args.first().unwrap(); + let left_branch = args.get(2).unwrap(); + let right_branch = args.last().unwrap(); + + let match_branches = [left_branch, right_branch]; + branches.insert( + cur_expr.id, + match_branches + .iter() + .map(|a| { + let expr = extract_expr_from_list(a); + (expr.span.start_line, expr.id) + }) + .collect(), + ); + + frontier.extend([input]); + frontier.extend(match_branches); + } + continue; + } + _ => {} + }; + }; + + // don't count list expressions as a whole, just their children + frontier.extend(children); + } else { + let line = cur_expr.span.start_line; + if let Some(line) = lines.get_mut(&line) { + line.push(cur_expr.id); + } else { + lines.insert(line, vec![cur_expr.id]); + } + } + } + } + (lines, branches) +} + +impl EvalHook for CoverageHook { + fn will_begin_eval( + &mut self, + env: &mut clarity::vm::Environment, + _context: &clarity::vm::LocalContext, + expr: &SymbolicExpression, + ) { + let contract = &env.contract_context.contract_identifier; + let mut contract_report = self.contracts_coverage.remove(contract).unwrap_or_default(); + report_eval(&mut contract_report, expr); + self.contracts_coverage + .insert(contract.clone(), contract_report); + } + + fn did_finish_eval( + &mut self, + _env: &mut clarity::vm::Environment, + _context: &clarity::vm::LocalContext, + _expr: &SymbolicExpression, + _res: &Result, + ) { + } + + fn did_complete(&mut self, _result: Result<&mut clarity::vm::ExecutionResult, String>) { + let test_name = self.current_test_name.clone().unwrap_or_default(); + let coverage = self.reports.0.entry(test_name).or_default(); + + for (contract_id, expr_coverage) in self.contracts_coverage.drain() { + let contract_coverage = coverage.entry(contract_id).or_default(); + for (expr_id, count) in expr_coverage { + *contract_coverage.entry(expr_id).or_insert(0) += count; + } + } + } +} + // LCOV format: + // TN: test name // SF: source file path // FN: line number,function name @@ -44,58 +206,52 @@ pub struct TestCoverageReport { // BRH: number branches hit // BRDA: branch data: line number, expr_id, branch_nb, hit count -impl CoverageReporter { - pub fn new() -> CoverageReporter { - CoverageReporter { - reports: vec![], - asts: BTreeMap::new(), - contract_paths: BTreeMap::new(), - } +impl CoverageReports { + pub fn new() -> Self { + Self(HashMap::new()) } - pub fn write_lcov_file + Copy>( - &self, - filename: P, - ) -> std::io::Result<()> { - let filepath = filename.as_ref().to_path_buf(); - let filepath = filepath.parent().ok_or(Error::new( - ErrorKind::NotFound, - "could not get directory to create coverage file", - ))?; - create_dir_all(filepath)?; - let mut out = File::create(filename)?; - let content = self.build_lcov_content(); - - write!(out, "{}", content)?; - - Ok(()) + pub fn merge(&mut self, other: Self) { + for (test_name, contracts_coverage) in other.0 { + let coverage = self.0.entry(test_name).or_default(); + for (contract_id, expr_coverage) in contracts_coverage { + let contract_coverage = coverage.entry(contract_id).or_default(); + for (expr_id, count) in expr_coverage { + *contract_coverage.entry(expr_id).or_insert(0) += count; + } + } + } } - pub fn build_lcov_content(&self) -> String { + pub fn build_lcov_content( + &mut self, + asts: &BTreeMap, + contract_paths: &BTreeMap, + ) -> String { let mut file_content = String::new(); let mut filtered_asts = HashMap::new(); - for (contract_id, ast) in self.asts.iter() { + for (contract_id, ast) in asts.iter() { let contract_name = contract_id.name.to_string(); - if self.contract_paths.contains_key(&contract_name) { + if contract_paths.contains_key(&contract_name) { filtered_asts.insert( contract_name, ( contract_id, - self.retrieve_functions(&ast.expressions), - self.retrieve_executable_lines_and_branches(&ast.expressions), + retrieve_functions(&ast.expressions), + retrieve_executable_lines_and_branches(&ast.expressions), ), ); } } let mut test_names = BTreeSet::new(); - for report in self.reports.iter() { - test_names.insert(report.test_name.to_string()); + for test_name in self.0.keys() { + test_names.insert(test_name.to_string()); } for test_name in test_names.iter() { - for (contract_name, contract_path) in self.contract_paths.iter() { + for (contract_name, contract_path) in contract_paths.iter() { file_content.push_str(&format!("TN:{}\n", test_name)); file_content.push_str(&format!("SF:{}\n", contract_path)); @@ -113,9 +269,9 @@ impl CoverageReporter { let mut branches_hits = HashSet::new(); let mut branch_execution_counts = BTreeMap::new(); - for report in self.reports.iter() { - if &report.test_name == test_name { - if let Some(coverage) = report.contracts_coverage.get(contract_id) { + for (report_test_name, contracts_coverage) in self.0.iter() { + if report_test_name == test_name { + if let Some(coverage) = contracts_coverage.get(contract_id) { let mut local_function_hits = BTreeSet::new(); for (line, expr_ids) in executable_lines.iter() { @@ -209,166 +365,6 @@ impl CoverageReporter { file_content } - - fn retrieve_functions(&self, exprs: &[SymbolicExpression]) -> Vec<(String, u32, u32)> { - let mut functions = vec![]; - for cur_expr in exprs.iter() { - if let Some(define_expr) = DefineFunctionsParsed::try_parse(cur_expr).ok().flatten() { - match define_expr { - DefineFunctionsParsed::PrivateFunction { signature, body: _ } - | DefineFunctionsParsed::PublicFunction { signature, body: _ } - | DefineFunctionsParsed::ReadOnlyFunction { signature, body: _ } => { - let expr = signature.first().expect("Invalid function signature"); - let function_name = expr.match_atom().expect("Invalid function signature"); - - functions.push(( - function_name.to_string(), - cur_expr.span.start_line, - cur_expr.span.end_line, - )); - } - _ => {} - } - continue; - } - } - functions - } - - fn retrieve_executable_lines_and_branches( - &self, - exprs: &[SymbolicExpression], - ) -> (ExecutableLines, ExecutableBranches) { - let mut lines: ExecutableLines = HashMap::new(); - let mut branches: ExecutableBranches = HashMap::new(); - - for expression in exprs.iter() { - let mut frontier = vec![expression]; - - while let Some(cur_expr) = frontier.pop() { - // Only consider functions declaration and body (ignore arguments) - if let Some(define_expr) = DefineFunctionsParsed::try_parse(cur_expr).ok().flatten() - { - match define_expr { - DefineFunctionsParsed::PrivateFunction { signature, body } - | DefineFunctionsParsed::PublicFunction { signature, body } - | DefineFunctionsParsed::ReadOnlyFunction { signature, body } => { - if let Some(function_name) = signature.first() { - frontier.push(function_name); - } - frontier.push(body); - } - _ => {} - } - - continue; - } - - if let Some(children) = cur_expr.match_list() { - if let Some((func, args)) = try_parse_native_func(children) { - // handle codes branches - // (if, asserts!, and, or, match) - match func { - NativeFunctions::If | NativeFunctions::Asserts => { - let (_cond, args) = args.split_first().unwrap(); - branches.insert( - cur_expr.id, - args.iter() - .map(|a| { - let expr = extract_expr_from_list(a); - (expr.span.start_line, expr.id) - }) - .collect(), - ); - } - NativeFunctions::And | NativeFunctions::Or => { - branches.insert( - cur_expr.id, - args.iter() - .map(|a| { - let expr = extract_expr_from_list(a); - (expr.span.start_line, expr.id) - }) - .collect(), - ); - } - NativeFunctions::Match => { - // for match ignore bindings children - some, ok, err - if args.len() == 4 || args.len() == 5 { - let input = args.first().unwrap(); - let left_branch = args.get(2).unwrap(); - let right_branch = args.last().unwrap(); - - let match_branches = [left_branch, right_branch]; - branches.insert( - cur_expr.id, - match_branches - .iter() - .map(|a| { - let expr = extract_expr_from_list(a); - (expr.span.start_line, expr.id) - }) - .collect(), - ); - - frontier.extend([input]); - frontier.extend(match_branches); - } - continue; - } - _ => {} - }; - }; - - // don't count list expressions as a whole, just their children - frontier.extend(children); - } else { - let line = cur_expr.span.start_line; - if let Some(line) = lines.get_mut(&line) { - line.push(cur_expr.id); - } else { - lines.insert(line, vec![cur_expr.id]); - } - } - } - } - (lines, branches) - } -} - -impl TestCoverageReport { - pub fn new(test_name: String) -> TestCoverageReport { - TestCoverageReport { - test_name, - contracts_coverage: HashMap::new(), - } - } -} - -impl EvalHook for TestCoverageReport { - fn will_begin_eval( - &mut self, - env: &mut clarity::vm::Environment, - _context: &clarity::vm::LocalContext, - expr: &SymbolicExpression, - ) { - let contract = &env.contract_context.contract_identifier; - let mut contract_report = self.contracts_coverage.remove(contract).unwrap_or_default(); - report_eval(&mut contract_report, expr); - self.contracts_coverage - .insert(contract.clone(), contract_report); - } - - fn did_finish_eval( - &mut self, - _env: &mut clarity::vm::Environment, - _context: &clarity::vm::LocalContext, - _expr: &SymbolicExpression, - _res: &Result, - ) { - } - - fn did_complete(&mut self, _result: Result<&mut clarity::vm::ExecutionResult, String>) {} } fn try_parse_native_func( diff --git a/components/clarity-repl/src/analysis/coverage_tests.rs b/components/clarity-repl/src/analysis/coverage_tests.rs index 2a659bcd8..a3e30a521 100644 --- a/components/clarity-repl/src/analysis/coverage_tests.rs +++ b/components/clarity-repl/src/analysis/coverage_tests.rs @@ -1,31 +1,28 @@ -use super::coverage::{CoverageReporter, TestCoverageReport}; +use std::collections::BTreeMap; + +use super::coverage::CoverageHook; use crate::repl::session::Session; use crate::repl::SessionSettings; -fn get_coverage_report(contract: &str, snippets: Vec) -> (TestCoverageReport, String) { +fn get_coverage_report(contract: &str, snippets: Vec) -> (CoverageHook, String) { let mut session = Session::new(SessionSettings::default()); - let mut report = TestCoverageReport::new("test_scenario".into()); - let _ = session.eval(contract.into(), Some(vec![&mut report]), false); + let mut coverage_hook = CoverageHook::new(); + coverage_hook.current_test_name = Some("test_scenario".into()); + let _ = session.eval(contract.into(), Some(vec![&mut coverage_hook]), false); for snippet in snippets { - let _ = session.eval(snippet, Some(vec![&mut report]), false); + let _ = session.eval(snippet, Some(vec![&mut coverage_hook]), false); } let (contract_id, contract) = session.contracts.pop_first().unwrap(); let ast = contract.ast; - let mut coverage_reporter = CoverageReporter::new(); - coverage_reporter - .asts - .insert(contract_id.clone(), ast.clone()); - coverage_reporter - .contract_paths - .insert(contract_id.name.to_string(), "/contract-0.clar".into()); - coverage_reporter.reports.append(&mut vec![report.clone()]); + let asts = BTreeMap::from([(contract_id.clone(), ast.clone())]); + let paths = BTreeMap::from([(contract_id.name.to_string(), "/contract-0.clar".into())]); - let lcov_content = coverage_reporter.build_lcov_content(); + let lcov_content = coverage_hook.reports.build_lcov_content(&asts, &paths); - (report, lcov_content) + (coverage_hook, lcov_content) } fn get_expected_report(body: String) -> String { @@ -262,42 +259,42 @@ fn simple_if_branches_with_exprs() { assert_eq!(cov, expect); } -#[test] -fn hit_all_if_branches() { - let contract = [ - "(define-read-only (add-or-sub (add bool))", - " (if add (+ 1 1) (- 1 1))", - ")", - ] - .join("\n"); - - // hit left branch 3 times and right branch 2 - let snippets: Vec = vec![ - "(contract-call? .contract-0 add-or-sub true)".into(), - "(contract-call? .contract-0 add-or-sub true)".into(), - "(contract-call? .contract-0 add-or-sub true)".into(), - "(contract-call? .contract-0 add-or-sub false)".into(), - "(contract-call? .contract-0 add-or-sub false)".into(), - ]; - let (_, cov) = get_coverage_report(&contract, snippets); - - let expect = get_expected_report( - [ - "FN:1,add-or-sub", - "FNDA:1,add-or-sub", - "FNF:1", - "FNH:1", - "DA:1,1", - "DA:2,5", - "BRF:2", - "BRH:2", - "BRDA:2,8,0,3", - "BRDA:2,8,1,2", - ] - .join("\n"), - ); - assert_eq!(cov, expect); -} +// #[test] +// fn hit_all_if_branches() { +// let contract = [ +// "(define-read-only (add-or-sub (add bool))", +// " (if add (+ 1 1) (- 1 1))", +// ")", +// ] +// .join("\n"); + +// // hit left branch 3 times and right branch 2 +// let snippets: Vec = vec![ +// "(contract-call? .contract-0 add-or-sub true)".into(), +// "(contract-call? .contract-0 add-or-sub true)".into(), +// "(contract-call? .contract-0 add-or-sub true)".into(), +// "(contract-call? .contract-0 add-or-sub false)".into(), +// "(contract-call? .contract-0 add-or-sub false)".into(), +// ]; +// let (_, cov) = get_coverage_report(&contract, snippets); + +// let expect = get_expected_report( +// [ +// "FN:1,add-or-sub", +// "FNDA:1,add-or-sub", +// "FNF:1", +// "FNH:1", +// "DA:1,1", +// "DA:2,5", +// "BRF:2", +// "BRH:2", +// "BRDA:2,8,0,3", +// "BRDA:2,8,1,2", +// ] +// .join("\n"), +// ); +// assert_eq!(cov, expect); +// } #[test] fn simple_asserts_branching() { diff --git a/components/clarity-repl/src/repl/session.rs b/components/clarity-repl/src/repl/session.rs index 7066f6985..980257311 100644 --- a/components/clarity-repl/src/repl/session.rs +++ b/components/clarity-repl/src/repl/session.rs @@ -1,7 +1,6 @@ use super::boot::{STACKS_BOOT_CODE_MAINNET, STACKS_BOOT_CODE_TESTNET}; use super::diagnostic::output_diagnostic; use super::{ClarityCodeSource, ClarityContract, ClarityInterpreter, ContractDeployer}; -use crate::analysis::coverage::TestCoverageReport; use crate::repl::clarity_values::value_to_string; use crate::repl::Settings; use crate::utils; @@ -100,8 +99,6 @@ pub struct Session { pub contracts: BTreeMap, pub interpreter: ClarityInterpreter, api_reference: HashMap, - pub coverage_reports: Vec, - pub costs_reports: Vec, pub show_costs: bool, pub executed: Vec, keywords_reference: HashMap, @@ -123,8 +120,6 @@ impl Session { current_epoch: settings.epoch_id.unwrap_or(StacksEpochId::Epoch2_05), contracts: BTreeMap::new(), api_reference: build_api_reference(), - coverage_reports: vec![], - costs_reports: vec![], show_costs: false, settings, executed: Vec::new(), @@ -578,7 +573,6 @@ impl Session { allow_private: bool, track_costs: bool, eval_hooks: Vec<&mut dyn EvalHook>, - test_name: String, ) -> Result> { let initial_tx_sender = self.get_tx_sender(); @@ -613,16 +607,6 @@ impl Session { }; self.set_tx_sender(&initial_tx_sender); - if let Some(ref cost) = execution.cost { - self.costs_reports.push(CostsReport { - test_name, - contract_id: contract_id_str, - method: method.to_string(), - args: args.iter().map(|a| a.to_string()).collect(), - cost_result: cost.clone(), - }); - } - Ok(execution) } @@ -1654,7 +1638,6 @@ mod tests { false, false, vec![], - "test".to_string(), ); assert_execution_result_value( &result, @@ -1694,7 +1677,6 @@ mod tests { false, false, vec![], - "test".to_owned(), ); assert_execution_result_value(&result, Value::okay(Value::UInt(1)).unwrap()); @@ -1706,7 +1688,6 @@ mod tests { false, false, vec![], - "test".to_owned(), ); assert_execution_result_value(&result, Value::UInt(1)); } From 99928c1906f7cd9c0e97b3010fdb355caaa3ae64 Mon Sep 17 00:00:00 2001 From: Hugo Caillard <911307+hugocaillard@users.noreply.github.com> Date: Tue, 8 Oct 2024 16:27:49 +0200 Subject: [PATCH 3/7] refactor: improve performance and result of the clarity coverage hook --- components/clarinet-sdk-wasm/src/core.rs | 27 +- .../clarinet-sdk/node/tests/reports.test.ts | 4 +- .../clarity-repl/src/analysis/coverage.rs | 270 ++++++++---------- .../src/analysis/coverage_tests.rs | 190 +++++++----- 4 files changed, 263 insertions(+), 228 deletions(-) diff --git a/components/clarinet-sdk-wasm/src/core.rs b/components/clarinet-sdk-wasm/src/core.rs index 7d93f5612..c3b4a8415 100644 --- a/components/clarinet-sdk-wasm/src/core.rs +++ b/components/clarinet-sdk-wasm/src/core.rs @@ -9,7 +9,7 @@ use clarinet_deployments::{ }; use clarinet_files::chainhook_types::StacksNetwork; use clarinet_files::{FileAccessor, FileLocation, ProjectManifest, WASMFileSystemAccessor}; -use clarity_repl::analysis::coverage::{CoverageHook, CoverageReports}; +use clarity_repl::analysis::coverage::{build_lcov_content, CoverageHook, CoverageReport}; use clarity_repl::clarity::analysis::contract_interface_builder::{ ContractInterface, ContractInterfaceFunction, ContractInterfaceFunctionAccess, }; @@ -287,7 +287,7 @@ pub struct SDK { options: SDKOptions, current_test_name: String, - coverage_reports: CoverageReports, + coverage_reports: Vec, costs_reports: Vec, } @@ -318,7 +318,7 @@ impl SDK { }, current_test_name: String::new(), - coverage_reports: CoverageReports::new(), + coverage_reports: vec![], costs_reports: vec![], } } @@ -720,12 +720,6 @@ impl SDK { }: &CallFnArgs, allow_private: bool, ) -> Result { - let contract_id = if contract.starts_with('S') { - contract.to_string() - } else { - format!("{}.{}", self.deployer, contract) - }; - let test_name = self.current_test_name.clone(); let SDKOptions { track_costs, @@ -751,7 +745,7 @@ impl SDK { let session = self.get_session_mut(); let execution = session .call_contract_fn( - &contract_id, + contract, method, &parsed_args, sender, @@ -778,6 +772,11 @@ impl SDK { if track_costs { if let Some(ref cost) = execution.cost { + let contract_id = if contract.starts_with('S') { + contract.to_string() + } else { + format!("{}.{}", self.deployer, contract) + }; self.costs_reports.push(CostsReport { test_name, contract_id, @@ -787,8 +786,8 @@ impl SDK { }); } } - if let Some(coverage_hook) = coverage_hook { - self.coverage_reports.merge(coverage_hook.reports); + if let Some(mut coverage_hook) = coverage_hook { + self.coverage_reports.append(&mut coverage_hook.reports); } Ok(execution_result_to_transaction_res(&execution)) @@ -1089,9 +1088,7 @@ impl SDK { } } - let coverage = self - .coverage_reports - .build_lcov_content(&asts, &contract_paths); + let coverage = build_lcov_content(&self.coverage_reports, &asts, &contract_paths); let mut costs_reports = Vec::new(); costs_reports.append(&mut self.costs_reports); diff --git a/components/clarinet-sdk/node/tests/reports.test.ts b/components/clarinet-sdk/node/tests/reports.test.ts index df6ebb870..1a6409acc 100644 --- a/components/clarinet-sdk/node/tests/reports.test.ts +++ b/components/clarinet-sdk/node/tests/reports.test.ts @@ -45,12 +45,11 @@ describe("simnet can get code coverage", () => { simnet.callPublicFn("counter", "increment", [], address1); simnet.callPublicFn("counter", "increment", [], address1); - // simnet.callPrivateFn("counter", "inner-increment", [], address1); + simnet.callPrivateFn("counter", "inner-increment", [], address1); const reports = simnet.collectReport(false, ""); // increment is called twice - console.log("reports.coverage", reports.coverage); expect(reports.coverage.includes("FNDA:2,increment")).toBe(true); // inner-increment is called one time directly and twice by `increment` expect(reports.coverage.includes("FNDA:3,inner-increment")).toBe(true); @@ -86,7 +85,6 @@ describe("simnet can get costs reports", () => { expect(parsedReports).toHaveLength(1); const report = parsedReports[0]; - console.log("report", report); expect(report.contract_id).toBe(`${simnet.deployer}.counter`); expect(report.method).toBe("increment"); expect(report.cost_result.total.write_count).toBe(3); diff --git a/components/clarity-repl/src/analysis/coverage.rs b/components/clarity-repl/src/analysis/coverage.rs index 5401f2921..ed6935546 100644 --- a/components/clarity-repl/src/analysis/coverage.rs +++ b/components/clarity-repl/src/analysis/coverage.rs @@ -15,11 +15,14 @@ type ExecutableLines = HashMap>; type ExecutableBranches = HashMap>; #[derive(Serialize, Deserialize, Debug, Default, Clone)] -pub struct CoverageReports(HashMap>); +pub struct CoverageReport { + test_name: String, + coverage: HashMap, +} #[derive(Serialize, Deserialize, Debug, Default, Clone)] pub struct CoverageHook { - pub reports: CoverageReports, + pub reports: Vec, pub current_test_name: Option, pub contracts_coverage: HashMap, } @@ -27,7 +30,7 @@ pub struct CoverageHook { impl CoverageHook { pub fn new() -> Self { Self { - reports: CoverageReports::default(), + reports: vec![], current_test_name: None, contracts_coverage: HashMap::new(), } @@ -181,15 +184,10 @@ impl EvalHook for CoverageHook { } fn did_complete(&mut self, _result: Result<&mut clarity::vm::ExecutionResult, String>) { - let test_name = self.current_test_name.clone().unwrap_or_default(); - let coverage = self.reports.0.entry(test_name).or_default(); - - for (contract_id, expr_coverage) in self.contracts_coverage.drain() { - let contract_coverage = coverage.entry(contract_id).or_default(); - for (expr_id, count) in expr_coverage { - *contract_coverage.entry(expr_id).or_insert(0) += count; - } - } + self.reports.push(CoverageReport { + test_name: self.current_test_name.clone().unwrap_or_default(), + coverage: std::mem::take(&mut self.contracts_coverage), + }); } } @@ -206,166 +204,146 @@ impl EvalHook for CoverageHook { // BRH: number branches hit // BRDA: branch data: line number, expr_id, branch_nb, hit count -impl CoverageReports { - pub fn new() -> Self { - Self(HashMap::new()) - } - - pub fn merge(&mut self, other: Self) { - for (test_name, contracts_coverage) in other.0 { - let coverage = self.0.entry(test_name).or_default(); - for (contract_id, expr_coverage) in contracts_coverage { - let contract_coverage = coverage.entry(contract_id).or_default(); - for (expr_id, count) in expr_coverage { - *contract_coverage.entry(expr_id).or_insert(0) += count; - } - } +pub fn build_lcov_content( + reports: &[CoverageReport], + asts: &BTreeMap, + contract_paths: &BTreeMap, +) -> String { + let mut file_content = String::new(); + + let mut filtered_asts = HashMap::new(); + for (contract_id, ast) in asts.iter() { + let contract_name = contract_id.name.to_string(); + if contract_paths.contains_key(&contract_name) { + filtered_asts.insert( + contract_name, + ( + contract_id, + retrieve_functions(&ast.expressions), + retrieve_executable_lines_and_branches(&ast.expressions), + ), + ); } } - pub fn build_lcov_content( - &mut self, - asts: &BTreeMap, - contract_paths: &BTreeMap, - ) -> String { - let mut file_content = String::new(); - - let mut filtered_asts = HashMap::new(); - for (contract_id, ast) in asts.iter() { - let contract_name = contract_id.name.to_string(); - if contract_paths.contains_key(&contract_name) { - filtered_asts.insert( - contract_name, - ( - contract_id, - retrieve_functions(&ast.expressions), - retrieve_executable_lines_and_branches(&ast.expressions), - ), - ); - } - } - - let mut test_names = BTreeSet::new(); - for test_name in self.0.keys() { - test_names.insert(test_name.to_string()); - } - - for test_name in test_names.iter() { - for (contract_name, contract_path) in contract_paths.iter() { - file_content.push_str(&format!("TN:{}\n", test_name)); - file_content.push_str(&format!("SF:{}\n", contract_path)); - - if let Some((contract_id, functions, executable)) = filtered_asts.get(contract_name) - { - for (function, line_start, _) in functions.iter() { - file_content.push_str(&format!("FN:{},{}\n", line_start, function)); - } - let (executable_lines, executables_branches) = executable; - - let mut function_hits = BTreeMap::new(); - let mut line_execution_counts = BTreeMap::new(); - - let mut branches = HashSet::new(); - let mut branches_hits = HashSet::new(); - let mut branch_execution_counts = BTreeMap::new(); - - for (report_test_name, contracts_coverage) in self.0.iter() { - if report_test_name == test_name { - if let Some(coverage) = contracts_coverage.get(contract_id) { - let mut local_function_hits = BTreeSet::new(); - - for (line, expr_ids) in executable_lines.iter() { - // in case of code branches on the line - // retrieve the expression with the most hits - let mut counts = vec![]; - for id in expr_ids { - if let Some(c) = coverage.get(id) { - counts.push(*c); - } - } - let count = counts.iter().max().unwrap_or(&0); + // for consistency in the result, we use a btreemap instead of a hashmap + let reports_per_tests: BTreeMap<&String, Vec<&CoverageReport>> = + reports.iter().fold(BTreeMap::new(), |mut acc, report| { + acc.entry(&report.test_name).or_default().push(report); + acc + }); + + for (test_name, test_reports) in reports_per_tests.iter() { + for (contract_name, contract_path) in contract_paths.iter() { + file_content.push_str(&format!("TN:{}\n", test_name)); + file_content.push_str(&format!("SF:{}\n", contract_path)); + + if let Some((contract_id, functions, executable)) = filtered_asts.get(contract_name) { + for (function, line_start, _) in functions.iter() { + file_content.push_str(&format!("FN:{},{}\n", line_start, function)); + } + let (executable_lines, executables_branches) = executable; + + let mut function_hits = BTreeMap::new(); + let mut line_execution_counts = BTreeMap::new(); + + let mut branches = HashSet::new(); + let mut branches_hits = HashSet::new(); + let mut branch_execution_counts = BTreeMap::new(); + + for report in test_reports { + if let Some(coverage) = report.coverage.get(contract_id) { + let mut local_function_hits = BTreeSet::new(); + + for (line, expr_ids) in executable_lines.iter() { + // in case of code branches on the line + // retrieve the expression with the most hits + let mut counts = vec![]; + for id in expr_ids { + if let Some(c) = coverage.get(id) { + counts.push(*c); + } + } + let count = counts.iter().max().unwrap_or(&0); - let total_count = - line_execution_counts.entry(line).or_insert(0); - *total_count += count; + let total_count = line_execution_counts.entry(line).or_insert(0); + *total_count += count; - if count == &0 { - continue; - } + if count == &0 { + continue; + } - for (function, line_start, line_end) in functions.iter() { - if line >= line_start && line <= line_end { - local_function_hits.insert(function); - // functions hits must have a matching line hit - // if we hit a line inside a function, make sure to count one line hit - if line > line_start { - let hit_count = - line_execution_counts.get(&line_start); - if hit_count.is_none() || hit_count == Some(&0) { - line_execution_counts.insert(line_start, 1); - } - } + for (function, line_start, line_end) in functions.iter() { + if line >= line_start && line <= line_end { + local_function_hits.insert(function); + // functions hits must have a matching line hit + // if we hit a line inside a function, make sure to count one line hit + if line > line_start { + let hit_count = line_execution_counts.get(&line_start); + if hit_count.is_none() || hit_count == Some(&0) { + line_execution_counts.insert(line_start, 1); } } } + } + } - for (expr_id, args) in executables_branches.iter() { - for (i, (line, arg_expr_id)) in args.iter().enumerate() { - let count = coverage.get(arg_expr_id).unwrap_or(&0); - - branches.insert(arg_expr_id); - if count > &0 { - branches_hits.insert(arg_expr_id); - } + for (expr_id, args) in executables_branches.iter() { + for (i, (line, arg_expr_id)) in args.iter().enumerate() { + let count = coverage.get(arg_expr_id).unwrap_or(&0); - let total_count = branch_execution_counts - .entry((line, expr_id, i)) - .or_insert(0); - *total_count += count; - } + branches.insert(arg_expr_id); + if count > &0 { + branches_hits.insert(arg_expr_id); } - for function in local_function_hits.into_iter() { - let hits = function_hits.entry(function).or_insert(0); - *hits += 1 - } + let total_count = branch_execution_counts + .entry((line, expr_id, i)) + .or_insert(0); + *total_count += count; } } - } - for (function, hits) in function_hits.iter() { - file_content.push_str(&format!("FNDA:{},{}\n", hits, function)); + for function in local_function_hits.into_iter() { + let hits = function_hits.entry(function).or_insert(0); + *hits += 1 + } } - file_content.push_str(&format!("FNF:{}\n", functions.len())); - file_content.push_str(&format!("FNH:{}\n", function_hits.len())); + } - for (line, count) in line_execution_counts.iter() { - // the ast can contain elements with a span starting at line 0 that we want to ignore - if line > &&0 { - file_content.push_str(&format!("DA:{},{}\n", line, count)); - } + for (function, hits) in function_hits.iter() { + file_content.push_str(&format!("FNDA:{},{}\n", hits, function)); + } + file_content.push_str(&format!("FNF:{}\n", functions.len())); + file_content.push_str(&format!("FNH:{}\n", function_hits.len())); + + for (line, count) in line_execution_counts.iter() { + // the ast can contain elements with a span starting at line 0 that we want to ignore + if line > &&0 { + file_content.push_str(&format!("DA:{},{}\n", line, count)); } + } - file_content.push_str(&format!("BRF:{}\n", branches.len())); - file_content.push_str(&format!("BRH:{}\n", branches_hits.len())); + file_content.push_str(&format!("BRF:{}\n", branches.len())); + file_content.push_str(&format!("BRH:{}\n", branches_hits.len())); - for ((line, block_id, branch_nb), count) in branch_execution_counts.iter() { - // the ast can contain elements with a span starting at line 0 that we want to ignore - if line > &&0 { - file_content.push_str(&format!( - "BRDA:{},{},{},{}\n", - line, block_id, branch_nb, count - )); - } + for ((line, block_id, branch_nb), count) in branch_execution_counts.iter() { + // the ast can contain elements with a span starting at line 0 that we want to ignore + if line > &&0 { + file_content.push_str(&format!( + "BRDA:{},{},{},{}\n", + line, block_id, branch_nb, count + )); } } - file_content.push_str("end_of_record\n"); } + file_content.push_str("end_of_record\n"); } - - file_content } + + file_content } +// } fn try_parse_native_func( expr: &[SymbolicExpression], @@ -379,7 +357,7 @@ fn try_parse_native_func( fn report_eval(expr_coverage: &mut ExprCoverage, expr: &SymbolicExpression) { if let Some(children) = expr.match_list() { if let Some((func, args)) = try_parse_native_func(children) { - if [Fold, Map, Filter].contains(&func) { + if matches!(func, Fold | Map | Filter) { if let Some(iterator_func) = args.first() { report_eval(expr_coverage, iterator_func); } diff --git a/components/clarity-repl/src/analysis/coverage_tests.rs b/components/clarity-repl/src/analysis/coverage_tests.rs index a3e30a521..3d1ee0333 100644 --- a/components/clarity-repl/src/analysis/coverage_tests.rs +++ b/components/clarity-repl/src/analysis/coverage_tests.rs @@ -1,14 +1,14 @@ use std::collections::BTreeMap; -use super::coverage::CoverageHook; +use super::coverage::{build_lcov_content, CoverageHook}; use crate::repl::session::Session; use crate::repl::SessionSettings; -fn get_coverage_report(contract: &str, snippets: Vec) -> (CoverageHook, String) { +fn get_coverage_report(contract: &str, snippets: Vec, test_name: Option) -> String { let mut session = Session::new(SessionSettings::default()); let mut coverage_hook = CoverageHook::new(); - coverage_hook.current_test_name = Some("test_scenario".into()); + coverage_hook.current_test_name = Some(test_name.unwrap_or("test_scenario".to_string())); let _ = session.eval(contract.into(), Some(vec![&mut coverage_hook]), false); for snippet in snippets { let _ = session.eval(snippet, Some(vec![&mut coverage_hook]), false); @@ -20,9 +20,7 @@ fn get_coverage_report(contract: &str, snippets: Vec) -> (CoverageHook, let asts = BTreeMap::from([(contract_id.clone(), ast.clone())]); let paths = BTreeMap::from([(contract_id.name.to_string(), "/contract-0.clar".into())]); - let lcov_content = coverage_hook.reports.build_lcov_content(&asts, &paths); - - (coverage_hook, lcov_content) + build_lcov_content(&coverage_hook.reports, &asts, &paths) } fn get_expected_report(body: String) -> String { @@ -33,7 +31,7 @@ fn get_expected_report(body: String) -> String { fn line_is_executed() { let contract = "(define-read-only (add) (+ 1 2))"; let snippet = "(contract-call? .contract-0 add)"; - let (_, cov) = get_coverage_report(contract, vec![snippet.into()]); + let cov = get_coverage_report(contract, vec![snippet.into()], None); let expect = get_expected_report( [ @@ -55,7 +53,7 @@ fn line_is_executed_twice() { let contract = "(define-read-only (add) (+ 1 2))"; // call it twice let snippet = "(contract-call? .contract-0 add) (contract-call? .contract-0 add)"; - let (_, cov) = get_coverage_report(contract, vec![snippet.into()]); + let cov = get_coverage_report(contract, vec![snippet.into()], None); let expect = get_expected_report( [ @@ -82,7 +80,7 @@ fn line_count_in_iterator() { ] .join("\n"); let snippet = "(contract-call? .contract-0 map-add-1)"; - let (_, cov) = get_coverage_report(contract.as_str(), vec![snippet.into()]); + let cov = get_coverage_report(contract.as_str(), vec![snippet.into()], None); let expect = get_expected_report( [ @@ -109,7 +107,7 @@ fn function_hit_should_have_line_hit() { let contract = ["(define-read-only (t)", " true", ")"].join("\n"); let snippet = "(contract-call? .contract-0 t)"; - let (_, cov) = get_coverage_report(contract.as_str(), vec![snippet.into()]); + let cov = get_coverage_report(contract.as_str(), vec![snippet.into()], None); let expect = get_expected_report( [ @@ -133,7 +131,7 @@ fn multiple_line_execution() { .join("\n"); let snippet = "(contract-call? .contract-0 add)"; - let (_, cov) = get_coverage_report(contract.as_str(), vec![snippet.into()]); + let cov = get_coverage_report(contract.as_str(), vec![snippet.into()], None); let expect = get_expected_report( [ @@ -167,7 +165,7 @@ fn let_binding() { .join("\n"); let snippet = "(contract-call? .contract-0 add-print)"; - let (_, cov) = get_coverage_report(contract.as_str(), vec![snippet.into()]); + let cov = get_coverage_report(contract.as_str(), vec![snippet.into()], None); let expect = get_expected_report( [ @@ -209,7 +207,7 @@ fn simple_if_branching() { // left path let snippet = "(contract-call? .contract-0 one-or-two true)"; - let (_, cov) = get_coverage_report(contract.as_str(), vec![snippet.into()]); + let cov = get_coverage_report(contract.as_str(), vec![snippet.into()], None); let expect = get_expected_report( [&expect_base[..], &["BRDA:2,8,0,1", "BRDA:2,8,1,0"]] @@ -220,7 +218,7 @@ fn simple_if_branching() { // right path let snippet = "(contract-call? .contract-0 one-or-two false)"; - let (_, cov) = get_coverage_report(contract.as_str(), vec![snippet.into()]); + let cov = get_coverage_report(contract.as_str(), vec![snippet.into()], None); let expect = get_expected_report( [&expect_base[..], &["BRDA:2,8,0,0", "BRDA:2,8,1,1"]] @@ -239,7 +237,7 @@ fn simple_if_branches_with_exprs() { ] .join("\n"); let snippet = "(contract-call? .contract-0 add-or-sub true)"; - let (_, cov) = get_coverage_report(contract.as_str(), vec![snippet.into()]); + let cov = get_coverage_report(contract.as_str(), vec![snippet.into()], None); let expect = get_expected_report( [ @@ -259,42 +257,42 @@ fn simple_if_branches_with_exprs() { assert_eq!(cov, expect); } -// #[test] -// fn hit_all_if_branches() { -// let contract = [ -// "(define-read-only (add-or-sub (add bool))", -// " (if add (+ 1 1) (- 1 1))", -// ")", -// ] -// .join("\n"); - -// // hit left branch 3 times and right branch 2 -// let snippets: Vec = vec![ -// "(contract-call? .contract-0 add-or-sub true)".into(), -// "(contract-call? .contract-0 add-or-sub true)".into(), -// "(contract-call? .contract-0 add-or-sub true)".into(), -// "(contract-call? .contract-0 add-or-sub false)".into(), -// "(contract-call? .contract-0 add-or-sub false)".into(), -// ]; -// let (_, cov) = get_coverage_report(&contract, snippets); - -// let expect = get_expected_report( -// [ -// "FN:1,add-or-sub", -// "FNDA:1,add-or-sub", -// "FNF:1", -// "FNH:1", -// "DA:1,1", -// "DA:2,5", -// "BRF:2", -// "BRH:2", -// "BRDA:2,8,0,3", -// "BRDA:2,8,1,2", -// ] -// .join("\n"), -// ); -// assert_eq!(cov, expect); -// } +#[test] +fn hit_all_if_branches() { + let contract = [ + "(define-read-only (add-or-sub (add bool))", + " (if add (+ 1 1) (- 1 1))", + ")", + ] + .join("\n"); + + // hit left branch 3 times and right branch 2 + let snippets: Vec = vec![ + "(contract-call? .contract-0 add-or-sub true)".into(), + "(contract-call? .contract-0 add-or-sub true)".into(), + "(contract-call? .contract-0 add-or-sub true)".into(), + "(contract-call? .contract-0 add-or-sub false)".into(), + "(contract-call? .contract-0 add-or-sub false)".into(), + ]; + let cov = get_coverage_report(&contract, snippets, None); + + let expect = get_expected_report( + [ + "FN:1,add-or-sub", + "FNDA:5,add-or-sub", + "FNF:1", + "FNH:1", + "DA:1,1", + "DA:2,5", + "BRF:2", + "BRH:2", + "BRDA:2,8,0,3", + "BRDA:2,8,1,2", + ] + .join("\n"), + ); + assert_eq!(cov, expect); +} #[test] fn simple_asserts_branching() { @@ -307,7 +305,7 @@ fn simple_asserts_branching() { // no hit on (err u1) let snippets: Vec = vec!["(contract-call? .contract-0 is-one 1)".into()]; - let (_, cov) = get_coverage_report(&contract, snippets); + let cov = get_coverage_report(&contract, snippets, None); let expect = get_expected_report( [ @@ -327,7 +325,7 @@ fn simple_asserts_branching() { // hit on (err u1) let snippets: Vec = vec!["(contract-call? .contract-0 is-one 2)".into()]; - let (_, cov) = get_coverage_report(&contract, snippets); + let cov = get_coverage_report(&contract, snippets, None); let expect = get_expected_report( [ @@ -359,7 +357,7 @@ fn branch_if_plus_and() { .join("\n"); // calling with `2`, so that evualuation should stop at (> v 2) (which is false) let snippet = "(contract-call? .contract-0 unecessary-ifs 2)"; - let (_, cov) = get_coverage_report(contract.as_str(), vec![snippet.into()]); + let cov = get_coverage_report(contract.as_str(), vec![snippet.into()], None); let expect = get_expected_report( vec![ @@ -398,7 +396,7 @@ fn branch_if_plus_or() { .join("\n"); // calling with 1, so that evualuation should stop at (is-eq v 1) let snippet = "(contract-call? .contract-0 unecessary-ors 1)"; - let (_, cov) = get_coverage_report(contract.as_str(), vec![snippet.into()]); + let cov = get_coverage_report(contract.as_str(), vec![snippet.into()], None); let expect = get_expected_report( vec![ @@ -446,7 +444,7 @@ fn match_opt_oneline() { // left path let snippets: Vec = vec!["(contract-call? .contract-0 match-opt (some 1))".into()]; - let (_, cov) = get_coverage_report(&contract, snippets); + let cov = get_coverage_report(&contract, snippets, None); let expect = get_expected_report( [&expect_base[..], &["BRDA:2,10,0,1", "BRDA:2,10,1,0"]] @@ -457,7 +455,7 @@ fn match_opt_oneline() { // right path let snippets: Vec = vec!["(contract-call? .contract-0 match-opt none)".into()]; - let (_, cov) = get_coverage_report(&contract, snippets); + let cov = get_coverage_report(&contract, snippets, None); let expect = get_expected_report( [&expect_base[..], &["BRDA:2,10,0,0", "BRDA:2,10,1,1"]] @@ -491,7 +489,7 @@ fn match_opt_multiline() { // left path let snippets: Vec = vec!["(contract-call? .contract-0 match-opt (some 1))".into()]; - let (_, cov) = get_coverage_report(&contract, snippets); + let cov = get_coverage_report(&contract, snippets, None); let expect = get_expected_report( [ @@ -512,7 +510,7 @@ fn match_opt_multiline() { // right path let snippets: Vec = vec!["(contract-call? .contract-0 match-opt none)".into()]; - let (_, cov) = get_coverage_report(&contract, snippets); + let cov = get_coverage_report(&contract, snippets, None); let expect = get_expected_report( [ @@ -549,12 +547,12 @@ fn match_res_oneline() { "(contract-call? .contract-0 match-res (ok 2))".into(), "(contract-call? .contract-0 match-res (err u1))".into(), ]; - let (_, cov) = get_coverage_report(&contract, snippets); + let cov = get_coverage_report(&contract, snippets, None); let expect = get_expected_report( [ "FN:1,match-res", - "FNDA:1,match-res", + "FNDA:3,match-res", "FNF:1", "FNH:1", "DA:1,1", @@ -587,7 +585,7 @@ fn fold_iterator() { .join("\n"); let snippets: Vec = vec!["(contract-call? .contract-0 sum)".into()]; - let (_, cov) = get_coverage_report(&contract, snippets); + let cov = get_coverage_report(&contract, snippets, None); let expect = get_expected_report( vec![ @@ -626,7 +624,7 @@ fn map_iterator() { .join("\n"); let snippets: Vec = vec!["(contract-call? .contract-0 square (list 1 2 3))".into()]; - let (_, cov) = get_coverage_report(&contract, snippets); + let cov = get_coverage_report(&contract, snippets, None); let expect = get_expected_report( [ @@ -664,7 +662,7 @@ fn filter_iterator() { let snippets: Vec = vec!["(contract-call? .contract-0 get-positive (list -1 2 3))".into()]; - let (_, cov) = get_coverage_report(&contract, snippets); + let cov = get_coverage_report(&contract, snippets, None); let expect = get_expected_report( [ @@ -686,3 +684,67 @@ fn filter_iterator() { ); assert_eq!(cov, expect); } + +#[test] +fn multiple_test_files() { + let mut session = Session::new(SessionSettings::default()); + + let mut reports = vec![]; + + let contract = "(define-read-only (add) (+ 1 2))"; + let _ = session.eval(contract.into(), None, false); + + let mut coverage_hook = CoverageHook::new(); + coverage_hook.current_test_name = Some("a_test".to_string()); + let snippet = "(contract-call? .contract-0 add)"; + let _ = session.eval(snippet.to_owned(), Some(vec![&mut coverage_hook]), false); + reports.append(coverage_hook.reports.as_mut()); + + let mut coverage_hook = CoverageHook::new(); + coverage_hook.current_test_name = Some("a_test".to_string()); + let snippet = "(contract-call? .contract-0 add)"; + let _ = session.eval(snippet.to_owned(), Some(vec![&mut coverage_hook]), false); + reports.append(coverage_hook.reports.as_mut()); + + let mut coverage_hook = CoverageHook::new(); + coverage_hook.current_test_name = Some("b_test".to_string()); + let snippet = "(contract-call? .contract-0 add)"; + let _ = session.eval(snippet.to_owned(), Some(vec![&mut coverage_hook]), false); + reports.append(coverage_hook.reports.as_mut()); + + let (contract_id, contract) = session.contracts.pop_first().unwrap(); + let ast = contract.ast; + + let asts = BTreeMap::from([(contract_id.clone(), ast.clone())]); + let paths = BTreeMap::from([(contract_id.name.to_string(), "/contract-0.clar".into())]); + + let cov = build_lcov_content(&reports, &asts, &paths); + + assert_eq!( + [ + "TN:a_test", + "SF:/contract-0.clar", + "FN:1,add", + "FNDA:2,add", + "FNF:1", + "FNH:1", + "DA:1,2", + "BRF:0", + "BRH:0", + "end_of_record", + "TN:b_test", + "SF:/contract-0.clar", + "FN:1,add", + "FNDA:1,add", + "FNF:1", + "FNH:1", + "DA:1,1", + "BRF:0", + "BRH:0", + "end_of_record", + "", + ] + .join("\n"), + cov + ); +} From e285150b89d096e851556b7c18e74315e3abc8ba Mon Sep 17 00:00:00 2001 From: Hugo Caillard <911307+hugocaillard@users.noreply.github.com> Date: Wed, 9 Oct 2024 00:08:08 +0200 Subject: [PATCH 4/7] tests: remove .only --- components/clarinet-sdk/node/tests/reports.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/clarinet-sdk/node/tests/reports.test.ts b/components/clarinet-sdk/node/tests/reports.test.ts index 1a6409acc..5e6e9eebe 100644 --- a/components/clarinet-sdk/node/tests/reports.test.ts +++ b/components/clarinet-sdk/node/tests/reports.test.ts @@ -37,7 +37,7 @@ describe("simnet can get code coverage", () => { expect(reports.coverage.length).toBe(0); }); - it.only("reports coverage if enabled", async () => { + it("reports coverage if enabled", async () => { const simnet = await initSimnet("tests/fixtures/Clarinet.toml", true, { trackCoverage: true, trackCosts: false, From 6723f1e1360b9eb23a21104c7723c405bc7f3561 Mon Sep 17 00:00:00 2001 From: Hugo Caillard <911307+hugocaillard@users.noreply.github.com> Date: Wed, 9 Oct 2024 13:11:39 +0200 Subject: [PATCH 5/7] refactor: only instantiate one coverage hook --- components/clarinet-sdk-wasm/src/core.rs | 44 +- .../clarinet-sdk/node/tests/reports.test.ts | 2 +- .../clarity-repl/src/analysis/coverage.rs | 387 +++++++++--------- .../src/analysis/coverage_tests.rs | 111 ++--- package-lock.json | 12 +- 5 files changed, 293 insertions(+), 263 deletions(-) diff --git a/components/clarinet-sdk-wasm/src/core.rs b/components/clarinet-sdk-wasm/src/core.rs index c3b4a8415..137d53c19 100644 --- a/components/clarinet-sdk-wasm/src/core.rs +++ b/components/clarinet-sdk-wasm/src/core.rs @@ -9,7 +9,7 @@ use clarinet_deployments::{ }; use clarinet_files::chainhook_types::StacksNetwork; use clarinet_files::{FileAccessor, FileLocation, ProjectManifest, WASMFileSystemAccessor}; -use clarity_repl::analysis::coverage::{build_lcov_content, CoverageHook, CoverageReport}; +use clarity_repl::analysis::coverage::CoverageHook; use clarity_repl::clarity::analysis::contract_interface_builder::{ ContractInterface, ContractInterfaceFunction, ContractInterfaceFunctionAccess, }; @@ -277,17 +277,14 @@ pub struct SDK { #[wasm_bindgen(getter_with_clone)] pub deployer: String, cache: HashMap, - accounts: HashMap, contracts_locations: HashMap, contracts_interfaces: HashMap, session: Option, - file_accessor: Box, options: SDKOptions, current_test_name: String, - - coverage_reports: Vec, + coverage_hook: Option, costs_reports: Vec, } @@ -302,23 +299,26 @@ impl SDK { let track_coverage = options.as_ref().map_or(false, |o| o.track_coverage); let track_costs = options.as_ref().map_or(false, |o| o.track_costs); + let coverage_hook = if track_coverage { + Some(CoverageHook::new()) + } else { + None + }; + Self { deployer: String::new(), cache: HashMap::new(), - accounts: HashMap::new(), contracts_interfaces: HashMap::new(), contracts_locations: HashMap::new(), session: None, - file_accessor: fs, options: SDKOptions { track_coverage, track_costs, }, current_test_name: String::new(), - - coverage_reports: vec![], + coverage_hook, costs_reports: vec![], } } @@ -721,19 +721,12 @@ impl SDK { allow_private: bool, ) -> Result { let test_name = self.current_test_name.clone(); - let SDKOptions { - track_costs, - track_coverage, - } = self.options; + let SDKOptions { track_costs, .. } = self.options; let mut hooks: Vec<&mut dyn EvalHook> = Vec::new(); - let mut coverage_hook = if track_coverage { - Some(CoverageHook::new()) - } else { - None - }; - if let Some(hook) = coverage_hook.as_mut() { + let mut coverage_hook = self.coverage_hook.take(); + if let Some(ref mut hook) = coverage_hook { hooks.push(hook); } @@ -786,9 +779,8 @@ impl SDK { }); } } - if let Some(mut coverage_hook) = coverage_hook { - self.coverage_reports.append(&mut coverage_hook.reports); - } + + self.coverage_hook = coverage_hook; Ok(execution_result_to_transaction_res(&execution)) } @@ -1054,6 +1046,9 @@ impl SDK { #[wasm_bindgen(js_name=setCurrentTestName)] pub fn set_current_test_name(&mut self, test_name: String) { + if let Some(coverage_hook) = &mut self.coverage_hook { + coverage_hook.set_current_test_name(test_name.clone()); + } self.current_test_name = test_name; } @@ -1088,7 +1083,10 @@ impl SDK { } } - let coverage = build_lcov_content(&self.coverage_reports, &asts, &contract_paths); + let coverage = match self.coverage_hook { + Some(ref mut hook) => hook.collect_lcov_content(&asts, &contract_paths), + None => "".to_string(), + }; let mut costs_reports = Vec::new(); costs_reports.append(&mut self.costs_reports); diff --git a/components/clarinet-sdk/node/tests/reports.test.ts b/components/clarinet-sdk/node/tests/reports.test.ts index 5e6e9eebe..39ef49821 100644 --- a/components/clarinet-sdk/node/tests/reports.test.ts +++ b/components/clarinet-sdk/node/tests/reports.test.ts @@ -43,6 +43,7 @@ describe("simnet can get code coverage", () => { trackCosts: false, }); + simnet.setCurrentTestName("test1"); simnet.callPublicFn("counter", "increment", [], address1); simnet.callPublicFn("counter", "increment", [], address1); simnet.callPrivateFn("counter", "inner-increment", [], address1); @@ -53,7 +54,6 @@ describe("simnet can get code coverage", () => { expect(reports.coverage.includes("FNDA:2,increment")).toBe(true); // inner-increment is called one time directly and twice by `increment` expect(reports.coverage.includes("FNDA:3,inner-increment")).toBe(true); - expect(reports.coverage.startsWith("TN:")).toBe(true); expect(reports.coverage.endsWith("end_of_record\n")).toBe(true); }); diff --git a/components/clarity-repl/src/analysis/coverage.rs b/components/clarity-repl/src/analysis/coverage.rs index ed6935546..e98730088 100644 --- a/components/clarity-repl/src/analysis/coverage.rs +++ b/components/clarity-repl/src/analysis/coverage.rs @@ -23,10 +23,22 @@ pub struct CoverageReport { #[derive(Serialize, Deserialize, Debug, Default, Clone)] pub struct CoverageHook { pub reports: Vec, - pub current_test_name: Option, - pub contracts_coverage: HashMap, + current_test_name: Option, + contracts_coverage: HashMap, } +// LCOV format: +// TN: test name +// SF: source file path +// FN: line number,function name +// FNDA: execution count, function name +// FNF: number functions found +// FNH: number functions hit +// DA: line data: line number, hit count +// BRF: number branches found +// BRH: number branches hit +// BRDA: branch data: line number, expr_id, branch_nb, hit count + impl CoverageHook { pub fn new() -> Self { Self { @@ -35,6 +47,192 @@ impl CoverageHook { contracts_coverage: HashMap::new(), } } + + pub fn set_current_test_name(&mut self, test_name: String) { + self.current_test_name = Some(test_name); + } + + fn clear(&mut self) { + self.current_test_name = None; + self.contracts_coverage.clear(); + self.reports.clear(); + } + + pub fn collect_lcov_content( + &mut self, + asts: &BTreeMap, + contract_paths: &BTreeMap, + ) -> String { + let mut file_content = String::new(); + + let mut filtered_asts = HashMap::new(); + for (contract_id, ast) in asts.iter() { + let contract_name = contract_id.name.to_string(); + if contract_paths.contains_key(&contract_name) { + filtered_asts.insert( + contract_name, + ( + contract_id, + retrieve_functions(&ast.expressions), + retrieve_executable_lines_and_branches(&ast.expressions), + ), + ); + } + } + + // for consistency in the result, we use a btreemap instead of a hashmap + let reports_per_tests: BTreeMap<&String, Vec<&CoverageReport>> = + self.reports + .iter() + .fold(BTreeMap::new(), |mut acc, report| { + acc.entry(&report.test_name).or_default().push(report); + acc + }); + + for (test_name, test_reports) in reports_per_tests.iter() { + file_content.push_str(&format!("TN:{}\n", **test_name)); + for (contract_name, contract_path) in contract_paths.iter() { + file_content.push_str(&format!("SF:{}\n", contract_path)); + + if let Some((contract_id, functions, executable)) = filtered_asts.get(contract_name) + { + for (function, line_start, _) in functions.iter() { + file_content.push_str(&format!("FN:{},{}\n", line_start, function)); + } + let (executable_lines, executables_branches) = executable; + + let mut function_hits = BTreeMap::new(); + let mut line_execution_counts = BTreeMap::new(); + + let mut branches = HashSet::new(); + let mut branches_hits = HashSet::new(); + let mut branch_execution_counts = BTreeMap::new(); + + for report in test_reports { + if let Some(coverage) = report.coverage.get(contract_id) { + let mut local_function_hits = BTreeSet::new(); + + for (line, expr_ids) in executable_lines.iter() { + // in case of code branches on the line + // retrieve the expression with the most hits + let mut counts = vec![]; + for id in expr_ids { + if let Some(c) = coverage.get(id) { + counts.push(*c); + } + } + let count = counts.iter().max().unwrap_or(&0); + + let total_count = line_execution_counts.entry(line).or_insert(0); + *total_count += count; + + if count == &0 { + continue; + } + + for (function, line_start, line_end) in functions.iter() { + if line >= line_start && line <= line_end { + local_function_hits.insert(function); + // functions hits must have a matching line hit + // if we hit a line inside a function, make sure to count one line hit + if line > line_start { + let hit_count = line_execution_counts.get(&line_start); + if hit_count.is_none() || hit_count == Some(&0) { + line_execution_counts.insert(line_start, 1); + } + } + } + } + } + + for (expr_id, args) in executables_branches.iter() { + for (i, (line, arg_expr_id)) in args.iter().enumerate() { + let count = coverage.get(arg_expr_id).unwrap_or(&0); + + branches.insert(arg_expr_id); + if count > &0 { + branches_hits.insert(arg_expr_id); + } + + let total_count = branch_execution_counts + .entry((line, expr_id, i)) + .or_insert(0); + *total_count += count; + } + } + + for function in local_function_hits.into_iter() { + let hits = function_hits.entry(function).or_insert(0); + *hits += 1 + } + } + } + + for (function, hits) in function_hits.iter() { + file_content.push_str(&format!("FNDA:{},{}\n", hits, function)); + } + file_content.push_str(&format!("FNF:{}\n", functions.len())); + file_content.push_str(&format!("FNH:{}\n", function_hits.len())); + + for (line, count) in line_execution_counts.iter() { + // the ast can contain elements with a span starting at line 0 that we want to ignore + if line > &&0 { + file_content.push_str(&format!("DA:{},{}\n", line, count)); + } + } + + file_content.push_str(&format!("BRF:{}\n", branches.len())); + file_content.push_str(&format!("BRH:{}\n", branches_hits.len())); + + for ((line, block_id, branch_nb), count) in branch_execution_counts.iter() { + // the ast can contain elements with a span starting at line 0 that we want to ignore + if line > &&0 { + file_content.push_str(&format!( + "BRDA:{},{},{},{}\n", + line, block_id, branch_nb, count + )); + } + } + } + file_content.push_str("end_of_record\n"); + } + } + + self.clear(); + + file_content + } +} + +impl EvalHook for CoverageHook { + fn will_begin_eval( + &mut self, + env: &mut clarity::vm::Environment, + _context: &clarity::vm::LocalContext, + expr: &SymbolicExpression, + ) { + let contract = &env.contract_context.contract_identifier; + let mut contract_report = self.contracts_coverage.remove(contract).unwrap_or_default(); + report_eval(&mut contract_report, expr); + self.contracts_coverage + .insert(contract.clone(), contract_report); + } + + fn did_finish_eval( + &mut self, + _env: &mut clarity::vm::Environment, + _context: &clarity::vm::LocalContext, + _expr: &SymbolicExpression, + _res: &Result, + ) { + } + + fn did_complete(&mut self, _result: Result<&mut clarity::vm::ExecutionResult, String>) { + self.reports.push(CoverageReport { + test_name: self.current_test_name.clone().unwrap_or_default(), + coverage: self.contracts_coverage.drain().collect(), + }); + } } fn retrieve_functions(exprs: &[SymbolicExpression]) -> Vec<(String, u32, u32)> { @@ -160,191 +358,6 @@ fn retrieve_executable_lines_and_branches( (lines, branches) } -impl EvalHook for CoverageHook { - fn will_begin_eval( - &mut self, - env: &mut clarity::vm::Environment, - _context: &clarity::vm::LocalContext, - expr: &SymbolicExpression, - ) { - let contract = &env.contract_context.contract_identifier; - let mut contract_report = self.contracts_coverage.remove(contract).unwrap_or_default(); - report_eval(&mut contract_report, expr); - self.contracts_coverage - .insert(contract.clone(), contract_report); - } - - fn did_finish_eval( - &mut self, - _env: &mut clarity::vm::Environment, - _context: &clarity::vm::LocalContext, - _expr: &SymbolicExpression, - _res: &Result, - ) { - } - - fn did_complete(&mut self, _result: Result<&mut clarity::vm::ExecutionResult, String>) { - self.reports.push(CoverageReport { - test_name: self.current_test_name.clone().unwrap_or_default(), - coverage: std::mem::take(&mut self.contracts_coverage), - }); - } -} - -// LCOV format: - -// TN: test name -// SF: source file path -// FN: line number,function name -// FNDA: execution count, function name -// FNF: number functions found -// FNH: number functions hit -// DA: line data: line number, hit count -// BRF: number branches found -// BRH: number branches hit -// BRDA: branch data: line number, expr_id, branch_nb, hit count - -pub fn build_lcov_content( - reports: &[CoverageReport], - asts: &BTreeMap, - contract_paths: &BTreeMap, -) -> String { - let mut file_content = String::new(); - - let mut filtered_asts = HashMap::new(); - for (contract_id, ast) in asts.iter() { - let contract_name = contract_id.name.to_string(); - if contract_paths.contains_key(&contract_name) { - filtered_asts.insert( - contract_name, - ( - contract_id, - retrieve_functions(&ast.expressions), - retrieve_executable_lines_and_branches(&ast.expressions), - ), - ); - } - } - - // for consistency in the result, we use a btreemap instead of a hashmap - let reports_per_tests: BTreeMap<&String, Vec<&CoverageReport>> = - reports.iter().fold(BTreeMap::new(), |mut acc, report| { - acc.entry(&report.test_name).or_default().push(report); - acc - }); - - for (test_name, test_reports) in reports_per_tests.iter() { - for (contract_name, contract_path) in contract_paths.iter() { - file_content.push_str(&format!("TN:{}\n", test_name)); - file_content.push_str(&format!("SF:{}\n", contract_path)); - - if let Some((contract_id, functions, executable)) = filtered_asts.get(contract_name) { - for (function, line_start, _) in functions.iter() { - file_content.push_str(&format!("FN:{},{}\n", line_start, function)); - } - let (executable_lines, executables_branches) = executable; - - let mut function_hits = BTreeMap::new(); - let mut line_execution_counts = BTreeMap::new(); - - let mut branches = HashSet::new(); - let mut branches_hits = HashSet::new(); - let mut branch_execution_counts = BTreeMap::new(); - - for report in test_reports { - if let Some(coverage) = report.coverage.get(contract_id) { - let mut local_function_hits = BTreeSet::new(); - - for (line, expr_ids) in executable_lines.iter() { - // in case of code branches on the line - // retrieve the expression with the most hits - let mut counts = vec![]; - for id in expr_ids { - if let Some(c) = coverage.get(id) { - counts.push(*c); - } - } - let count = counts.iter().max().unwrap_or(&0); - - let total_count = line_execution_counts.entry(line).or_insert(0); - *total_count += count; - - if count == &0 { - continue; - } - - for (function, line_start, line_end) in functions.iter() { - if line >= line_start && line <= line_end { - local_function_hits.insert(function); - // functions hits must have a matching line hit - // if we hit a line inside a function, make sure to count one line hit - if line > line_start { - let hit_count = line_execution_counts.get(&line_start); - if hit_count.is_none() || hit_count == Some(&0) { - line_execution_counts.insert(line_start, 1); - } - } - } - } - } - - for (expr_id, args) in executables_branches.iter() { - for (i, (line, arg_expr_id)) in args.iter().enumerate() { - let count = coverage.get(arg_expr_id).unwrap_or(&0); - - branches.insert(arg_expr_id); - if count > &0 { - branches_hits.insert(arg_expr_id); - } - - let total_count = branch_execution_counts - .entry((line, expr_id, i)) - .or_insert(0); - *total_count += count; - } - } - - for function in local_function_hits.into_iter() { - let hits = function_hits.entry(function).or_insert(0); - *hits += 1 - } - } - } - - for (function, hits) in function_hits.iter() { - file_content.push_str(&format!("FNDA:{},{}\n", hits, function)); - } - file_content.push_str(&format!("FNF:{}\n", functions.len())); - file_content.push_str(&format!("FNH:{}\n", function_hits.len())); - - for (line, count) in line_execution_counts.iter() { - // the ast can contain elements with a span starting at line 0 that we want to ignore - if line > &&0 { - file_content.push_str(&format!("DA:{},{}\n", line, count)); - } - } - - file_content.push_str(&format!("BRF:{}\n", branches.len())); - file_content.push_str(&format!("BRH:{}\n", branches_hits.len())); - - for ((line, block_id, branch_nb), count) in branch_execution_counts.iter() { - // the ast can contain elements with a span starting at line 0 that we want to ignore - if line > &&0 { - file_content.push_str(&format!( - "BRDA:{},{},{},{}\n", - line, block_id, branch_nb, count - )); - } - } - } - file_content.push_str("end_of_record\n"); - } - } - - file_content -} -// } - fn try_parse_native_func( expr: &[SymbolicExpression], ) -> Option<(NativeFunctions, &[SymbolicExpression])> { diff --git a/components/clarity-repl/src/analysis/coverage_tests.rs b/components/clarity-repl/src/analysis/coverage_tests.rs index 3d1ee0333..33908766a 100644 --- a/components/clarity-repl/src/analysis/coverage_tests.rs +++ b/components/clarity-repl/src/analysis/coverage_tests.rs @@ -1,14 +1,15 @@ use std::collections::BTreeMap; -use super::coverage::{build_lcov_content, CoverageHook}; +use super::coverage::CoverageHook; use crate::repl::session::Session; use crate::repl::SessionSettings; -fn get_coverage_report(contract: &str, snippets: Vec, test_name: Option) -> String { +fn get_coverage_report(contract: &str, snippets: Vec) -> String { let mut session = Session::new(SessionSettings::default()); let mut coverage_hook = CoverageHook::new(); - coverage_hook.current_test_name = Some(test_name.unwrap_or("test_scenario".to_string())); + coverage_hook.set_current_test_name("test_scenario".to_string()); + let _ = session.eval(contract.into(), Some(vec![&mut coverage_hook]), false); for snippet in snippets { let _ = session.eval(snippet, Some(vec![&mut coverage_hook]), false); @@ -20,7 +21,7 @@ fn get_coverage_report(contract: &str, snippets: Vec, test_name: Option< let asts = BTreeMap::from([(contract_id.clone(), ast.clone())]); let paths = BTreeMap::from([(contract_id.name.to_string(), "/contract-0.clar".into())]); - build_lcov_content(&coverage_hook.reports, &asts, &paths) + coverage_hook.collect_lcov_content(&asts, &paths) } fn get_expected_report(body: String) -> String { @@ -31,7 +32,7 @@ fn get_expected_report(body: String) -> String { fn line_is_executed() { let contract = "(define-read-only (add) (+ 1 2))"; let snippet = "(contract-call? .contract-0 add)"; - let cov = get_coverage_report(contract, vec![snippet.into()], None); + let cov = get_coverage_report(contract, vec![snippet.into()]); let expect = get_expected_report( [ @@ -53,7 +54,7 @@ fn line_is_executed_twice() { let contract = "(define-read-only (add) (+ 1 2))"; // call it twice let snippet = "(contract-call? .contract-0 add) (contract-call? .contract-0 add)"; - let cov = get_coverage_report(contract, vec![snippet.into()], None); + let cov = get_coverage_report(contract, vec![snippet.into()]); let expect = get_expected_report( [ @@ -80,7 +81,7 @@ fn line_count_in_iterator() { ] .join("\n"); let snippet = "(contract-call? .contract-0 map-add-1)"; - let cov = get_coverage_report(contract.as_str(), vec![snippet.into()], None); + let cov = get_coverage_report(contract.as_str(), vec![snippet.into()]); let expect = get_expected_report( [ @@ -107,7 +108,7 @@ fn function_hit_should_have_line_hit() { let contract = ["(define-read-only (t)", " true", ")"].join("\n"); let snippet = "(contract-call? .contract-0 t)"; - let cov = get_coverage_report(contract.as_str(), vec![snippet.into()], None); + let cov = get_coverage_report(contract.as_str(), vec![snippet.into()]); let expect = get_expected_report( [ @@ -131,7 +132,7 @@ fn multiple_line_execution() { .join("\n"); let snippet = "(contract-call? .contract-0 add)"; - let cov = get_coverage_report(contract.as_str(), vec![snippet.into()], None); + let cov = get_coverage_report(contract.as_str(), vec![snippet.into()]); let expect = get_expected_report( [ @@ -165,7 +166,7 @@ fn let_binding() { .join("\n"); let snippet = "(contract-call? .contract-0 add-print)"; - let cov = get_coverage_report(contract.as_str(), vec![snippet.into()], None); + let cov = get_coverage_report(contract.as_str(), vec![snippet.into()]); let expect = get_expected_report( [ @@ -207,7 +208,7 @@ fn simple_if_branching() { // left path let snippet = "(contract-call? .contract-0 one-or-two true)"; - let cov = get_coverage_report(contract.as_str(), vec![snippet.into()], None); + let cov = get_coverage_report(contract.as_str(), vec![snippet.into()]); let expect = get_expected_report( [&expect_base[..], &["BRDA:2,8,0,1", "BRDA:2,8,1,0"]] @@ -218,7 +219,7 @@ fn simple_if_branching() { // right path let snippet = "(contract-call? .contract-0 one-or-two false)"; - let cov = get_coverage_report(contract.as_str(), vec![snippet.into()], None); + let cov = get_coverage_report(contract.as_str(), vec![snippet.into()]); let expect = get_expected_report( [&expect_base[..], &["BRDA:2,8,0,0", "BRDA:2,8,1,1"]] @@ -237,7 +238,7 @@ fn simple_if_branches_with_exprs() { ] .join("\n"); let snippet = "(contract-call? .contract-0 add-or-sub true)"; - let cov = get_coverage_report(contract.as_str(), vec![snippet.into()], None); + let cov = get_coverage_report(contract.as_str(), vec![snippet.into()]); let expect = get_expected_report( [ @@ -274,7 +275,7 @@ fn hit_all_if_branches() { "(contract-call? .contract-0 add-or-sub false)".into(), "(contract-call? .contract-0 add-or-sub false)".into(), ]; - let cov = get_coverage_report(&contract, snippets, None); + let cov = get_coverage_report(&contract, snippets); let expect = get_expected_report( [ @@ -305,7 +306,7 @@ fn simple_asserts_branching() { // no hit on (err u1) let snippets: Vec = vec!["(contract-call? .contract-0 is-one 1)".into()]; - let cov = get_coverage_report(&contract, snippets, None); + let cov = get_coverage_report(&contract, snippets); let expect = get_expected_report( [ @@ -325,7 +326,7 @@ fn simple_asserts_branching() { // hit on (err u1) let snippets: Vec = vec!["(contract-call? .contract-0 is-one 2)".into()]; - let cov = get_coverage_report(&contract, snippets, None); + let cov = get_coverage_report(&contract, snippets); let expect = get_expected_report( [ @@ -357,7 +358,7 @@ fn branch_if_plus_and() { .join("\n"); // calling with `2`, so that evualuation should stop at (> v 2) (which is false) let snippet = "(contract-call? .contract-0 unecessary-ifs 2)"; - let cov = get_coverage_report(contract.as_str(), vec![snippet.into()], None); + let cov = get_coverage_report(contract.as_str(), vec![snippet.into()]); let expect = get_expected_report( vec![ @@ -396,7 +397,7 @@ fn branch_if_plus_or() { .join("\n"); // calling with 1, so that evualuation should stop at (is-eq v 1) let snippet = "(contract-call? .contract-0 unecessary-ors 1)"; - let cov = get_coverage_report(contract.as_str(), vec![snippet.into()], None); + let cov = get_coverage_report(contract.as_str(), vec![snippet.into()]); let expect = get_expected_report( vec![ @@ -444,7 +445,7 @@ fn match_opt_oneline() { // left path let snippets: Vec = vec!["(contract-call? .contract-0 match-opt (some 1))".into()]; - let cov = get_coverage_report(&contract, snippets, None); + let cov = get_coverage_report(&contract, snippets); let expect = get_expected_report( [&expect_base[..], &["BRDA:2,10,0,1", "BRDA:2,10,1,0"]] @@ -455,7 +456,7 @@ fn match_opt_oneline() { // right path let snippets: Vec = vec!["(contract-call? .contract-0 match-opt none)".into()]; - let cov = get_coverage_report(&contract, snippets, None); + let cov = get_coverage_report(&contract, snippets); let expect = get_expected_report( [&expect_base[..], &["BRDA:2,10,0,0", "BRDA:2,10,1,1"]] @@ -489,7 +490,7 @@ fn match_opt_multiline() { // left path let snippets: Vec = vec!["(contract-call? .contract-0 match-opt (some 1))".into()]; - let cov = get_coverage_report(&contract, snippets, None); + let cov = get_coverage_report(&contract, snippets); let expect = get_expected_report( [ @@ -510,7 +511,7 @@ fn match_opt_multiline() { // right path let snippets: Vec = vec!["(contract-call? .contract-0 match-opt none)".into()]; - let cov = get_coverage_report(&contract, snippets, None); + let cov = get_coverage_report(&contract, snippets); let expect = get_expected_report( [ @@ -547,7 +548,7 @@ fn match_res_oneline() { "(contract-call? .contract-0 match-res (ok 2))".into(), "(contract-call? .contract-0 match-res (err u1))".into(), ]; - let cov = get_coverage_report(&contract, snippets, None); + let cov = get_coverage_report(&contract, snippets); let expect = get_expected_report( [ @@ -585,7 +586,7 @@ fn fold_iterator() { .join("\n"); let snippets: Vec = vec!["(contract-call? .contract-0 sum)".into()]; - let cov = get_coverage_report(&contract, snippets, None); + let cov = get_coverage_report(&contract, snippets); let expect = get_expected_report( vec![ @@ -624,7 +625,7 @@ fn map_iterator() { .join("\n"); let snippets: Vec = vec!["(contract-call? .contract-0 square (list 1 2 3))".into()]; - let cov = get_coverage_report(&contract, snippets, None); + let cov = get_coverage_report(&contract, snippets); let expect = get_expected_report( [ @@ -662,7 +663,7 @@ fn filter_iterator() { let snippets: Vec = vec!["(contract-call? .contract-0 get-positive (list -1 2 3))".into()]; - let cov = get_coverage_report(&contract, snippets, None); + let cov = get_coverage_report(&contract, snippets); let expect = get_expected_report( [ @@ -689,40 +690,42 @@ fn filter_iterator() { fn multiple_test_files() { let mut session = Session::new(SessionSettings::default()); - let mut reports = vec![]; - let contract = "(define-read-only (add) (+ 1 2))"; + + // insert 2 contracts + // contract-0 + let _ = session.eval(contract.into(), None, false); + // contract-1 let _ = session.eval(contract.into(), None, false); let mut coverage_hook = CoverageHook::new(); - coverage_hook.current_test_name = Some("a_test".to_string()); + + // call contract-0 twice in test-1 + coverage_hook.set_current_test_name("test-1".to_string()); let snippet = "(contract-call? .contract-0 add)"; let _ = session.eval(snippet.to_owned(), Some(vec![&mut coverage_hook]), false); - reports.append(coverage_hook.reports.as_mut()); - - let mut coverage_hook = CoverageHook::new(); - coverage_hook.current_test_name = Some("a_test".to_string()); let snippet = "(contract-call? .contract-0 add)"; let _ = session.eval(snippet.to_owned(), Some(vec![&mut coverage_hook]), false); - reports.append(coverage_hook.reports.as_mut()); - let mut coverage_hook = CoverageHook::new(); - coverage_hook.current_test_name = Some("b_test".to_string()); + // call contract-0 once and contract-1 once in test-2 + coverage_hook.set_current_test_name("test-2".to_string()); let snippet = "(contract-call? .contract-0 add)"; let _ = session.eval(snippet.to_owned(), Some(vec![&mut coverage_hook]), false); - reports.append(coverage_hook.reports.as_mut()); - - let (contract_id, contract) = session.contracts.pop_first().unwrap(); - let ast = contract.ast; + let snippet = "(contract-call? .contract-1 add)"; + let _ = session.eval(snippet.to_owned(), Some(vec![&mut coverage_hook]), false); - let asts = BTreeMap::from([(contract_id.clone(), ast.clone())]); - let paths = BTreeMap::from([(contract_id.name.to_string(), "/contract-0.clar".into())]); + let mut asts = BTreeMap::new(); + let mut paths = BTreeMap::new(); + for (i, (contract_id, contract)) in session.contracts.iter().enumerate() { + asts.insert(contract_id.clone(), contract.ast.clone()); + paths.insert(contract_id.name.to_string(), format!("/contract-{i}.clar")); + } - let cov = build_lcov_content(&reports, &asts, &paths); + let cov = coverage_hook.collect_lcov_content(&asts, &paths); assert_eq!( [ - "TN:a_test", + "TN:test-1", "SF:/contract-0.clar", "FN:1,add", "FNDA:2,add", @@ -732,7 +735,14 @@ fn multiple_test_files() { "BRF:0", "BRH:0", "end_of_record", - "TN:b_test", + "SF:/contract-1.clar", + "FN:1,add", + "FNF:1", + "FNH:0", + "BRF:0", + "BRH:0", + "end_of_record", + "TN:test-2", "SF:/contract-0.clar", "FN:1,add", "FNDA:1,add", @@ -742,7 +752,16 @@ fn multiple_test_files() { "BRF:0", "BRH:0", "end_of_record", - "", + "SF:/contract-1.clar", + "FN:1,add", + "FNDA:1,add", + "FNF:1", + "FNH:1", + "DA:1,1", + "BRF:0", + "BRH:0", + "end_of_record", + "" ] .join("\n"), cov diff --git a/package-lock.json b/package-lock.json index f52d4a596..101ab0868 100644 --- a/package-lock.json +++ b/package-lock.json @@ -25,20 +25,20 @@ }, "components/clarinet-sdk-wasm/pkg-browser": { "name": "@hirosystems/clarinet-sdk-wasm-browser", - "version": "2.9.0", + "version": "2.10.0-beta-1", "license": "GPL-3.0" }, "components/clarinet-sdk-wasm/pkg-node": { "name": "@hirosystems/clarinet-sdk-wasm", - "version": "2.9.0", + "version": "2.10.0-beta-1", "license": "GPL-3.0" }, "components/clarinet-sdk/browser": { "name": "@hirosystems/clarinet-sdk-browser", - "version": "2.9.0", + "version": "2.10.0-beta-1", "license": "GPL-3.0", "dependencies": { - "@hirosystems/clarinet-sdk-wasm-browser": "^2.9.0", + "@hirosystems/clarinet-sdk-wasm-browser": "^2.10.0-beta-1", "@stacks/transactions": "^6.13.0" } }, @@ -48,10 +48,10 @@ }, "components/clarinet-sdk/node": { "name": "@hirosystems/clarinet-sdk", - "version": "2.9.0", + "version": "2.10.0-beta-1", "license": "GPL-3.0", "dependencies": { - "@hirosystems/clarinet-sdk-wasm": "^2.9.0", + "@hirosystems/clarinet-sdk-wasm": "^2.10.0-beta-1", "@stacks/transactions": "^6.13.0", "kolorist": "^1.8.0", "prompts": "^2.4.2", From 0b27bcb08b76fb0baa2229b973b28f6422f46be4 Mon Sep 17 00:00:00 2001 From: Hugo Caillard <911307+hugocaillard@users.noreply.github.com> Date: Wed, 9 Oct 2024 15:54:33 +0200 Subject: [PATCH 6/7] feat: add coverage hook on execute and runSnippet in simnet --- components/clarinet-sdk-wasm/src/core.rs | 87 +++++++++++++------ .../clarinet-sdk/node/tests/reports.test.ts | 28 ++++++ 2 files changed, 89 insertions(+), 26 deletions(-) diff --git a/components/clarinet-sdk-wasm/src/core.rs b/components/clarinet-sdk-wasm/src/core.rs index 137d53c19..46b8d417c 100644 --- a/components/clarinet-sdk-wasm/src/core.rs +++ b/components/clarinet-sdk-wasm/src/core.rs @@ -859,6 +859,13 @@ impl SDK { args: &DeployContractArgs, advance_chain_tip: bool, ) -> Result { + let mut hooks: Vec<&mut dyn EvalHook> = Vec::new(); + + let mut coverage_hook = self.coverage_hook.take(); + if let Some(ref mut hook) = coverage_hook { + hooks.push(hook); + } + let execution = { let session = self.get_session_mut(); if advance_chain_tip { @@ -888,6 +895,8 @@ impl SDK { } }; + self.coverage_hook = coverage_hook; + if let EvaluationResult::Contract(ref result) = &execution.result { let contract_id = result.contract.analysis.contract_identifier.clone(); if let Some(contract_interface) = &result.contract.analysis.contract_interface { @@ -989,38 +998,64 @@ impl SDK { #[wasm_bindgen(js_name=runSnippet)] pub fn run_snippet(&mut self, snippet: String) -> String { - let session = self.get_session_mut(); - match session.eval(snippet.clone(), None, false) { - Ok(res) => match res.result { - EvaluationResult::Snippet(result) => clarity_values::to_raw_value(&result.result), - EvaluationResult::Contract(_) => unreachable!( - "Contract evaluation result should not be returned from eval_snippet", - ), - }, - Err(diagnostics) => { - let mut message = "error:".to_string(); - diagnostics.iter().for_each(|d| { - message = format!("{message}\n{}", d.message); - }); - message - } + let mut hooks: Vec<&mut dyn EvalHook> = Vec::new(); + + let mut coverage_hook = self.coverage_hook.take(); + if let Some(ref mut hook) = coverage_hook { + hooks.push(hook); } + + let execution = { + let session = self.get_session_mut(); + match session.eval(snippet.clone(), Some(hooks), false) { + Ok(res) => match res.result { + EvaluationResult::Snippet(result) => { + clarity_values::to_raw_value(&result.result) + } + EvaluationResult::Contract(_) => unreachable!( + "Contract evaluation result should not be returned from eval_snippet", + ), + }, + Err(diagnostics) => { + let mut message = "error:".to_string(); + diagnostics.iter().for_each(|d| { + message = format!("{message}\n{}", d.message); + }); + message + } + } + }; + + self.coverage_hook = coverage_hook; + execution } #[wasm_bindgen(js_name=execute)] pub fn execute(&mut self, snippet: String) -> Result { - let session = self.get_session_mut(); - match session.eval(snippet.clone(), None, false) { - Ok(res) => Ok(execution_result_to_transaction_res(&res)), - Err(diagnostics) => { - let message = diagnostics - .iter() - .map(|d| d.message.to_string()) - .collect::>() - .join("\n"); - Err(format!("error: {}", message)) - } + let mut hooks: Vec<&mut dyn EvalHook> = Vec::new(); + + let mut coverage_hook = self.coverage_hook.take(); + if let Some(ref mut hook) = coverage_hook { + hooks.push(hook); } + + let execution = { + let session = self.get_session_mut(); + match session.eval(snippet.clone(), Some(hooks), false) { + Ok(res) => Ok(execution_result_to_transaction_res(&res)), + Err(diagnostics) => { + let message = diagnostics + .iter() + .map(|d| d.message.to_string()) + .collect::>() + .join("\n"); + Err(format!("error: {}", message)) + } + } + }; + + self.coverage_hook = coverage_hook; + execution } #[wasm_bindgen(js_name=executeCommand)] diff --git a/components/clarinet-sdk/node/tests/reports.test.ts b/components/clarinet-sdk/node/tests/reports.test.ts index 39ef49821..6f047abfd 100644 --- a/components/clarinet-sdk/node/tests/reports.test.ts +++ b/components/clarinet-sdk/node/tests/reports.test.ts @@ -107,3 +107,31 @@ describe("simnet can report both costs and coverage", () => { expect(reports.coverage.length).greaterThan(0); }); }); + +describe.only("run-snippet and execute also report coverage", () => { + it("simnet.execute reports coverage", async () => { + const simnet = await initSimnet("tests/fixtures/Clarinet.toml", true, { + trackCoverage: true, + trackCosts: false, + }); + simnet.execute("(contract-call? .counter increment)"); + simnet.execute("(contract-call? .counter increment)"); + + const reports = simnet.collectReport(false, ""); + // line 26, within the increment function, is executed twice + expect(reports.coverage).toContain("DA:26,2"); + }); + + it("simnet.runSnippet reports coverage", async () => { + const simnet = await initSimnet("tests/fixtures/Clarinet.toml", true, { + trackCoverage: true, + trackCosts: false, + }); + simnet.runSnippet("(contract-call? .counter increment)"); + simnet.runSnippet("(contract-call? .counter increment)"); + + const reports = simnet.collectReport(false, ""); + // line 26, within the increment function, is executed twice + expect(reports.coverage).toContain("DA:26,2"); + }); +}); From 3a5f98fbebd62d8a41c5d0d87adae7e5bb889d42 Mon Sep 17 00:00:00 2001 From: Hugo Caillard <911307+hugocaillard@users.noreply.github.com> Date: Wed, 9 Oct 2024 19:31:53 +0200 Subject: [PATCH 7/7] tests: remove .only --- components/clarinet-sdk/node/tests/reports.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/clarinet-sdk/node/tests/reports.test.ts b/components/clarinet-sdk/node/tests/reports.test.ts index 6f047abfd..77aad629c 100644 --- a/components/clarinet-sdk/node/tests/reports.test.ts +++ b/components/clarinet-sdk/node/tests/reports.test.ts @@ -108,7 +108,7 @@ describe("simnet can report both costs and coverage", () => { }); }); -describe.only("run-snippet and execute also report coverage", () => { +describe("simnet.run-snippet and .execute also report coverage", () => { it("simnet.execute reports coverage", async () => { const simnet = await initSimnet("tests/fixtures/Clarinet.toml", true, { trackCoverage: true,