diff --git a/CHANGELOG.md b/CHANGELOG.md index 17f5063a..9c5425bf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ - **aiken-lang**: The compiler now raises a warning when attempting to destructure a record constructor without using named fields. See [#1084](https://github.com/aiken-lang/aiken/issues/1084). @KtorZ - **aiken-lang**: Fix blueprint schema definitions related to pairs (no longer omit (sometimes) Pairs definitions, and generate them as data List). See [#1086](https://github.com/aiken-lang/aiken/issues/1086) and [#970](https://github.com/aiken-lang/aiken/issues/970). @KtorZ +- **aiken-project**: Improve feedback returned when matching tests or modules - see [#1092](https://github.com/aiken-lang/aiken/issues/1092). @KtorZ ## v1.1.10 - 2025-01-21 @@ -31,7 +32,7 @@ ### Changed - **aiken-project**: The `aiken.toml` file no longer supports `v1` and `v2` for the plutus version field. @rvcas -- **aiken-project**: `Error::TomlLoading` now looks much better - [see](https://github.com/aiken-lang/aiken/issues/1032#issuecomment-2562122101). @rvcas +- **aiken-project**: `Error::TomlLoading` now looks much better - see [#1032](https://github.com/aiken-lang/aiken/issues/1032#issuecomment-2562122101). @rvcas - **aiken-lang**: 10-20% optimization improvements via case-constr, rearranging function definitions (while maintaining dependency ordering), and allowing inlining in if_then_else_error cases which preserve the same error semantics for a program. @Microproofs diff --git a/crates/aiken-project/src/error.rs b/crates/aiken-project/src/error.rs index 0c210121..b5048195 100644 --- a/crates/aiken-project/src/error.rs +++ b/crates/aiken-project/src/error.rs @@ -594,6 +594,8 @@ pub enum Warning { CompilerVersionMismatch { demanded: String, current: String }, #[error("No configuration found for environment {env}.")] NoConfigurationForEnv { env: String }, + #[error("Suspicious test filter (-m) yielding no test scenarios.")] + SuspiciousTestMatch { test: String }, } impl ExtraData for Warning { @@ -603,7 +605,8 @@ impl ExtraData for Warning { | Warning::DependencyAlreadyExists { .. } | Warning::InvalidModuleName { .. } | Warning::CompilerVersionMismatch { .. } - | Warning::NoConfigurationForEnv { .. } => None, + | Warning::NoConfigurationForEnv { .. } + | Warning::SuspiciousTestMatch { .. } => None, Warning::Type { warning, .. } => warning.extra_data(), } } @@ -616,7 +619,8 @@ impl GetSource for Warning { Warning::NoValidators | Warning::DependencyAlreadyExists { .. } | Warning::NoConfigurationForEnv { .. } - | Warning::CompilerVersionMismatch { .. } => None, + | Warning::CompilerVersionMismatch { .. } + | Warning::SuspiciousTestMatch { .. } => None, } } @@ -627,7 +631,8 @@ impl GetSource for Warning { | Warning::InvalidModuleName { .. } | Warning::DependencyAlreadyExists { .. } | Warning::NoConfigurationForEnv { .. } - | Warning::CompilerVersionMismatch { .. } => None, + | Warning::CompilerVersionMismatch { .. } + | Warning::SuspiciousTestMatch { .. } => None, } } } @@ -644,7 +649,8 @@ impl Diagnostic for Warning { | Warning::InvalidModuleName { .. } | Warning::NoConfigurationForEnv { .. } | Warning::DependencyAlreadyExists { .. } - | Warning::CompilerVersionMismatch { .. } => None, + | Warning::CompilerVersionMismatch { .. } + | Warning::SuspiciousTestMatch { .. } => None, } } @@ -655,7 +661,8 @@ impl Diagnostic for Warning { | Warning::NoValidators | Warning::DependencyAlreadyExists { .. } | Warning::NoConfigurationForEnv { .. } - | Warning::CompilerVersionMismatch { .. } => None, + | Warning::CompilerVersionMismatch { .. } + | Warning::SuspiciousTestMatch { .. } => None, } } @@ -676,6 +683,7 @@ impl Diagnostic for Warning { Warning::NoConfigurationForEnv { .. } => { Some(Box::new("aiken::project::config::missing::env")) } + Warning::SuspiciousTestMatch { .. } => Some(Box::new("aiken::check::suspicious_match")), } } @@ -696,6 +704,12 @@ impl Diagnostic for Warning { Warning::NoConfigurationForEnv { .. } => Some(Box::new( "When configuration keys are missing for a target environment, no 'config' module will be created. This may lead to issues down the line.", )), + Warning::SuspiciousTestMatch { test } => Some(Box::new( + format!( + "Did you mean to match all tests within a specific module? Like so:\n\n╰─▶ {}", + format!("-m \"{test}.{{..}}\"").if_supports_color(Stderr, |s| s.bold()), + ) + )), } } } diff --git a/crates/aiken-project/src/lib.rs b/crates/aiken-project/src/lib.rs index 7b1cc13f..d18528a6 100644 --- a/crates/aiken-project/src/lib.rs +++ b/crates/aiken-project/src/lib.rs @@ -980,12 +980,35 @@ where "" }; - let match_names = match_split_dot.next().map(|names| { + let match_names = match_split_dot.next().and_then(|names| { let names = names.replace(&['{', '}'][..], ""); - let names_split_comma = names.split(','); - names_split_comma.map(str::to_string).collect() + let result = names_split_comma + .filter_map(|s| { + let s = s.trim(); + if s.is_empty() { + None + } else { + Some(s.to_string()) + } + }) + .collect::>(); + + if result.is_empty() { + None + } else { + Some(result) + } + }); + + self.event_listener.handle_event(Event::CollectingTests { + matching_module: if match_module.is_empty() { + None + } else { + Some(match_module.to_string()) + }, + matching_names: match_names.clone().unwrap_or_default(), }); (match_module.to_string(), match_names) @@ -993,6 +1016,13 @@ where .collect::>)>>() }); + if match_tests.is_none() { + self.event_listener.handle_event(Event::CollectingTests { + matching_module: None, + matching_names: vec![], + }); + } + for checked_module in self.checked_modules.values() { if checked_module.package != self.config.name.to_string() { continue; @@ -1064,6 +1094,27 @@ where )); } + // NOTE: The filtering syntax for tests isn't quite obvious. A common pitfall when willing + // to match over a top-level module is to simple pass in `-m module_name`, which will be + // treated as a match for a test name. + // + // In this case, we raise an additional warning to suggest a different match syntax, which + // may be quite helpful in teaching users how to deal with module filtering. + match match_tests.as_deref() { + Some(&[(ref s, Some(ref names))]) if tests.is_empty() => { + if let [test] = names.as_slice() { + self.warnings.push(Warning::SuspiciousTestMatch { + test: if s.is_empty() { + test.to_string() + } else { + s.to_string() + }, + }); + } + } + _ => (), + } + Ok(tests) } diff --git a/crates/aiken-project/src/telemetry.rs b/crates/aiken-project/src/telemetry.rs index 221d8bfd..b056f708 100644 --- a/crates/aiken-project/src/telemetry.rs +++ b/crates/aiken-project/src/telemetry.rs @@ -43,6 +43,10 @@ pub enum Event { name: String, path: PathBuf, }, + CollectingTests { + matching_module: Option, + matching_names: Vec, + }, RunningTests, RunningBenchmarks, FinishedTests { diff --git a/crates/aiken-project/src/telemetry/terminal.rs b/crates/aiken-project/src/telemetry/terminal.rs index afacfcef..2be6bc52 100644 --- a/crates/aiken-project/src/telemetry/terminal.rs +++ b/crates/aiken-project/src/telemetry/terminal.rs @@ -117,6 +117,53 @@ impl EventListener for Terminal { name.if_supports_color(Stderr, |s| s.bright_blue()), ); } + Event::CollectingTests { + matching_module, + matching_names, + } => { + eprintln!( + "{:>13} {tests} {module}", + "Collecting" + .if_supports_color(Stderr, |s| s.bold()) + .if_supports_color(Stderr, |s| s.purple()), + tests = if matching_names.is_empty() { + if matching_module.is_some() { + "all tests scenarios" + .if_supports_color(Stderr, |s| s.bold()) + .to_string() + } else { + "all tests scenarios".to_string() + } + } else { + format!( + "test{} {}", + if matching_names.len() > 1 { "s" } else { "" }, + matching_names + .iter() + .map(|s| format!("*{s}*")) + .collect::>() + .join(", ") + .if_supports_color(Stderr, |s| s.bold()) + ) + }, + module = match matching_module { + None => format!( + "across {}", + if matching_names.is_empty() { + "all modules".to_string() + } else { + "all modules" + .if_supports_color(Stderr, |s| s.bold()) + .to_string() + } + ), + Some(module) => format!( + "within module(s): {}", + format!("*{module}*").if_supports_color(Stderr, |s| s.bold()) + ), + } + ); + } Event::RunningTests => { eprintln!( "{} {}", diff --git a/crates/aiken-project/src/watch.rs b/crates/aiken-project/src/watch.rs index 201e2871..8f54eae7 100644 --- a/crates/aiken-project/src/watch.rs +++ b/crates/aiken-project/src/watch.rs @@ -123,6 +123,7 @@ where if !json { for warning in &warnings { + eprintln!(); warning.report() }