Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: session.eval_with_hooks #1580

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat: session.eval_with_hooks #1580

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion components/clarinet-cli/src/frontend/dap.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ pub fn run_dap() -> Result<(), String> {
}

// Begin execution of the expression in debug mode
match session.eval(expression.clone(), Some(vec![&mut dap]), false) {
match session.eval_with_hooks(expression.clone(), Some(vec![&mut dap]), false) {
Ok(_result) => Ok(()),
Err(_diagnostics) => Err("unable to interpret expression".to_string()),
}
Expand Down
5 changes: 2 additions & 3 deletions components/clarinet-deployments/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,15 +203,15 @@ fn handle_emulated_contract_publish(
epoch,
};

let result = session.deploy_contract(&contract, None, false, contract_ast);
let result = session.deploy_contract(&contract, false, contract_ast);

session.set_tx_sender(&default_tx_sender);
result
}

/// Used to evalutate function arguments passed in deployment plans
fn eval_clarity_string(session: &mut Session, snippet: &str) -> SymbolicExpression {
let eval_result = session.eval(snippet.to_string(), None, false);
let eval_result = session.eval(snippet.to_string(), false);
let value = match eval_result.unwrap().result {
EvaluationResult::Contract(_) => unreachable!(),
EvaluationResult::Snippet(snippet_result) => snippet_result.result,
Expand All @@ -238,7 +238,6 @@ fn handle_emulated_contract_call(
&tx.emulated_sender.to_string(),
true,
false,
vec![],
);
if let Err(errors) = &result {
println!("error: {:?}", errors.first().unwrap().message);
Expand Down
4 changes: 2 additions & 2 deletions components/clarinet-deployments/src/onchain/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ pub fn update_deployment_costs(
.parameters
.iter()
.map(|value| {
let execution = session.eval(value.to_string(), None, false).unwrap();
let execution = session.eval(value.to_string(), false).unwrap();
match execution.result {
EvaluationResult::Snippet(result) => result.result,
_ => unreachable!("Contract result from snippet"),
Expand Down Expand Up @@ -499,7 +499,7 @@ pub fn apply_on_chain_deployment(

let mut function_args = vec![];
for value in tx.parameters.iter() {
let execution = match session.eval(value.to_string(), None, false) {
let execution = match session.eval(value.to_string(), false) {
Ok(res) => res,
Err(_e) => {
let _ = deployment_event_tx.send(DeploymentEvent::Interrupted(
Expand Down
124 changes: 34 additions & 90 deletions components/clarinet-sdk-wasm/src/core.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ use clarinet_deployments::{
};
use clarinet_files::chainhook_types::StacksNetwork;
use clarinet_files::{FileAccessor, FileLocation, ProjectManifest, WASMFileSystemAccessor};
use clarity_repl::analysis::coverage::CoverageHook;
use clarity_repl::clarity::analysis::contract_interface_builder::{
ContractInterface, ContractInterfaceFunction, ContractInterfaceFunctionAccess,
};
Expand All @@ -18,8 +17,7 @@ use clarity_repl::clarity::vm::types::{
PrincipalData, QualifiedContractIdentifier, StandardPrincipalData,
};
use clarity_repl::clarity::{
Address, ClarityVersion, EvalHook, EvaluationResult, ExecutionResult, StacksEpochId,
SymbolicExpression,
Address, ClarityVersion, EvaluationResult, ExecutionResult, StacksEpochId, SymbolicExpression,
};
use clarity_repl::repl::clarity_values::{uint8_to_string, uint8_to_value};
use clarity_repl::repl::session::{CostsReport, BOOT_CONTRACTS_DATA};
Expand Down Expand Up @@ -284,7 +282,6 @@ pub struct SDK {
file_accessor: Box<dyn FileAccessor>,
options: SDKOptions,
current_test_name: String,
coverage_hook: Option<CoverageHook>,
costs_reports: Vec<CostsReport>,
}

Expand All @@ -299,12 +296,6 @@ 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(),
Expand All @@ -318,7 +309,6 @@ impl SDK {
track_costs,
},
current_test_name: String::new(),
coverage_hook,
costs_reports: vec![],
}
}
Expand Down Expand Up @@ -453,6 +443,9 @@ impl SDK {
}

let mut session = initiate_session_from_manifest(&manifest);
if self.options.track_coverage {
session.enable_coverage();
}
let executed_contracts = update_session_with_deployment_plan(
&mut session,
&deployment,
Expand Down Expand Up @@ -723,13 +716,6 @@ impl SDK {
let test_name = self.current_test_name.clone();
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()
.map(|a| SymbolicExpression::atom_value(uint8_to_value(a)))
Expand All @@ -744,7 +730,6 @@ impl SDK {
sender,
allow_private,
track_costs,
hooks,
)
.map_err(|diagnostics| {
let mut message = format!(
Expand Down Expand Up @@ -780,8 +765,6 @@ impl SDK {
}
}

self.coverage_hook = coverage_hook;

Ok(execution_result_to_transaction_res(&execution))
}

Expand Down Expand Up @@ -859,13 +842,6 @@ impl SDK {
args: &DeployContractArgs,
advance_chain_tip: bool,
) -> Result<TransactionRes, String> {
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 {
Expand All @@ -880,7 +856,7 @@ impl SDK {
epoch: session.current_epoch,
};

match session.deploy_contract(&contract, None, false, None) {
match session.deploy_contract(&contract, false, None) {
Ok(res) => res,
Err(diagnostics) => {
let mut message = format!(
Expand All @@ -895,8 +871,6 @@ 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 {
Expand Down Expand Up @@ -998,64 +972,38 @@ impl SDK {

#[wasm_bindgen(js_name=runSnippet)]
pub fn run_snippet(&mut self, snippet: String) -> String {
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
}
let session = self.get_session_mut();
match session.eval(snippet.clone(), 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<TransactionRes, String> {
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::<Vec<String>>()
.join("\n");
Err(format!("error: {}", message))
}
let session = self.get_session_mut();
match session.eval(snippet.clone(), false) {
Ok(res) => Ok(execution_result_to_transaction_res(&res)),
Err(diagnostics) => {
let message = diagnostics
.iter()
.map(|d| d.message.to_string())
.collect::<Vec<String>>()
.join("\n");
Err(format!("error: {}", message))
}
};

self.coverage_hook = coverage_hook;
execution
}
}

#[wasm_bindgen(js_name=executeCommand)]
Expand All @@ -1081,9 +1029,8 @@ 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());
}
let session = self.get_session_mut();
session.set_test_name(test_name.clone());
self.current_test_name = test_name;
}

Expand Down Expand Up @@ -1118,10 +1065,7 @@ impl SDK {
}
}

let coverage = match self.coverage_hook {
Some(ref mut hook) => hook.collect_lcov_content(&asts, &contract_paths),
None => "".to_string(),
};
let coverage = session.collect_lcov_content(&asts, &contract_paths);

let mut costs_reports = Vec::new();
costs_reports.append(&mut self.costs_reports);
Expand Down
2 changes: 1 addition & 1 deletion components/clarity-events/src/bin.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ pub fn main() {
let file = FileLocation::from_path_string(&cmd.file_path).unwrap();
let snippet = file.read_content_as_utf8().unwrap();
let mut session = Session::new(SessionSettings::default());
let mut contract_analysis = match session.eval(snippet, None, false) {
let mut contract_analysis = match session.eval(snippet, false) {
Ok(execution) => match execution.result {
EvaluationResult::Contract(evaluation) => evaluation.contract.analysis,
_ => {
Expand Down
48 changes: 30 additions & 18 deletions components/clarity-repl/src/analysis/coverage_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,18 +1,16 @@
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<String>) -> String {
let mut session = Session::new(SessionSettings::default());
session.enable_coverage();
session.set_test_name("test_scenario".to_string());

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);
let _ = session.eval(contract.into(), false);
for snippet in snippets {
let _ = session.eval(snippet, Some(vec![&mut coverage_hook]), false);
let _ = session.eval(snippet, false);
}

let (contract_id, contract) = session.contracts.pop_first().unwrap();
Expand All @@ -21,7 +19,7 @@ fn get_coverage_report(contract: &str, snippets: Vec<String>) -> String {
let asts = BTreeMap::from([(contract_id.clone(), ast.clone())]);
let paths = BTreeMap::from([(contract_id.name.to_string(), "/contract-0.clar".into())]);

coverage_hook.collect_lcov_content(&asts, &paths)
session.collect_lcov_content(&asts, &paths)
}

fn get_expected_report(body: String) -> String {
Expand Down Expand Up @@ -689,30 +687,29 @@ fn filter_iterator() {
#[test]
fn multiple_test_files() {
let mut session = Session::new(SessionSettings::default());
session.enable_coverage();

let contract = "(define-read-only (add) (+ 1 2))";

// insert 2 contracts
// contract-0
let _ = session.eval(contract.into(), None, false);
let _ = session.eval(contract.into(), false);
// contract-1
let _ = session.eval(contract.into(), None, false);

let mut coverage_hook = CoverageHook::new();
let _ = session.eval(contract.into(), false);

// call contract-0 twice in test-1
coverage_hook.set_current_test_name("test-1".to_string());
session.set_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 _ = session.eval(snippet.to_owned(), false);
let snippet = "(contract-call? .contract-0 add)";
let _ = session.eval(snippet.to_owned(), Some(vec![&mut coverage_hook]), false);
let _ = session.eval(snippet.to_owned(), false);

// call contract-0 once and contract-1 once in test-2
coverage_hook.set_current_test_name("test-2".to_string());
session.set_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 _ = session.eval(snippet.to_owned(), false);
let snippet = "(contract-call? .contract-1 add)";
let _ = session.eval(snippet.to_owned(), Some(vec![&mut coverage_hook]), false);
let _ = session.eval(snippet.to_owned(), false);

let mut asts = BTreeMap::new();
let mut paths = BTreeMap::new();
Expand All @@ -721,10 +718,25 @@ fn multiple_test_files() {
paths.insert(contract_id.name.to_string(), format!("/contract-{i}.clar"));
}

let cov = coverage_hook.collect_lcov_content(&asts, &paths);
let cov = session.collect_lcov_content(&asts, &paths);

assert_eq!(
[
"TN:",
"SF:/contract-0.clar",
"FN:1,add",
"FNF:1",
"FNH:0",
"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",
Comment on lines +725 to +739
Copy link
Collaborator Author

@hugocaillard hugocaillard Oct 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a side effect, this is the wanted behaviour for code coverage.
Because session.deploy_contract uses the session coverage hook.
This is therefore the initial analysis of the contracts.
If we wanted to pass the eval_hooks to update_session_with_deployment_plan (here) instead, this would make pass eval_hooks args in a whole lot more of functions (and it comes with some difficulties down the line)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this a wanted behavior for better coverage handling; It doesn't really impact code coverage % though

"TN:test-1",
"SF:/contract-0.clar",
"FN:1,add",
Expand Down
Loading
Loading