diff --git a/crates/bulloak/tests/check.rs b/crates/bulloak/tests/check.rs index 42b39e0..fed7a38 100644 --- a/crates/bulloak/tests/check.rs +++ b/crates/bulloak/tests/check.rs @@ -736,3 +736,24 @@ fn checks_no_submodule_multi_root_error() { assert!(stderr.contains(r#"an error occurred while parsing the tree: separator missing at tree root #1 "no_submodule_multi_root". Expected to find `::` between the contract name and the function name when multiple roots exist"#)); } + +/// Regression test for https://github.com/defi-wonderland/bulloak/pull/9#issuecomment-3710452952 +/// When multiple roots share the same condition, check should report the missing hoisted setup hook. +#[test] +fn checks_missing_hoisted_setup_hook_for_shared_condition() { + let cwd = env::current_dir().unwrap(); + let binary_path = get_binary_path(); + let tree_path = cwd + .join("tests") + .join("check") + .join("hoisted_hook_regression.tree"); + + let output = cmd(&binary_path, "check", &tree_path, &["-l", "noir"]); + let stderr = String::from_utf8(output.stderr).unwrap(); + + assert!( + stderr.contains(r#"Missing setup hook "when_passing_valid_parameters""#), + "Expected check to report missing setup hook 'when_passing_valid_parameters', but it did not.\nStderr:\n{}", + stderr + ); +} diff --git a/crates/bulloak/tests/check/hoisted_hook_regression.tree b/crates/bulloak/tests/check/hoisted_hook_regression.tree new file mode 100644 index 0000000..b007cbf --- /dev/null +++ b/crates/bulloak/tests/check/hoisted_hook_regression.tree @@ -0,0 +1,9 @@ +hoisted_hook_regression::constructor_with_minter +└── when passing valid parameters + ├── it sets name + └── it sets symbol + +hoisted_hook_regression::constructor_with_initial_supply +└── when passing valid parameters + ├── it sets name + └── it sets symbol diff --git a/crates/bulloak/tests/check/hoisted_hook_regression_test.nr b/crates/bulloak/tests/check/hoisted_hook_regression_test.nr new file mode 100644 index 0000000..9a86324 --- /dev/null +++ b/crates/bulloak/tests/check/hoisted_hook_regression_test.nr @@ -0,0 +1,22 @@ +// Generated by bulloak + + +mod constructor_with_minter { + #[test] + unconstrained fn test_when_passing_valid_parameters() { + when_passing_valid_parameters(); + // it sets name + // it sets symbol + } + +} + +mod constructor_with_initial_supply { + #[test] + unconstrained fn test_when_passing_valid_parameters() { + when_passing_valid_parameters(); + // it sets name + // it sets symbol + } + +} diff --git a/crates/bulloak/tests/check/hoisted_setup_hooks_test.nr b/crates/bulloak/tests/check/hoisted_setup_hooks_test.nr index 4559165..d1de9ff 100644 --- a/crates/bulloak/tests/check/hoisted_setup_hooks_test.nr +++ b/crates/bulloak/tests/check/hoisted_setup_hooks_test.nr @@ -6,6 +6,7 @@ unconstrained fn when_b() { mod foo { + use super::when_b; #[test] unconstrained fn test_when_b() { when_b(); @@ -21,6 +22,7 @@ mod foo { } mod bar { + // import is not present in this module. `check` ignores imports. #[test] unconstrained fn test_when_b() { when_b(); diff --git a/crates/bulloak/tests/scaffold.rs b/crates/bulloak/tests/scaffold.rs index b5d6179..7597095 100644 --- a/crates/bulloak/tests/scaffold.rs +++ b/crates/bulloak/tests/scaffold.rs @@ -350,3 +350,43 @@ fn scaffold_dissambiguates_function_name_collisions() { assert_eq!(expected.trim(), actual.trim()); } } + +/// Regression test for https://github.com/defi-wonderland/bulloak/pull/9#issuecomment-3710452952 +/// When multiple roots share the same condition, the hoisted setup hook should be generated. +#[test] +fn scaffold_noir_generates_hoisted_setup_hook_for_shared_condition() { + let cwd = env::current_dir().unwrap(); + let binary_path = get_binary_path(); + let tests_path = cwd.join("tests").join("scaffold"); + let tree_path = tests_path.join("hoisted_hook_regression.tree"); + + let output = cmd(&binary_path, "scaffold", &tree_path, &["-l", "noir"]); + let actual = String::from_utf8(output.stdout).unwrap(); + + assert!( + actual.contains("unconstrained fn when_passing_valid_parameters()"), + "Expected hoisted setup hook 'when_passing_valid_parameters' to be generated, but it was not.\nActual output:\n{}", + actual + ); + + // Also verify the full expected output matches + let noir_filename = tests_path.join("hoisted_hook_regression_test.nr"); + let expected = fs::read_to_string(noir_filename).unwrap(); + assert_eq!(expected.trim(), actual.trim()); +} + +#[test] +fn scaffold_noir_generates_skips_modifiers_on_imported_hoks() { + let cwd = env::current_dir().unwrap(); + let binary_path = get_binary_path(); + let tests_path = cwd.join("tests").join("scaffold"); + let tree_path = tests_path.join("hoisted_hook_skip_modifiers.tree"); + + let output = cmd(&binary_path, "scaffold", &tree_path, &["-l", "noir", "-m"]); + let actual = String::from_utf8(output.stdout).unwrap(); + + // Verify the full expected output matches + let noir_filename = tests_path.join("hoisted_hook_skip_modifiers_test.nr"); + let expected = fs::read_to_string(noir_filename).unwrap(); + assert_eq!(expected.trim(), actual.trim()); +} diff --git a/crates/bulloak/tests/scaffold/hoisted_hook_regression.tree b/crates/bulloak/tests/scaffold/hoisted_hook_regression.tree new file mode 100644 index 0000000..b007cbf --- /dev/null +++ b/crates/bulloak/tests/scaffold/hoisted_hook_regression.tree @@ -0,0 +1,9 @@ +hoisted_hook_regression::constructor_with_minter +└── when passing valid parameters + ├── it sets name + └── it sets symbol + +hoisted_hook_regression::constructor_with_initial_supply +└── when passing valid parameters + ├── it sets name + └── it sets symbol diff --git a/crates/bulloak/tests/scaffold/hoisted_hook_regression_test.nr b/crates/bulloak/tests/scaffold/hoisted_hook_regression_test.nr new file mode 100644 index 0000000..c7732b1 --- /dev/null +++ b/crates/bulloak/tests/scaffold/hoisted_hook_regression_test.nr @@ -0,0 +1,28 @@ +// Generated by bulloak + +/// Setup hook for condition +unconstrained fn when_passing_valid_parameters() { +} + + +mod constructor_with_minter { + use super::when_passing_valid_parameters; + #[test] + unconstrained fn test_when_passing_valid_parameters() { + when_passing_valid_parameters(); + // it sets name + // it sets symbol + } + +} + +mod constructor_with_initial_supply { + use super::when_passing_valid_parameters; + #[test] + unconstrained fn test_when_passing_valid_parameters() { + when_passing_valid_parameters(); + // it sets name + // it sets symbol + } + +} diff --git a/crates/bulloak/tests/scaffold/hoisted_hook_skip_modifiers.tree b/crates/bulloak/tests/scaffold/hoisted_hook_skip_modifiers.tree new file mode 100644 index 0000000..5a31aab --- /dev/null +++ b/crates/bulloak/tests/scaffold/hoisted_hook_skip_modifiers.tree @@ -0,0 +1,9 @@ +hoisted_hook_skip_modifiers::constructor_with_minter +└── when passing valid parameters + ├── it sets name + └── it sets symbol + +hoisted_hook_skip_modifiers::constructor_with_initial_supply +└── when passing valid parameters + ├── it sets name + └── it sets symbol diff --git a/crates/bulloak/tests/scaffold/hoisted_hook_skip_modifiers_test.nr b/crates/bulloak/tests/scaffold/hoisted_hook_skip_modifiers_test.nr new file mode 100644 index 0000000..6b4b725 --- /dev/null +++ b/crates/bulloak/tests/scaffold/hoisted_hook_skip_modifiers_test.nr @@ -0,0 +1,20 @@ +// Generated by bulloak + + +mod constructor_with_minter { + #[test] + unconstrained fn test_when_passing_valid_parameters() { + // it sets name + // it sets symbol + } + +} + +mod constructor_with_initial_supply { + #[test] + unconstrained fn test_when_passing_valid_parameters() { + // it sets name + // it sets symbol + } + +} diff --git a/crates/noir/src/check/rules/structural_match.rs b/crates/noir/src/check/rules/structural_match.rs index 940fd87..60163b4 100644 --- a/crates/noir/src/check/rules/structural_match.rs +++ b/crates/noir/src/check/rules/structural_match.rs @@ -906,6 +906,7 @@ mod compare_trees_test { functions: vec![], modules: vec![Module { name: "module1".to_string(), + imported_hooks: Vec::new(), functions: vec![Function::SetupHook(SetupHook { name: "when_something".to_string(), })], @@ -938,6 +939,7 @@ mod compare_trees_test { functions: vec![], modules: vec![Module { name: "wrong_name".to_string(), + imported_hooks: Vec::new(), functions: vec![Function::TestFunction(TestFunction { name: "test_something".to_string(), expect_fail: false, @@ -950,6 +952,7 @@ mod compare_trees_test { functions: vec![], modules: vec![Module { name: "correct_name".to_string(), + imported_hooks: Vec::new(), functions: vec![Function::TestFunction(TestFunction { name: "test_something".to_string(), expect_fail: false, @@ -974,6 +977,7 @@ mod compare_trees_test { modules: vec![ Module { name: "module_b".to_string(), + imported_hooks: Vec::new(), functions: vec![Function::TestFunction(TestFunction { name: "test_b".to_string(), expect_fail: false, @@ -983,6 +987,7 @@ mod compare_trees_test { }, Module { name: "module_a".to_string(), + imported_hooks: Vec::new(), functions: vec![Function::TestFunction(TestFunction { name: "test_a".to_string(), expect_fail: false, @@ -997,6 +1002,7 @@ mod compare_trees_test { modules: vec![ Module { name: "module_a".to_string(), + imported_hooks: Vec::new(), functions: vec![Function::TestFunction(TestFunction { name: "test_a".to_string(), expect_fail: false, @@ -1006,6 +1012,7 @@ mod compare_trees_test { }, Module { name: "module_b".to_string(), + imported_hooks: Vec::new(), functions: vec![Function::TestFunction(TestFunction { name: "test_b".to_string(), expect_fail: false, @@ -1040,9 +1047,14 @@ mod compare_trees_test { let actual = Root { functions: vec![], modules: vec![ - Module { name: "module_a".to_string(), functions: vec![] }, + Module { + name: "module_a".to_string(), + imported_hooks: Vec::new(), + functions: vec![], + }, Module { name: "module_b".to_string(), + imported_hooks: Vec::new(), functions: vec![Function::TestFunction(TestFunction { name: "test_foo".to_string(), expect_fail: false, @@ -1057,6 +1069,7 @@ mod compare_trees_test { modules: vec![ Module { name: "module_a".to_string(), + imported_hooks: Vec::new(), functions: vec![Function::TestFunction(TestFunction { name: "test_foo".to_string(), expect_fail: false, @@ -1064,7 +1077,11 @@ mod compare_trees_test { actions: vec![], })], }, - Module { name: "module_b".to_string(), functions: vec![] }, + Module { + name: "module_b".to_string(), + imported_hooks: Vec::new(), + functions: vec![], + }, ], }; let violations = diff --git a/crates/noir/src/noir/parser.rs b/crates/noir/src/noir/parser.rs index 7e190eb..98de562 100644 --- a/crates/noir/src/noir/parser.rs +++ b/crates/noir/src/noir/parser.rs @@ -62,6 +62,7 @@ impl ParsedNoirFile { self.find_functions_recursive(node, &mut functions); modules.push(Module { name: self.extract_module_name(node), + imported_hooks: Vec::new(), functions, }); } diff --git a/crates/noir/src/scaffold/scaffoldable.rs b/crates/noir/src/scaffold/scaffoldable.rs index 85880ad..276e0fb 100644 --- a/crates/noir/src/scaffold/scaffoldable.rs +++ b/crates/noir/src/scaffold/scaffoldable.rs @@ -69,6 +69,11 @@ impl Scaffoldable for Module { fn scaffold(&self, generate_setup_hooks: bool) -> String { let mut output = format!("\nmod {} {{\n", self.name); + if generate_setup_hooks { + for import in &self.imported_hooks { + output.push_str(format!(" use super::{};\n", import.name).as_str()); + } + } // indent the definitions inside the module for func in &self.functions { for line in func.scaffold(generate_setup_hooks).lines() { diff --git a/crates/noir/src/test_structure.rs b/crates/noir/src/test_structure.rs index 9c5ab5c..69d7195 100644 --- a/crates/noir/src/test_structure.rs +++ b/crates/noir/src/test_structure.rs @@ -23,15 +23,19 @@ impl Root { match forest.iter().len() { 0 => Ok(Root { functions, modules }), 1 => { + let test_functions = collect_tests(forest, &[]); + functions.extend( - collect_helpers(forest) + collect_helpers(&test_functions) .into_iter() - .map(|x| Function::SetupHook(x)), + .map(|x| Function::SetupHook(x)) + .collect::>(), ); functions.extend( - collect_tests(forest, &[]) + test_functions .into_iter() - .map(|x| Function::TestFunction(x)), + .map(|x| Function::TestFunction(x)) + .collect::>(), ); Ok(Root { functions, modules }) } @@ -57,38 +61,104 @@ impl Root { ); } - let tree_slice = std::slice::from_ref(ast); - let helpers = collect_helpers(tree_slice); + let local_tests = + collect_tests(std::slice::from_ref(ast), &[]); - for helper in &helpers { + let helpers = collect_helpers(&local_tests); + for hook in &helpers { // returns false if the key is already present - if !all_hooks.insert(helper.name.clone()) { + if !all_hooks.insert(hook.name.clone()) { // we don't care if it's repeated one or multiple times - repeated_hooks.insert(helper.name.clone()); + repeated_hooks.insert(hook.name.clone()); } } let mut local_functions = Vec::new(); local_functions.extend( - helpers.into_iter().map(|x| Function::SetupHook(x)), + helpers + .into_iter() + .map(|x| Function::SetupHook(x)) + .collect::>(), ); local_functions.extend( - collect_tests(tree_slice, &[]) + local_tests .into_iter() - .map(|x| Function::TestFunction(x)), + .map(|x| Function::TestFunction(x)) + .collect::>(), ); - modules.push(Module { name, functions: local_functions }); + modules.push(Module { + name, + functions: local_functions, + imported_hooks: Vec::new(), + }); } - modules = modules.into_iter().map(|module| Module{ - name: module.name.clone(), - functions: module.functions.into_iter().filter(|fun| !repeated_hooks.contains(&fun.name())).collect() - }).collect(); + Ok(Root { + modules: hoist_setup_hooks(modules, &repeated_hooks), + functions: repeated_hooks + .into_iter() + .map(|name| Function::SetupHook(SetupHook { name })) + .collect(), + }) + } + } + } +} - Ok(Root { modules, functions: repeated_hooks.into_iter().map(|name| Function::SetupHook(SetupHook{name})).collect() }) +fn hoist_setup_hooks( + modules: Vec, + repeated_hooks: &HashSet, +) -> Vec { + modules + .into_iter() + .map(|module| Module { + name: module.name.clone(), + imported_hooks: module + .functions + .iter() + .filter_map(|fun| { + if let Function::SetupHook(f) = fun { + if repeated_hooks.contains(&f.name) { + Some(f.clone()) + } else { + None + } + } else { + None + } + }) + .collect(), + functions: module + .functions + .into_iter() + .filter(|fun| { + if let Function::SetupHook(f) = fun { + if repeated_hooks.contains(&f.name) { + return false; + } else { + return true; + } + } else { + return true; + } + }) + .collect(), + }) + .collect() +} + +fn collect_helpers(test_functions: &Vec) -> Vec { + let mut hooks = Vec::new(); + let mut all_hooks: HashSet = HashSet::new(); + for func in test_functions { + for hook in &func.setup_hooks { + // skip repeated hooks + if all_hooks.insert(hook.name.clone()) { + hooks.push(hook.clone()); } } } + hooks } /// Used for both definition and invocation @@ -100,6 +170,7 @@ pub(crate) struct SetupHook { #[derive(PartialEq, Eq, PartialOrd, Ord, Hash, Clone, Debug)] pub(crate) struct Module { pub name: String, + pub imported_hooks: Vec, pub functions: Vec, } @@ -126,43 +197,6 @@ impl Function { } } -/// Collect all unique helper names from conditions. -fn collect_helpers(children: &[Ast]) -> Vec { - let mut helpers = HashSet::new(); - collect_helpers_recursive(children, &mut helpers); - let mut sorted: Vec = helpers.into_iter().collect(); - sorted.sort(); - sorted -} - -/// Recursively collect helper names. -fn collect_helpers_recursive( - children: &[Ast], - helpers: &mut HashSet, -) { - for child in children { - match child { - Ast::Condition(condition) => { - // only produce helpers for a branch if any of its children is also a branch, meaning - // there's a potential need to reuse them - if condition.children.iter().any(|c| match c { - Ast::Condition(_) => true, - _ => false, - }) { - helpers.insert(SetupHook { - name: to_snake_case(&condition.title), - }); - } - collect_helpers_recursive(&condition.children, helpers); - } - Ast::Root(root) => { - collect_helpers_recursive(&root.children, helpers); - } - _ => {} - } - } -} - fn collect_tests( children: &[Ast], parent_helpers: &[SetupHook], @@ -456,4 +490,376 @@ Contract::bar assert_eq!(bar_module.functions[2].name(), "test_when_c"); assert!(matches!(bar_module.functions[2], Function::TestFunction(_))); } + + /// Regression test for https://github.com/defi-wonderland/bulloak/pull/9#issuecomment-3710452952 + /// When multiple roots share the same leaf condition (a condition with only action children), + /// the shared setup hook should be hoisted to root.functions. + #[test] + fn test_hoist_shared_leaf_condition_setup() { + let tree = r" +hoisted_hook_regression::constructor_with_minter +└── when passing valid parameters + ├── it sets name + └── it sets symbol + +hoisted_hook_regression::constructor_with_initial_supply +└── when passing valid parameters + ├── it sets name + └── it sets symbol +"; + let forest = parse(tree).unwrap(); + let root = Root::new(&forest).unwrap(); + + // The shared condition should be hoisted to root.functions as a SetupHook + assert_eq!( + root.functions.len(), + 1, + "Expected 1 hoisted setup hook, found {}. Root functions: {:?}", + root.functions.len(), + root.functions + ); + assert_eq!(root.functions[0].name(), "when_passing_valid_parameters"); + assert!( + matches!(root.functions[0], Function::SetupHook(_)), + "Expected SetupHook, got {:?}", + root.functions[0] + ); + + // Verify modules are created correctly + assert_eq!(root.modules.len(), 2); + + let minter_module = &root.modules[0]; + assert_eq!(minter_module.name, "constructor_with_minter"); + assert_eq!(minter_module.functions.len(), 1); + assert_eq!( + minter_module.functions[0].name(), + "test_when_passing_valid_parameters" + ); + assert!(matches!( + minter_module.functions[0], + Function::TestFunction(_) + )); + + let supply_module = &root.modules[1]; + assert_eq!(supply_module.name, "constructor_with_initial_supply"); + assert_eq!(supply_module.functions.len(), 1); + assert_eq!( + supply_module.functions[0].name(), + "test_when_passing_valid_parameters" + ); + assert!(matches!( + supply_module.functions[0], + Function::TestFunction(_) + )); + } + + /// Related to https://github.com/defi-wonderland/bulloak/pull/9#issuecomment-3710452952 + /// check the modifier would be generated in the case of a single-root testfile + #[test] + fn test_single_root_setup_hook_generation() { + let tree = r" +hoisted_hook_regression::constructor_with_minter +└── when passing valid parameters + ├── it sets name + └── it sets symbol +"; + let forest = parse(tree).unwrap(); + let root = Root::new(&forest).unwrap(); + dbg!(&root); + + assert_eq!( + root.functions.len(), + 2, + "Expected 1 setup hook and 1 test function, found {}. Root functions: {:?}", + root.functions.len(), + root.functions + ); + assert_eq!(root.functions[0].name(), "when_passing_valid_parameters"); + assert!( + matches!(root.functions[0], Function::SetupHook(_)), + "Expected SetupHook, got {:?}", + root.functions[0] + ); + + // Verify no modules are defined + assert_eq!(root.modules.len(), 0); + + assert_eq!( + root.functions[1].name(), + "test_when_passing_valid_parameters" + ); + assert!(matches!(root.functions[1], Function::TestFunction(_))); + } +} + +#[cfg(test)] +mod tests_hoist_setup_hooks { + use super::*; + + #[test] + fn test_hoist_setup_hooks_empty_modules() { + let modules: Vec = vec![]; + let repeated_hooks: HashSet = HashSet::new(); + + let result = hoist_setup_hooks(modules, &repeated_hooks); + + assert!(result.is_empty()); + } + + #[test] + fn test_hoist_setup_hooks_empty_repeated_hooks() { + let modules = vec![Module { + name: "module_a".to_string(), + imported_hooks: Vec::new(), + functions: vec![ + Function::SetupHook(SetupHook { name: "when_a".to_string() }), + Function::TestFunction(TestFunction { + name: "test_when_a".to_string(), + expect_fail: false, + setup_hooks: vec![], + actions: vec![], + }), + ], + }]; + let repeated_hooks: HashSet = HashSet::new(); + + let result = hoist_setup_hooks(modules, &repeated_hooks); + + assert_eq!(result.len(), 1); + assert_eq!(result[0].name, "module_a"); + assert!(result[0].imported_hooks.is_empty()); + assert_eq!(result[0].functions.len(), 2); + assert_eq!(result[0].functions[0].name(), "when_a"); + assert_eq!(result[0].functions[1].name(), "test_when_a"); + } + + #[test] + fn test_hoist_setup_hooks_single_module_no_repeated_hooks() { + let modules = vec![Module { + name: "my_module".to_string(), + imported_hooks: Vec::new(), + functions: vec![ + Function::SetupHook(SetupHook { + name: "when_something".to_string(), + }), + Function::TestFunction(TestFunction { + name: "test_when_something".to_string(), + expect_fail: false, + setup_hooks: vec![SetupHook { + name: "when_something".to_string(), + }], + actions: vec!["it does something".to_string()], + }), + ], + }]; + let repeated_hooks: HashSet = HashSet::new(); + + let result = hoist_setup_hooks(modules, &repeated_hooks); + + assert_eq!(result.len(), 1); + assert_eq!(result[0].name, "my_module"); + assert!(result[0].imported_hooks.is_empty()); + assert_eq!(result[0].functions.len(), 2); + assert_eq!(result[0].functions[0].name(), "when_something"); + assert_eq!(result[0].functions[1].name(), "test_when_something"); + } + + // this borders on testing a bug, since there should be no repeated hooks + // if there is only one module + #[test] + fn test_hoist_setup_hooks_single_module_with_one_repeated_hook() { + let modules = vec![Module { + name: "my_module".to_string(), + imported_hooks: Vec::new(), + functions: vec![ + Function::SetupHook(SetupHook { + name: "when_shared".to_string(), + }), + Function::SetupHook(SetupHook { + name: "when_local".to_string(), + }), + Function::TestFunction(TestFunction { + name: "test_when_shared".to_string(), + expect_fail: false, + setup_hooks: vec![], + actions: vec![], + }), + ], + }]; + let mut repeated_hooks: HashSet = HashSet::new(); + repeated_hooks.insert("when_shared".to_string()); + + let result = hoist_setup_hooks(modules, &repeated_hooks); + + assert_eq!(result.len(), 1); + assert_eq!(result[0].name, "my_module"); + + // The repeated hook should be in imported_hooks + assert_eq!(result[0].imported_hooks.len(), 1); + assert_eq!(result[0].imported_hooks[0].name, "when_shared"); + + // The repeated hook should be removed from functions, but local hook and test remain + assert_eq!(result[0].functions.len(), 2); + assert_eq!(result[0].functions[0].name(), "when_local"); + assert_eq!(result[0].functions[1].name(), "test_when_shared"); + } + + #[test] + fn test_hoist_setup_hooks_multiple_modules_with_shared_hooks() { + let modules = vec![ + Module { + name: "foo".to_string(), + imported_hooks: Vec::new(), + functions: vec![ + Function::SetupHook(SetupHook { + name: "when_shared".to_string(), + }), + Function::TestFunction(TestFunction { + name: "test_foo".to_string(), + expect_fail: false, + setup_hooks: vec![], + actions: vec![], + }), + ], + }, + Module { + name: "bar".to_string(), + imported_hooks: Vec::new(), + functions: vec![ + Function::SetupHook(SetupHook { + name: "when_shared".to_string(), + }), + Function::SetupHook(SetupHook { + name: "when_bar_only".to_string(), + }), + Function::TestFunction(TestFunction { + name: "test_bar".to_string(), + expect_fail: false, + setup_hooks: vec![], + actions: vec![], + }), + ], + }, + ]; + let mut repeated_hooks: HashSet = HashSet::new(); + repeated_hooks.insert("when_shared".to_string()); + + let result = hoist_setup_hooks(modules, &repeated_hooks); + + assert_eq!(result.len(), 2); + + // First module (foo) + assert_eq!(result[0].name, "foo"); + assert_eq!(result[0].imported_hooks.len(), 1); + assert_eq!(result[0].imported_hooks[0].name, "when_shared"); + assert_eq!(result[0].functions.len(), 1); + assert_eq!(result[0].functions[0].name(), "test_foo"); + + // Second module (bar) + assert_eq!(result[1].name, "bar"); + assert_eq!(result[1].imported_hooks.len(), 1); + assert_eq!(result[1].imported_hooks[0].name, "when_shared"); + assert_eq!(result[1].functions.len(), 2); + assert_eq!(result[1].functions[0].name(), "when_bar_only"); + assert_eq!(result[1].functions[1].name(), "test_bar"); + } + + #[test] + fn test_hoist_setup_hooks_module_with_only_test_functions() { + let modules = vec![Module { + name: "tests_only".to_string(), + imported_hooks: Vec::new(), + functions: vec![ + Function::TestFunction(TestFunction { + name: "test_one".to_string(), + expect_fail: false, + setup_hooks: vec![], + actions: vec!["action one".to_string()], + }), + Function::TestFunction(TestFunction { + name: "test_two".to_string(), + expect_fail: true, + setup_hooks: vec![], + actions: vec!["action two".to_string()], + }), + ], + }]; + let mut repeated_hooks: HashSet = HashSet::new(); + repeated_hooks.insert("when_something".to_string()); // Not present in module + + let result = hoist_setup_hooks(modules, &repeated_hooks); + + assert_eq!(result.len(), 1); + assert_eq!(result[0].name, "tests_only"); + assert!(result[0].imported_hooks.is_empty()); + assert_eq!(result[0].functions.len(), 2); + assert_eq!(result[0].functions[0].name(), "test_one"); + assert_eq!(result[0].functions[1].name(), "test_two"); + } + + #[test] + fn test_hoist_setup_hooks_test_function_with_repeated_hook_name_not_removed( + ) { + // Edge case: a TestFunction has the same name as a repeated hook + // The function should NOT be removed because only SetupHooks are hoisted + let modules = vec![ + Module { + name: "edge".to_string(), + imported_hooks: Vec::new(), + functions: vec![ + Function::SetupHook(SetupHook { + name: "when_shared".to_string(), + }), + Function::TestFunction(TestFunction { + name: "when_shared".to_string(), // Same name as hook + expect_fail: false, + setup_hooks: vec![], + actions: vec![], + }), + ], + }, + Module { + name: "case".to_string(), + imported_hooks: Vec::new(), + functions: vec![ + Function::SetupHook(SetupHook { + name: "when_shared".to_string(), + }), + Function::TestFunction(TestFunction { + name: "when_shared".to_string(), // Same name as hook + expect_fail: false, + setup_hooks: vec![], + actions: vec![], + }), + ], + }, + ]; + let mut repeated_hooks: HashSet = HashSet::new(); + repeated_hooks.insert("when_shared".to_string()); + + let result = hoist_setup_hooks(modules, &repeated_hooks); + + assert_eq!(result.len(), 2); + assert_eq!(result[0].name, "edge"); + + // The SetupHook should be in imported_hooks + assert_eq!(result[0].imported_hooks.len(), 1); + assert_eq!(result[0].imported_hooks[0].name, "when_shared"); + + // the setup hook was removed but the test function remains in the module + assert_eq!(result[0].functions.len(), 1); + assert!(matches!(result[0].functions[0], Function::TestFunction(_))); + assert_eq!(result[0].functions[0].name(), "when_shared"); + + assert_eq!(result[1].name, "case"); + + // The SetupHook should be in imported_hooks + assert_eq!(result[1].imported_hooks.len(), 1); + assert_eq!(result[1].imported_hooks[0].name, "when_shared"); + + // the setup hook was removed but the test function remains in the module + assert_eq!(result[0].functions.len(), 1); + assert!(matches!(result[0].functions[0], Function::TestFunction(_))); + assert_eq!(result[0].functions[0].name(), "when_shared"); + } }