From 1ad9199bcfbc5dd52166038a25ddfc7b03d90981 Mon Sep 17 00:00:00 2001 From: Blaine Bublitz Date: Fri, 11 Aug 2023 09:38:39 -0700 Subject: [PATCH] feat(nargo): Add `--exact` flag to `nargo test` (#2272) * feat(nargo): Add `--exact` flag to `nargo test` * fix(lsp): Make test command work in context of workspaces * reviews --- crates/lsp/src/lib.rs | 11 ++++-- crates/nargo_cli/src/cli/test_cmd.rs | 27 ++++++++++++--- crates/noirc_frontend/src/hir/mod.rs | 52 ++++++++++++++++++++-------- 3 files changed, 68 insertions(+), 22 deletions(-) diff --git a/crates/lsp/src/lib.rs b/crates/lsp/src/lib.rs index b0b0871ad3f..83e196a183b 100644 --- a/crates/lsp/src/lib.rs +++ b/crates/lsp/src/lib.rs @@ -22,6 +22,7 @@ use nargo::prepare_package; use nargo_toml::{find_package_manifest, resolve_workspace_from_toml}; use noirc_driver::check_crate; use noirc_errors::{DiagnosticKind, FileDiagnostic}; +use noirc_frontend::hir::FunctionNameMatch; use serde_json::Value as JsonValue; use tower::Service; @@ -176,7 +177,8 @@ fn on_code_lens_request( let fm = &context.file_manager; let files = fm.as_simple_files(); - let tests = context.get_all_test_functions_in_crate_matching(&crate_id, ""); + let tests = context + .get_all_test_functions_in_crate_matching(&crate_id, FunctionNameMatch::Anything); for (func_name, func_id) in tests { let location = context.function_meta(&func_id).name.location; @@ -196,7 +198,12 @@ fn on_code_lens_request( let command = Command { title: TEST_CODELENS_TITLE.into(), command: TEST_COMMAND.into(), - arguments: Some(vec![func_name.into()]), + arguments: Some(vec![ + "--package".into(), + format!("{}", package.name).into(), + "--exact".into(), + func_name.into(), + ]), }; let lens = CodeLens { range, command: command.into(), data: None }; diff --git a/crates/nargo_cli/src/cli/test_cmd.rs b/crates/nargo_cli/src/cli/test_cmd.rs index ecbee5b4668..95d7f907196 100644 --- a/crates/nargo_cli/src/cli/test_cmd.rs +++ b/crates/nargo_cli/src/cli/test_cmd.rs @@ -5,7 +5,11 @@ use clap::Args; use nargo::{ops::execute_circuit, package::Package, prepare_package}; use nargo_toml::{find_package_manifest, resolve_workspace_from_toml}; use noirc_driver::{compile_no_check, CompileOptions}; -use noirc_frontend::{graph::CrateName, hir::Context, node_interner::FuncId}; +use noirc_frontend::{ + graph::CrateName, + hir::{Context, FunctionNameMatch}, + node_interner::FuncId, +}; use termcolor::{Color, ColorChoice, ColorSpec, StandardStream, WriteColor}; use crate::{cli::check_cmd::check_crate_and_report_errors, errors::CliError}; @@ -22,6 +26,10 @@ pub(crate) struct TestCommand { #[arg(long)] show_output: bool, + /// Only run tests that match exactly + #[clap(long)] + exact: bool, + /// The name of the package to test #[clap(long)] package: Option, @@ -35,13 +43,22 @@ pub(crate) fn run( args: TestCommand, config: NargoConfig, ) -> Result<(), CliError> { - let test_name: String = args.test_name.unwrap_or_else(|| "".to_owned()); - let toml_path = find_package_manifest(&config.program_dir)?; let workspace = resolve_workspace_from_toml(&toml_path, args.package)?; + let pattern = match &args.test_name { + Some(name) => { + if args.exact { + FunctionNameMatch::Exact(name) + } else { + FunctionNameMatch::Contains(name) + } + } + None => FunctionNameMatch::Anything, + }; + for package in &workspace { - run_tests(backend, package, &test_name, args.show_output, &args.compile_options)?; + run_tests(backend, package, pattern, args.show_output, &args.compile_options)?; } Ok(()) @@ -50,7 +67,7 @@ pub(crate) fn run( fn run_tests( backend: &B, package: &Package, - test_name: &str, + test_name: FunctionNameMatch, show_output: bool, compile_options: &CompileOptions, ) -> Result<(), CliError> { diff --git a/crates/noirc_frontend/src/hir/mod.rs b/crates/noirc_frontend/src/hir/mod.rs index 91a6006d096..497935c8ece 100644 --- a/crates/noirc_frontend/src/hir/mod.rs +++ b/crates/noirc_frontend/src/hir/mod.rs @@ -25,6 +25,13 @@ pub struct Context { pub storage_slots: HashMap, } +#[derive(Debug, Copy, Clone)] +pub enum FunctionNameMatch<'a> { + Anything, + Exact(&'a str), + Contains(&'a str), +} + pub type StorageSlot = u32; impl Context { @@ -52,10 +59,29 @@ impl Context { self.crate_graph.iter_keys() } + // TODO: Decide if we actually need `function_name` and `fully_qualified_function_name` pub fn function_name(&self, id: &FuncId) -> &str { self.def_interner.function_name(id) } + pub fn fully_qualified_function_name(&self, crate_id: &CrateId, id: &FuncId) -> String { + let def_map = self.def_map(crate_id).expect("The local crate should be analyzed already"); + + let name = self.def_interner.function_name(id); + + let meta = self.def_interner.function_meta(id); + let module = self.module(meta.module_id); + + let parent = + def_map.get_module_path_with_separator(meta.module_id.local_id.0, module.parent, "::"); + + if parent.is_empty() { + name.into() + } else { + format!("{parent}::{name}") + } + } + pub fn function_meta(&self, func_id: &FuncId) -> FuncMeta { self.def_interner.function_meta(func_id) } @@ -76,7 +102,7 @@ impl Context { pub fn get_all_test_functions_in_crate_matching( &self, crate_id: &CrateId, - pattern: &str, + pattern: FunctionNameMatch, ) -> Vec<(String, FuncId)> { let interner = &self.def_interner; let def_map = self.def_map(crate_id).expect("The local crate should be analyzed already"); @@ -84,20 +110,16 @@ impl Context { def_map .get_all_test_functions(interner) .filter_map(|id| { - let name = interner.function_name(&id); - - let meta = interner.function_meta(&id); - let module = self.module(meta.module_id); - - let parent = def_map.get_module_path_with_separator( - meta.module_id.local_id.0, - module.parent, - "::", - ); - let path = - if parent.is_empty() { name.into() } else { format!("{parent}::{name}") }; - - path.contains(pattern).then_some((path, id)) + let fully_qualified_name = self.fully_qualified_function_name(crate_id, &id); + match &pattern { + FunctionNameMatch::Anything => Some((fully_qualified_name, id)), + FunctionNameMatch::Exact(pattern) => { + (&fully_qualified_name == pattern).then_some((fully_qualified_name, id)) + } + FunctionNameMatch::Contains(pattern) => { + fully_qualified_name.contains(pattern).then_some((fully_qualified_name, id)) + } + } }) .collect() }