From 6f178d8ca8a9f7a29db749c077214082462741ff Mon Sep 17 00:00:00 2001 From: Hugo C <911307+hugocaillard@users.noreply.github.com> Date: Wed, 9 Oct 2024 23:47:16 +0200 Subject: [PATCH] refactor: better eval hooks handling in contract calls (#1575) * refactor: better eval hooks handling in contract calls * refactor: code coverage hook * refactor: improve performance and result of the clarity coverage hook * tests: remove .only * refactor: only instantiate one coverage hook * feat: add coverage hook on execute and runSnippet in simnet * tests: remove .only --- .../clarinet-cli/src/generate/contract.rs | 5 - components/clarinet-deployments/src/lib.rs | 25 +- components/clarinet-sdk-wasm/src/core.rs | 175 ++++--- .../clarinet-sdk/node/tests/reports.test.ts | 30 +- components/clarity-lsp/src/common/state.rs | 1 - .../clarity-repl/src/analysis/coverage.rs | 449 +++++++++--------- .../src/analysis/coverage_tests.rs | 158 ++++-- components/clarity-repl/src/analysis/mod.rs | 3 +- .../clarity-repl/src/repl/interpreter.rs | 28 +- components/clarity-repl/src/repl/session.rs | 79 +-- package-lock.json | 12 +- 11 files changed, 527 insertions(+), 438 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..3f6062322 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,8 +238,7 @@ fn handle_emulated_contract_call( &tx.emulated_sender.to_string(), true, false, - false, - "deployment".to_string(), + vec![], ); if let Err(errors) = &result { println!("error: {:?}", errors.first().unwrap().message); @@ -930,7 +917,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..46b8d417c 100644 --- a/components/clarinet-sdk-wasm/src/core.rs +++ b/components/clarinet-sdk-wasm/src/core.rs @@ -9,20 +9,20 @@ 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::CoverageHook; 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, }; 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; +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, @@ -277,15 +277,15 @@ 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_hook: Option, + costs_reports: Vec, } #[wasm_bindgen] @@ -299,6 +299,12 @@ 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(), @@ -312,6 +318,8 @@ impl SDK { track_costs, }, current_test_name: String::new(), + coverage_hook, + costs_reports: vec![], } } @@ -449,7 +457,6 @@ impl SDK { &mut session, &deployment, Some(&artifacts.asts), - false, Some(DEFAULT_EPOCH), ); @@ -714,10 +721,14 @@ 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 = self.coverage_hook.take(); + if let Some(ref mut hook) = coverage_hook { + hooks.push(hook); + } let parsed_args = args .iter() @@ -733,8 +744,7 @@ impl SDK { sender, allow_private, track_costs, - track_coverage, - test_name, + hooks, ) .map_err(|diagnostics| { let mut message = format!( @@ -753,6 +763,25 @@ impl SDK { message })?; + 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, + method: method.to_string(), + args: parsed_args.iter().map(|a| a.to_string()).collect(), + cost_result: cost.clone(), + }); + } + } + + self.coverage_hook = coverage_hook; + Ok(execution_result_to_transaction_res(&execution)) } @@ -830,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 { @@ -844,7 +880,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!( @@ -859,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 { @@ -960,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)] @@ -1017,6 +1081,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; } @@ -1030,38 +1097,34 @@ 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 = 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 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..77aad629c 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); }); @@ -107,3 +107,31 @@ describe("simnet can report both costs and coverage", () => { expect(reports.coverage.length).greaterThan(0); }); }); + +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, + 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"); + }); +}); 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/coverage.rs b/components/clarity-repl/src/analysis/coverage.rs index 5839a3182..e98730088 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,22 +10,21 @@ 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 contracts_coverage: HashMap, +pub struct CoverageReport { + test_name: String, + coverage: HashMap, +} + +#[derive(Serialize, Deserialize, Debug, Default, Clone)] +pub struct CoverageHook { + pub reports: Vec, + current_test_name: Option, + contracts_coverage: HashMap, } // LCOV format: @@ -44,59 +39,59 @@ 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 { +impl CoverageHook { + pub fn new() -> Self { + Self { reports: vec![], - asts: BTreeMap::new(), - contract_paths: BTreeMap::new(), + current_test_name: None, + contracts_coverage: 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 set_current_test_name(&mut self, test_name: String) { + self.current_test_name = Some(test_name); } - pub fn build_lcov_content(&self) -> String { + 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 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 test_names.iter() { - for (contract_name, contract_path) in self.contract_paths.iter() { - file_content.push_str(&format!("TN:{}\n", test_name)); + // 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) @@ -113,67 +108,63 @@ 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) { - 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); - } + 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 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 in local_function_hits.into_iter() { + let hits = function_hits.entry(function).or_insert(0); + *hits += 1 + } } } @@ -207,168 +198,164 @@ impl CoverageReporter { } } + self.clear(); + 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, - )); - } - _ => {} +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)> { + 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; + _ => {} } + continue; } - functions } + functions +} - fn retrieve_executable_lines_and_branches( - &self, - exprs: &[SymbolicExpression], - ) -> (ExecutableLines, ExecutableBranches) { - let mut lines: ExecutableLines = HashMap::new(); - let mut branches: ExecutableBranches = HashMap::new(); +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]; + 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); + 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 => { + 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, - args.iter() + 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); } - 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; - } - _ => {} - }; + continue; + } + _ => {} }; - - // don't count list expressions as a whole, just their children - frontier.extend(children); + }; + + // 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 { - 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.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>) {} + (lines, branches) } fn try_parse_native_func( @@ -383,7 +370,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 2a659bcd8..33908766a 100644 --- a/components/clarity-repl/src/analysis/coverage_tests.rs +++ b/components/clarity-repl/src/analysis/coverage_tests.rs @@ -1,31 +1,27 @@ -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) -> 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.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 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 lcov_content = coverage_reporter.build_lcov_content(); + let asts = BTreeMap::from([(contract_id.clone(), ast.clone())]); + let paths = BTreeMap::from([(contract_id.name.to_string(), "/contract-0.clar".into())]); - (report, lcov_content) + coverage_hook.collect_lcov_content(&asts, &paths) } fn get_expected_report(body: String) -> String { @@ -36,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()]); + let cov = get_coverage_report(contract, vec![snippet.into()]); let expect = get_expected_report( [ @@ -58,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()]); + let cov = get_coverage_report(contract, vec![snippet.into()]); let expect = get_expected_report( [ @@ -85,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()]); + let cov = get_coverage_report(contract.as_str(), vec![snippet.into()]); let expect = get_expected_report( [ @@ -112,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()]); + let cov = get_coverage_report(contract.as_str(), vec![snippet.into()]); let expect = get_expected_report( [ @@ -136,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()]); + let cov = get_coverage_report(contract.as_str(), vec![snippet.into()]); let expect = get_expected_report( [ @@ -170,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()]); + let cov = get_coverage_report(contract.as_str(), vec![snippet.into()]); let expect = get_expected_report( [ @@ -212,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()]); + 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"]] @@ -223,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()]); + 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"]] @@ -242,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()]); + let cov = get_coverage_report(contract.as_str(), vec![snippet.into()]); let expect = get_expected_report( [ @@ -279,12 +275,12 @@ 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); + let cov = get_coverage_report(&contract, snippets); let expect = get_expected_report( [ "FN:1,add-or-sub", - "FNDA:1,add-or-sub", + "FNDA:5,add-or-sub", "FNF:1", "FNH:1", "DA:1,1", @@ -310,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); + let cov = get_coverage_report(&contract, snippets); let expect = get_expected_report( [ @@ -330,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); + let cov = get_coverage_report(&contract, snippets); let expect = get_expected_report( [ @@ -362,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()]); + let cov = get_coverage_report(contract.as_str(), vec![snippet.into()]); let expect = get_expected_report( vec![ @@ -401,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()]); + let cov = get_coverage_report(contract.as_str(), vec![snippet.into()]); let expect = get_expected_report( vec![ @@ -449,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); + let cov = get_coverage_report(&contract, snippets); let expect = get_expected_report( [&expect_base[..], &["BRDA:2,10,0,1", "BRDA:2,10,1,0"]] @@ -460,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); + let cov = get_coverage_report(&contract, snippets); let expect = get_expected_report( [&expect_base[..], &["BRDA:2,10,0,0", "BRDA:2,10,1,1"]] @@ -494,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); + let cov = get_coverage_report(&contract, snippets); let expect = get_expected_report( [ @@ -515,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); + let cov = get_coverage_report(&contract, snippets); let expect = get_expected_report( [ @@ -552,12 +548,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); let expect = get_expected_report( [ "FN:1,match-res", - "FNDA:1,match-res", + "FNDA:3,match-res", "FNF:1", "FNH:1", "DA:1,1", @@ -590,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); + let cov = get_coverage_report(&contract, snippets); let expect = get_expected_report( vec![ @@ -629,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); + let cov = get_coverage_report(&contract, snippets); let expect = get_expected_report( [ @@ -667,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); + let cov = get_coverage_report(&contract, snippets); let expect = get_expected_report( [ @@ -689,3 +685,85 @@ fn filter_iterator() { ); assert_eq!(cov, expect); } + +#[test] +fn multiple_test_files() { + let mut session = Session::new(SessionSettings::default()); + + 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(); + + // 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); + let snippet = "(contract-call? .contract-0 add)"; + let _ = session.eval(snippet.to_owned(), Some(vec![&mut coverage_hook]), false); + + // 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); + let snippet = "(contract-call? .contract-1 add)"; + let _ = session.eval(snippet.to_owned(), Some(vec![&mut coverage_hook]), false); + + 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 = coverage_hook.collect_lcov_content(&asts, &paths); + + assert_eq!( + [ + "TN:test-1", + "SF:/contract-0.clar", + "FN:1,add", + "FNDA:2,add", + "FNF:1", + "FNH:1", + "DA:1,2", + "BRF:0", + "BRH:0", + "end_of_record", + "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", + "FNF:1", + "FNH:1", + "DA:1,1", + "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/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..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(), @@ -183,7 +178,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 +523,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 +550,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 +572,27 @@ impl Session { sender: &str, allow_private: bool, track_costs: bool, - track_coverage: bool, - test_name: String, + eval_hooks: Vec<&mut dyn EvalHook>, ) -> 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,20 +607,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, - contract_id: contract_id_str, - method: method.to_string(), - args: args.iter().map(|a| a.to_string()).collect(), - cost_result: cost.clone(), - }); - } - Ok(execution) } @@ -1543,7 +1499,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 +1517,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 +1557,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 +1615,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,8 +1637,7 @@ mod tests { BOOT_TESTNET_ADDRESS, false, false, - false, - "test".to_string(), + vec![], ); assert_execution_result_value( &result, @@ -1710,7 +1665,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,8 +1676,7 @@ mod tests { &session.get_tx_sender(), false, false, - false, - "test".to_owned(), + vec![], ); assert_execution_result_value(&result, Value::okay(Value::UInt(1)).unwrap()); @@ -1733,8 +1687,7 @@ mod tests { &session.get_tx_sender(), false, false, - false, - "test".to_owned(), + vec![], ); assert_execution_result_value(&result, Value::UInt(1)); } 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",