diff --git a/docs/developers-guide.md b/docs/developers-guide.md index a646f5fe7..7adda4805 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -1295,6 +1295,20 @@ Netsuke uses a mixed strategy: adjacent to the code under test, included via `#[cfg(test)] #[path = "..."] mod ...;` declarations. +Cargo discovers integration-test binaries only from Rust files directly below +`tests/`. Module trees rooted at `tests/*/mod.rs` must therefore be declared by +at least one top-level integration-test source, either with `mod name;` or an +explicit `#[path = "name/mod.rs"]` attribute. The narrowly scoped discovery +helpers in `tests/integration_test_wiring_tests.rs` own this structural check; +reuse them only for the immediate integration-test tree rather than as a +general Rust source parser. + +The `std_filter_tests` target owns its command fixtures within +`tests/std_filter_tests/command_filters/`. `CommandFixture` provides the +capability-scoped temporary workspace, while `ShellCase` groups each +parameterized shell scenario. Keep both private to that test feature; shared +integration-test facilities belong in `test_support` instead. + The Dependabot integration tests parse the checked-in configuration and verify that repository dependency manifests remain covered as the tree changes. They assert the Cargo and GitHub Actions update policies, the configured schedules, @@ -1930,9 +1944,11 @@ it forces a split layout with its own private `CARGO_TARGET_DIR` and `CARGO_BUILD_BUILD_DIR` roots, confirms the collected dependency directories span the split, and then compiles a fixture against them. The roots are private to the test rather than the ambient target directory because the -`#[once]` `test_support_rlib` fixture builds concurrently in the other test; -sharing a target directory between the two races on the uplifted rlibs and -fails with version-skew errors (`E0460`). +`#[once]` `test_support_rlib` fixture builds concurrently for +`stub_env_default_does_not_compile` and +`stub_env_builders_compile_under_the_same_harness`. Sharing a target +directory would make `harness_compiles_under_a_split_build_dir` race that +build on the uplifted rlibs and fail with version-skew errors (`E0460`). ### Manifest `env()` reader diff --git a/src/stdlib/command/error.rs b/src/stdlib/command/error.rs index d4d94e595..83350e0e4 100644 --- a/src/stdlib/command/error.rs +++ b/src/stdlib/command/error.rs @@ -231,6 +231,14 @@ mod tests { use super::*; use crate::localization::{self, keys}; + /// Assert `failure` renders for `command` as `expected`; callers keep their + /// own failure construction and localized message assembly. + fn assert_command_error_message(failure: CommandFailure, command: &str, expected: &str) { + let err = command_error(failure, "template.html", command); + assert_eq!(err.kind(), ErrorKind::InvalidOperation); + assert_eq!(err.to_string(), format!("invalid operation: {expected}")); + } + /// `category()` supplies the bounded `error_category` label on the command /// execution span and counter, so every variant must map to a stable, /// low-cardinality string. Constructing each variant here also means a new @@ -277,18 +285,16 @@ mod tests { #[test] fn spawn_errors_include_source() { - let err = command_error( - CommandFailure::Spawn(io::Error::new(io::ErrorKind::NotFound, "command not found")), - "template.html", - "missing_cmd", - ); - assert_eq!(err.kind(), ErrorKind::InvalidOperation); let location = CommandLocation::new("template.html", "missing_cmd").describe(); let expected = localization::message(keys::COMMAND_SPAWN_FAILED) .with_arg("location", location) .with_arg("details", "command not found") .to_string(); - assert_eq!(err.to_string(), format!("invalid operation: {expected}")); + assert_command_error_message( + CommandFailure::Spawn(io::Error::new(io::ErrorKind::NotFound, "command not found")), + "missing_cmd", + &expected, + ); } #[test] @@ -378,17 +384,15 @@ mod tests { #[test] fn timeout_errors_report_duration() { - let err = command_error( - CommandFailure::Timeout(Duration::from_secs(3)), - "template.html", - "sleep", - ); - assert_eq!(err.kind(), ErrorKind::InvalidOperation); let location = CommandLocation::new("template.html", "sleep").describe(); let expected = localization::message(keys::COMMAND_TIMEOUT) .with_arg("location", location) .with_arg("seconds", 3.0) .to_string(); - assert_eq!(err.to_string(), format!("invalid operation: {expected}")); + assert_command_error_message( + CommandFailure::Timeout(Duration::from_secs(3)), + "sleep", + &expected, + ); } } diff --git a/tests/integration_test_wiring_tests.rs b/tests/integration_test_wiring_tests.rs new file mode 100644 index 000000000..94239794f --- /dev/null +++ b/tests/integration_test_wiring_tests.rs @@ -0,0 +1,419 @@ +//! Contract tests that keep integration-test module trees wired to Cargo targets. + +use std::{collections::BTreeSet, process::Command}; + +use anyhow::{Context, Result, ensure}; +use camino::Utf8Path; +use cap_std::{ambient_authority, fs_utf8::Dir}; +use proptest::prelude::*; +use rstest::rstest; + +/// The names of the `tests/*.rs` files Cargo should turn into test targets. +fn top_level_test_sources(tests_dir: &Dir) -> Result> { + let mut names = BTreeSet::new(); + for entry_result in tests_dir + .read_dir(".") + .context("read integration-test directory")? + { + let directory_entry = entry_result.context("read integration-test directory entry")?; + let name = directory_entry + .file_name() + .context("read integration-test entry name")?; + if Utf8Path::new(&name).extension() == Some("rs") { + names.insert(name); + } + } + Ok(names) +} + +/// The `tests/*.rs` sources Cargo actually reports as integration-test targets. +fn cargo_discovered_test_targets( + manifest_path: &Utf8Path, + tests_dir: &Utf8Path, +) -> Result> { + // `env!("CARGO")` is the cargo that built this test, so the metadata comes + // from the same toolchain rather than whichever cargo is first on PATH. + let output = Command::new(env!("CARGO")) + .args(["metadata", "--no-deps", "--format-version", "1"]) + .arg("--manifest-path") + .arg(manifest_path.as_str()) + .output() + .context("run cargo metadata")?; + ensure!( + output.status.success(), + "cargo metadata failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + + let metadata: serde_json::Value = + serde_json::from_slice(&output.stdout).context("parse cargo metadata output")?; + let package = metadata + .get("packages") + .and_then(serde_json::Value::as_array) + .context("cargo metadata should list packages")? + .iter() + .find(|package| { + package + .get("manifest_path") + .and_then(serde_json::Value::as_str) + == Some(manifest_path.as_str()) + }) + .context("cargo metadata should list this package")?; + + let discovered = package + .get("targets") + .and_then(serde_json::Value::as_array) + .context("package metadata should list targets")? + .iter() + .filter(|target| { + target + .get("kind") + .and_then(serde_json::Value::as_array) + .is_some_and(|kinds| kinds.iter().any(|kind| kind == "test")) + }) + .filter_map(|target| target.get("src_path").and_then(serde_json::Value::as_str)) + .filter_map(|path| Utf8Path::new(path).strip_prefix(tests_dir).ok()) + .map(ToString::to_string) + .collect(); + Ok(discovered) +} + +fn integration_test_sources(tests_dir: &Dir) -> Result> { + let mut sources = Vec::new(); + for entry_result in tests_dir + .read_dir(".") + .context("read integration-test directory")? + { + let directory_entry = entry_result.context("read integration-test directory entry")?; + let name = directory_entry + .file_name() + .context("read integration-test entry name")?; + if Utf8Path::new(&name).extension() == Some("rs") { + sources.push( + tests_dir + .read_to_string(&name) + .with_context(|| format!("read integration-test source {name}"))?, + ); + } + } + Ok(sources) +} + +fn orphaned_module_trees(tests_dir: &Dir, sources: &[String]) -> Result> { + let mut orphaned = Vec::new(); + for entry_result in tests_dir + .read_dir(".") + .context("read integration-test directory")? + { + let directory_entry = entry_result.context("read integration-test directory entry")?; + if !directory_entry + .file_type() + .context("read integration-test entry type")? + .is_dir() + { + continue; + } + + let name = directory_entry + .file_name() + .context("read integration-test directory name")?; + if !tests_dir.try_exists(format!("{name}/mod.rs"))? { + continue; + } + + // A declaration wires the tree whatever visibility it carries. Matching + // whole trimmed lines keeps commented-out declarations excluded, which a + // prefix or substring test would not. + let conventional_declarations = [ + format!("mod {name};"), + format!("pub mod {name};"), + format!("pub(crate) mod {name};"), + ]; + let explicit_path_attribute = format!("#[path = \"{name}/mod.rs\"]"); + let is_wired = sources.iter().any(|source| { + source.lines().any(|line| { + let trimmed_line = line.trim(); + trimmed_line == explicit_path_attribute + || conventional_declarations + .iter() + .any(|declaration| trimmed_line == declaration) + }) + }); + if !is_wired { + orphaned.push(name); + } + } + orphaned.sort_unstable(); + Ok(orphaned) +} + +#[test] +fn module_trees_are_wired_to_cargo_test_targets() -> Result<()> { + let tests_path = Utf8Path::new(env!("CARGO_MANIFEST_DIR")).join("tests"); + let tests_dir = Dir::open_ambient_dir(&tests_path, ambient_authority()) + .context("open integration-test directory")?; + let sources = integration_test_sources(&tests_dir)?; + let orphaned = orphaned_module_trees(&tests_dir, &sources)?; + + ensure!( + orphaned.is_empty(), + "tests/*/mod.rs trees must be declared by a Cargo-discovered tests/*.rs target; orphaned: {}", + orphaned.join(", ") + ); + Ok(()) +} + +/// Cargo's target list must account for every top-level `tests/*.rs` source. +/// +/// The sibling guards read declaration text out of the sources and so can only +/// police what the sources say. This one asks Cargo what it actually resolved, +/// covering the manifest instead: a partial `[[test]]` list under +/// `autotests = false`, or a target whose `path` points outside `tests/`, both +/// leave the sources untouched and are invisible to any amount of scanning. +/// +/// It does *not* catch #520. A tree carrying its own `mod.rs` and no top-level +/// source contributes to neither set, so the equality still holds; +/// `module_trees_are_wired_to_cargo_test_targets` owns that direction. The two +/// together close the loop — that one from the tree upwards, this one from +/// Cargo's manifest view downwards. +/// +/// The assertion is a set equality both ways. A missing entry is a source Cargo +/// never compiles; an extra one is a target resolving outside `tests/`, which +/// would put the sibling guards' path assumptions wrong. +/// +/// `cargo metadata --no-deps` neither builds nor uses the network; the call +/// costs roughly 20ms. +#[test] +fn cargo_discovers_every_top_level_integration_test_source() -> Result<()> { + let manifest_dir = Utf8Path::new(env!("CARGO_MANIFEST_DIR")); + let manifest_path = manifest_dir.join("Cargo.toml"); + let tests_path = manifest_dir.join("tests"); + let tests_dir = Dir::open_ambient_dir(&tests_path, ambient_authority()) + .context("open integration-test directory")?; + + let on_disk = top_level_test_sources(&tests_dir)?; + let discovered = cargo_discovered_test_targets(&manifest_path, &tests_path)?; + + ensure!( + discovered == on_disk, + "Cargo's integration-test targets must match tests/*.rs exactly; \ + on disk but not built by Cargo: {:?}; built by Cargo but not a \ + top-level tests/*.rs: {:?}", + on_disk.difference(&discovered).collect::>(), + discovered.difference(&on_disk).collect::>() + ); + Ok(()) +} + +#[test] +fn orphaned_and_commented_module_trees_are_reported() -> Result<()> { + let temp = tempfile::tempdir().context("create integration-test fixture")?; + let tests_path = Utf8Path::from_path(temp.path()).context("fixture path is not valid UTF-8")?; + let tests_dir = Dir::open_ambient_dir(tests_path, ambient_authority()) + .context("open integration-test fixture")?; + tests_dir + .create_dir("wired") + .context("create wired module tree")?; + tests_dir + .write("wired/mod.rs", "//! Wired fixture.\n") + .context("write wired module root")?; + tests_dir + .create_dir("orphaned") + .context("create orphaned module tree")?; + tests_dir + .write("orphaned/mod.rs", "//! Orphaned fixture.\n") + .context("write orphaned module root")?; + tests_dir + .create_dir("commented") + .context("create commented module tree")?; + tests_dir + .write("commented/mod.rs", "//! Commented fixture.\n") + .context("write commented module root")?; + tests_dir + .write("wired_tests.rs", "mod wired;\n") + .context("write wired integration-test target")?; + tests_dir + .write( + "commented_tests.rs", + "// #[path = \"commented/mod.rs\"]\n// mod commented;\n", + ) + .context("write commented integration-test target")?; + + let sources = integration_test_sources(&tests_dir)?; + let orphaned = orphaned_module_trees(&tests_dir, &sources)?; + + ensure!( + orphaned == ["commented", "orphaned"], + "expected commented and orphaned fixtures to be reported, got {orphaned:?}" + ); + Ok(()) +} + +/// A visibility qualifier does not change whether a declaration wires a tree. +/// +/// The commented cases pin the other half of the rule: the guard matches whole +/// trimmed lines, so prefixing a declaration with `//` must still leave the +/// tree orphaned no matter which visibility it carries. +#[rstest] +#[case::plain("mod wired;", true)] +#[case::public("pub mod wired;", true)] +#[case::crate_visible("pub(crate) mod wired;", true)] +#[case::path_attribute("#[path = \"wired/mod.rs\"]\nmod wired_alias;", true)] +#[case::commented_plain("// mod wired;", false)] +#[case::commented_public("// pub mod wired;", false)] +#[case::commented_crate_visible("// pub(crate) mod wired;", false)] +fn visibility_qualified_declarations_wire_module_trees( + #[case] declaration: &str, + #[case] expect_wired: bool, +) -> Result<()> { + let temp = tempfile::tempdir().context("create visibility fixture")?; + let tests_path = Utf8Path::from_path(temp.path()).context("fixture path is not valid UTF-8")?; + let tests_dir = Dir::open_ambient_dir(tests_path, ambient_authority()) + .context("open visibility fixture")?; + tests_dir + .create_dir("wired") + .context("create wired module tree")?; + tests_dir + .write("wired/mod.rs", "//! Wired fixture.\n") + .context("write wired module root")?; + tests_dir + .write("wired_tests.rs", format!("{declaration}\n")) + .context("write integration-test target")?; + + let sources = integration_test_sources(&tests_dir)?; + let orphaned = orphaned_module_trees(&tests_dir, &sources)?; + + ensure!( + orphaned.is_empty() == expect_wired, + "declaration {declaration:?} should {}wire the tree, got orphaned {orphaned:?}", + if expect_wired { "" } else { "not " } + ); + Ok(()) +} + +/// How a generated module tree is declared by the generated test target. +/// +/// `orphaned_module_trees` claims a universal property: a tree is excluded from +/// the orphan list exactly when some source declares it with an active `mod` +/// item or `#[path]` attribute. Enumerating the declaration forms lets the +/// property test below assert both halves of that biconditional. +#[derive(Debug, Clone, Copy)] +enum Declaration { + Conventional, + PubConventional, + PubCrateConventional, + PathAttribute, + CommentedConventional, + CommentedPath, + Absent, +} + +impl Declaration { + /// Whether this form should keep the tree out of the orphan list. + const fn wires(self) -> bool { + matches!( + self, + Self::Conventional + | Self::PubConventional + | Self::PubCrateConventional + | Self::PathAttribute + ) + } + + /// Render this declaration for `name`, indented by `indent`. + /// + /// The indentation exercises the guard's line trimming; the `_tree` alias + /// on the `#[path]` form mirrors how a real target names a relocated + /// module. Generated names always end in `_`, so that alias can + /// never collide with another generated tree. + fn render(self, name: &str, indent: &str) -> String { + match self { + Self::Conventional => format!("{indent}mod {name};\n"), + Self::PubConventional => format!("{indent}pub mod {name};\n"), + Self::PubCrateConventional => format!("{indent}pub(crate) mod {name};\n"), + Self::PathAttribute => { + format!("{indent}#[path = \"{name}/mod.rs\"]\n{indent}mod {name}_tree;\n") + } + Self::CommentedConventional => format!("{indent}// mod {name};\n"), + Self::CommentedPath => { + format!("{indent}// #[path = \"{name}/mod.rs\"]\n{indent}// mod {name};\n") + } + Self::Absent => String::new(), + } + } +} + +type ModuleTreeSpec = (String, Declaration, String); + +fn module_tree_specs() -> impl Strategy> { + let declaration_strategy = prop_oneof![ + Just(Declaration::Conventional), + Just(Declaration::PubConventional), + Just(Declaration::PubCrateConventional), + Just(Declaration::PathAttribute), + Just(Declaration::CommentedConventional), + Just(Declaration::CommentedPath), + Just(Declaration::Absent), + ]; + proptest::collection::vec( + ("[a-z][a-z0-9_]{0,6}", declaration_strategy, 0usize..4), + 1..6, + ) + .prop_map(|specs| { + specs + .into_iter() + .enumerate() + .map(|(index, (name, declaration, indent))| { + // Suffix the index so generated names stay distinct even + // when the string strategy repeats a value. + (format!("{name}_{index}"), declaration, " ".repeat(indent)) + }) + .collect() + }) +} + +/// Materialize `specs` as a tests directory and return `(expected, actual)`. +fn run_wiring_scenario(specs: &[ModuleTreeSpec]) -> Result<(Vec, Vec)> { + let temp = tempfile::tempdir().context("create generated wiring fixture")?; + let tests_path = Utf8Path::from_path(temp.path()).context("fixture path is not valid UTF-8")?; + let tests_dir = + Dir::open_ambient_dir(tests_path, ambient_authority()).context("open generated fixture")?; + + let mut source = String::from("//! Generated wiring fixture.\n"); + let mut expected = Vec::new(); + for (name, declaration, indent) in specs { + tests_dir + .create_dir(name) + .with_context(|| format!("create generated module tree {name}"))?; + tests_dir + .write(format!("{name}/mod.rs"), "//! Generated fixture.\n") + .with_context(|| format!("write generated module root {name}"))?; + source.push_str(&declaration.render(name, indent)); + if !declaration.wires() { + expected.push(name.clone()); + } + } + tests_dir + .write("generated_tests.rs", source) + .context("write generated integration-test target")?; + expected.sort_unstable(); + + let sources = integration_test_sources(&tests_dir)?; + let orphaned = orphaned_module_trees(&tests_dir, &sources)?; + Ok((expected, orphaned)) +} + +proptest! { + /// Only genuinely wired trees are excluded from the orphan list. + /// + /// The three handwritten fixtures above pin one example of each outcome; + /// this covers arbitrary tree names against every declaration form, so a + /// guard that matched substrings rather than whole trimmed lines, or that + /// mistook a commented declaration for an active one, would be caught. + #[test] + fn only_unwired_module_trees_are_reported(specs in module_tree_specs()) { + let (expected, orphaned) = run_wiring_scenario(&specs) + .map_err(|error| TestCaseError::fail(error.to_string()))?; + prop_assert_eq!(orphaned, expected); + } +} diff --git a/tests/std_filter_tests/mod.rs b/tests/std_filter_tests.rs similarity index 100% rename from tests/std_filter_tests/mod.rs rename to tests/std_filter_tests.rs index d3e0789a7..1236f590c 100644 --- a/tests/std_filter_tests/mod.rs +++ b/tests/std_filter_tests.rs @@ -12,9 +12,9 @@ mod io_filters; mod network_functions; #[path = "std_filter_tests/path_filters.rs"] mod path_filters; +#[path = "std_filter_tests/support.rs"] +mod support; #[path = "std_filter_tests/which_filter_common.rs"] mod which_filter_common; #[path = "std_filter_tests/which_filter_tests.rs"] mod which_filter_tests; -#[path = "std_filter_tests/support.rs"] -mod support; diff --git a/tests/std_filter_tests/collection_filters.rs b/tests/std_filter_tests/collection_filters.rs index 52e9d3d9e..fe50b0374 100644 --- a/tests/std_filter_tests/collection_filters.rs +++ b/tests/std_filter_tests/collection_filters.rs @@ -4,10 +4,11 @@ //! These tests exercise the filters end-to-end through a configured template //! environment to ensure we keep parity between unit expectations and rendered //! output, especially across error handling scenarios. -use anyhow::{bail, ensure, Context, Result}; -use minijinja::{context, value::Value, ErrorKind}; +use anyhow::{Context, Result, bail, ensure}; +use minijinja::{ErrorKind, context, value::Value}; use rstest::rstest; use serde::Serialize; +use test_support::fluent::normalize_fluent_isolates; use super::support::fallible; @@ -15,9 +16,7 @@ use super::support::fallible; fn uniq_removes_duplicate_strings() -> Result<()> { let mut env = fallible::stdlib_env()?; fallible::register_template(&mut env, "uniq", "{{ values | uniq | join(',') }}")?; - let template = env - .get_template("uniq") - .context("fetch template 'uniq'")?; + let template = env.get_template("uniq").context("fetch template 'uniq'")?; let output = template .render(context!(values => vec!["a", "a", "b", "b", "c"])) .context("render template 'uniq'")?; @@ -68,9 +67,7 @@ fn flatten_flattens_deeply_nested_lists() -> Result<()> { fn flatten_errors_on_scalar_items() -> Result<()> { let env = fallible::stdlib_env()?; let err = match env.render_str("{{ [[1], 2] | flatten }}", context! {}) { - Ok(output) => bail!( - "expected flatten to reject scalar items but rendered {output}" - ), + Ok(output) => bail!("expected flatten to reject scalar items but rendered {output}"), Err(err) => err, }; ensure!( @@ -79,7 +76,7 @@ fn flatten_errors_on_scalar_items() -> Result<()> { err.kind() ); ensure!( - err.to_string().contains("flatten expected sequence items"), + normalize_fluent_isolates(&err.to_string()).contains("Flatten expected sequence items"), "error should describe the invalid item: {err}" ); Ok(()) @@ -128,7 +125,10 @@ fn group_by_reads_mapping_entries() -> Result<()> { let output = env .render_str(template, context!(values => values)) .context("render group_by on mapping items")?; - ensure!(output == "2", "expected two 'tool' items but rendered {output}"); + ensure!( + output == "2", + "expected two 'tool' items but rendered {output}" + ); Ok(()) } @@ -140,7 +140,10 @@ fn group_by_preserves_insertion_order() -> Result<()> { let output = env .render_str(template, context!(values => values)) .context("render group_by preserves order")?; - ensure!(output == "1,2", "expected '1,2' ordering but rendered {output}"); + ensure!( + output == "1,2", + "expected '1,2' ordering but rendered {output}" + ); Ok(()) } @@ -167,7 +170,10 @@ fn group_by_supports_non_string_keys() -> Result<()> { let output = env .render_str(template, context!(values => values)) .context("render group_by with non-string keys")?; - ensure!(output == "2", "expected two entries with key 1 but rendered {output}"); + ensure!( + output == "2", + "expected two entries with key 1 but rendered {output}" + ); Ok(()) } @@ -190,9 +196,7 @@ fn group_by_errors_for_invalid_attributes( context!(values => vec![Item { class: "a", name: "alpha" }]), ); let err = match result { - Ok(output) => bail!( - "expected group_by to fail ({description}), but rendered {output}" - ), + Ok(output) => bail!("expected group_by to fail ({description}), but rendered {output}"), Err(err) => err, }; ensure!( diff --git a/tests/std_filter_tests/command_filters/grep_filter_tests.rs b/tests/std_filter_tests/command_filters/grep_filter_tests.rs index dd0fd2533..ba8c9fc03 100644 --- a/tests/std_filter_tests/command_filters/grep_filter_tests.rs +++ b/tests/std_filter_tests/command_filters/grep_filter_tests.rs @@ -1,19 +1,25 @@ //! Grep filter behaviour tests. -use anyhow::{bail, ensure, Context, Result}; -use minijinja::{context, ErrorKind}; +use anyhow::{Context, Result, bail, ensure}; +use cap_std::{ambient_authority, fs_utf8::Dir}; +use minijinja::{ErrorKind, context}; use rstest::rstest; -use std::fs; +use test_support::fluent::normalize_fluent_isolates; +use test_support::fs; use super::{StdlibConfig, fallible, streaming_match_payload}; #[cfg(not(windows))] #[rstest] fn grep_filter_streams_to_tempfiles() -> Result<()> { - let config = StdlibConfig::from_current_dir()? + let (_temp, root) = fallible::filter_workspace()?; + let workspace = Dir::open_ambient_dir(&root, ambient_authority()) + .context("open grep streaming workspace")?; + let config = StdlibConfig::new(workspace)? + .with_workspace_root_path(&root)? .with_command_max_output_bytes(512)? .with_command_max_stream_bytes(200_000)?; - let (mut env, mut state) = fallible::stdlib_env_with_config(config)?; + let (mut env, state) = fallible::stdlib_env_with_config(config)?; state.reset_impure(); fallible::register_template( &mut env, @@ -27,16 +33,19 @@ fn grep_filter_streams_to_tempfiles() -> Result<()> { let rendered = template .render(context!(text => payload.clone())) .context("render grep streaming template")?; - ensure!(state.is_impure(), "grep streaming should mark template impure"); + ensure!( + state.is_impure(), + "grep streaming should mark template impure" + ); let path = camino::Utf8Path::new(rendered.as_str()); - let metadata = fs::metadata(path.as_std_path()) - .with_context(|| format!("stat streamed grep output {}", path))?; + let output_len = + fs::file_len(path).with_context(|| format!("stat streamed grep output {path}"))?; ensure!( - metadata.len() >= payload.len() as u64, + output_len >= payload.len() as u64, "streamed grep output should retain payload size" ); - let contents = fs::read_to_string(path.as_std_path()) - .with_context(|| format!("read streamed grep output {}", path))?; + let contents = + fs::read_to_string(path).with_context(|| format!("read streamed grep output {path}"))?; ensure!( contents == payload, "streamed grep file should contain the helper payload" @@ -47,16 +56,16 @@ fn grep_filter_streams_to_tempfiles() -> Result<()> { #[cfg(not(windows))] #[rstest] fn grep_filter_enforces_output_limit() -> Result<()> { - let config = StdlibConfig::from_current_dir()? + let (_temp, root) = fallible::filter_workspace()?; + let workspace = + Dir::open_ambient_dir(&root, ambient_authority()).context("open grep capture workspace")?; + let config = StdlibConfig::new(workspace)? + .with_workspace_root_path(&root)? .with_command_max_output_bytes(1024)?; - let (mut env, mut state) = fallible::stdlib_env_with_config(config)?; + let (mut env, state) = fallible::stdlib_env_with_config(config)?; state.reset_impure(); let long_text = "x".repeat(2_500); - fallible::register_template( - &mut env, - "grep_limit", - "{{ text | grep('x') }}", - )?; + fallible::register_template(&mut env, "grep_limit", "{{ text | grep('x') }}")?; let template = env .get_template("grep_limit") .context("fetch template 'grep_limit'")?; @@ -70,7 +79,8 @@ fn grep_filter_enforces_output_limit() -> Result<()> { err.kind() ); ensure!( - err.to_string().contains("exceeded capture stdout limit of 1024 bytes"), + normalize_fluent_isolates(&err.to_string()) + .contains("exceeded capture stdout limit of 1024 bytes"), "grep error should mention configured limit: {err}" ); ensure!(state.is_impure(), "grep limit should mark template impure"); @@ -79,16 +89,21 @@ fn grep_filter_enforces_output_limit() -> Result<()> { #[rstest] fn grep_filter_filters_lines() -> Result<()> { - let (mut env, mut state) = fallible::stdlib_env_with_state()?; + let (mut env, state) = fallible::stdlib_env_with_state()?; state.reset_impure(); - fallible::register_template(&mut env, "grep", "{{ 'alpha\\nbeta\\n' | grep('beta') | trim }}")?; - let template = env - .get_template("grep") - .context("fetch template 'grep'")?; + fallible::register_template( + &mut env, + "grep", + "{{ 'alpha\\nbeta\\n' | grep('beta') | trim }}", + )?; + let template = env.get_template("grep").context("fetch template 'grep'")?; let rendered = template .render(context! {}) .context("render grep template")?; - ensure!(rendered == "beta", "expected 'beta' but rendered {rendered}"); + ensure!( + rendered == "beta", + "expected 'beta' but rendered {rendered}" + ); ensure!(state.is_impure(), "grep should mark template impure"); Ok(()) } @@ -96,14 +111,16 @@ fn grep_filter_filters_lines() -> Result<()> { #[rstest] fn grep_filter_rejects_invalid_flags() -> Result<()> { let (mut env, _state) = fallible::stdlib_env_with_state()?; - fallible::register_template(&mut env, "grep_invalid", "{{ 'alpha' | grep('a', [1, 2, 3]) }}")?; + fallible::register_template( + &mut env, + "grep_invalid", + "{{ 'alpha' | grep('a', [1, 2, 3]) }}", + )?; let template = env .get_template("grep_invalid") .context("fetch template 'grep_invalid'")?; let err = match template.render(context! {}) { - Ok(output) => bail!( - "expected grep to reject non-string flags but rendered {output}" - ), + Ok(output) => bail!("expected grep to reject non-string flags but rendered {output}"), Err(err) => err, }; ensure!( @@ -120,7 +137,7 @@ fn grep_filter_rejects_invalid_flags() -> Result<()> { #[rstest] fn grep_filter_rejects_empty_pattern() -> Result<()> { - let (mut env, mut state) = fallible::stdlib_env_with_state()?; + let (mut env, state) = fallible::stdlib_env_with_state()?; state.reset_impure(); fallible::register_template(&mut env, "grep_empty", "{{ 'alpha' | grep('') }}")?; let template = env @@ -146,7 +163,7 @@ fn grep_filter_rejects_empty_pattern() -> Result<()> { #[cfg(not(windows))] #[rstest] fn grep_filter_handles_patterns_with_spaces() -> Result<()> { - let (mut env, mut state) = fallible::stdlib_env_with_state()?; + let (mut env, state) = fallible::stdlib_env_with_state()?; state.reset_impure(); fallible::register_template( &mut env, @@ -159,7 +176,10 @@ fn grep_filter_handles_patterns_with_spaces() -> Result<()> { let rendered = template .render(context!(text => "needs space\nother")) .context("render grep space template")?; - ensure!(rendered == "needs space", "grep should match spaced pattern"); + ensure!( + rendered == "needs space", + "grep should match spaced pattern" + ); ensure!(state.is_impure(), "grep should mark template impure"); Ok(()) } diff --git a/tests/std_filter_tests/command_filters/mod.rs b/tests/std_filter_tests/command_filters/mod.rs index fcded3a8a..79de9d242 100644 --- a/tests/std_filter_tests/command_filters/mod.rs +++ b/tests/std_filter_tests/command_filters/mod.rs @@ -1,11 +1,12 @@ //! Command filter integration tests for stdlib shell and grep filters. -use anyhow::{anyhow, bail, ensure, Context, Result}; +use anyhow::{Context, Result, anyhow, bail, ensure}; use camino::Utf8PathBuf; use cap_std::{ambient_authority, fs_utf8::Dir}; -use minijinja::{context, value::Value, Environment, ErrorKind}; +use minijinja::{Environment, ErrorKind, context, value::Value}; use rstest::rstest; use tempfile::tempdir; +use test_support::fluent::normalize_fluent_isolates; pub(super) use super::support::fallible; pub(super) use netsuke::stdlib::StdlibConfig; @@ -13,7 +14,7 @@ pub(super) use netsuke::stdlib::StdlibConfig; /// Tempdir-backed test fixture that compiles a helper binary and initialises stdlib state. /// /// This fixture manages the lifecycle of a temporary directory containing a compiled -/// test helper binary. It provides access to a configured MiniJinja environment (`env`) +/// test helper binary. It provides access to a configured `MiniJinja` environment (`env`) /// with stdlib filters registered, mutable stdlib state (`state`) for tracking impurity, /// and a quoted command string (`command`) pointing to the compiled helper executable. /// @@ -27,19 +28,25 @@ pub(super) struct CommandFixture { } impl CommandFixture { - pub(super) fn with_config( + pub(super) fn with_config( compiler: CommandCompiler, binary: &str, - config: StdlibConfig, - ) -> Result { + configure: F, + ) -> Result + where + F: FnOnce(StdlibConfig) -> Result, + { let temp = tempdir().context("create command fixture tempdir")?; let root = Utf8PathBuf::from_path_buf(temp.path().to_path_buf()) .map_err(|path| anyhow!("command fixture root is not valid UTF-8: {path:?}"))?; - let dir = Dir::open_ambient_dir(&root, ambient_authority()) + let helper_dir = Dir::open_ambient_dir(&root, ambient_authority()) .context("open command fixture directory")?; - let helper = compiler(&dir, &root, binary)?; + let helper = compiler(&helper_dir, &root, binary)?; let command = format!("\"{}\"", helper.as_str()); - let (env, mut state) = fallible::stdlib_env_with_config(config)?; + let workspace = Dir::open_ambient_dir(&root, ambient_authority()) + .context("open command fixture workspace")?; + let config = configure(StdlibConfig::new(workspace)?.with_workspace_root_path(&root)?)?; + let (env, state) = fallible::stdlib_env_with_config(config)?; state.reset_impure(); Ok(Self { _temp: temp, @@ -50,10 +57,10 @@ impl CommandFixture { } pub(super) fn new(compiler: CommandCompiler, binary: &str) -> Result { - Self::with_config(compiler, binary, StdlibConfig::from_current_dir()?) + Self::with_config(compiler, binary, Ok) } - pub(super) fn env(&mut self) -> &mut Environment<'static> { + pub(super) const fn env(&mut self) -> &mut Environment<'static> { &mut self.env } @@ -61,7 +68,7 @@ impl CommandFixture { &self.command } - pub(super) fn state(&self) -> &netsuke::stdlib::StdlibState { + pub(super) const fn state(&self) -> &netsuke::stdlib::StdlibState { &self.state } } @@ -79,9 +86,7 @@ pub(super) type CommandCompiler = fn(&Dir, &Utf8PathBuf, &str) -> Result Value, #[case] impure_message: &str, ) -> Result<()> { - let (mut env, mut state) = fallible::stdlib_env_with_state()?; + let (mut env, state) = fallible::stdlib_env_with_state()?; state.reset_impure(); fallible::register_template(&mut env, name, template_src)?; let template = env .get_template(name) .with_context(|| format!("fetch template '{name}'"))?; let err = match template.render(context_fn()) { - Ok(output) => bail!( - "expected filter to reject undefined input but rendered {output}" - ), + Ok(output) => bail!("expected filter to reject undefined input but rendered {output}"), Err(err) => err, }; ensure!( @@ -144,14 +147,14 @@ fn filters_reject_undefined_input( err.kind() ); ensure!( - err.to_string().contains("input value is undefined"), + normalize_fluent_isolates(&err.to_string()).contains("Input value is undefined"), "error should mention undefined input: {err}" ); ensure!(state.is_impure(), "{impure_message}"); Ok(()) } -mod shell_filter_tests; mod grep_filter_tests; +mod shell_filter_tests; #[cfg(windows)] mod windows_filter_tests; diff --git a/tests/std_filter_tests/command_filters/shell_filter_tests.rs b/tests/std_filter_tests/command_filters/shell_filter_tests.rs index dcf54002b..831786329 100644 --- a/tests/std_filter_tests/command_filters/shell_filter_tests.rs +++ b/tests/std_filter_tests/command_filters/shell_filter_tests.rs @@ -1,43 +1,51 @@ //! Shell filter behaviour tests. -use anyhow::{bail, ensure, Context, Result}; -use minijinja::{context, ErrorKind}; +use anyhow::{Context, Result, bail, ensure}; +use minijinja::{ErrorKind, context}; use rstest::rstest; -use std::fs; use test_support::command_helper::{ - compile_failure_helper, - compile_large_output_helper, - compile_uppercase_helper, + compile_failure_helper, compile_large_output_helper, compile_uppercase_helper, }; +use test_support::fluent::normalize_fluent_isolates; +use test_support::fs; -use super::{ - CommandCompiler, CommandFixture, ShellExpectation, StdlibConfig, fallible, -}; +use super::{CommandCompiler, CommandFixture, ShellExpectation, fallible}; + +struct ShellCase { + compiler: CommandCompiler, + binary: &'static str, + template_name: &'static str, + template_src: &'static str, + expectation: ShellExpectation, +} #[rstest] -#[case::uppercase( - compile_uppercase_helper, - "cmd_upper", - "shell_upper", - "{{ 'hello' | shell(cmd) | trim }}", - ShellExpectation::Success("HELLO") -)] +#[case::uppercase(ShellCase { + compiler: compile_uppercase_helper, + binary: "cmd_upper", + template_name: "shell_upper", + template_src: "{{ 'hello' | shell(cmd) | trim }}", + expectation: ShellExpectation::Success("HELLO"), +})] #[case::failure( - compile_failure_helper, - "cmd_fail", - "shell_fail", - "{{ 'data' | shell(cmd) }}", - ShellExpectation::Failure { - substrings: &["command", "exited"], - }, + ShellCase { + compiler: compile_failure_helper, + binary: "cmd_fail", + template_name: "shell_fail", + template_src: "{{ 'data' | shell(cmd) }}", + expectation: ShellExpectation::Failure { + substrings: &["command", "exited"], + }, + } )] -fn shell_filter_behaviour( - #[case] compiler: CommandCompiler, - #[case] binary: &'static str, - #[case] template_name: &'static str, - #[case] template_src: &'static str, - #[case] expectation: ShellExpectation, -) -> Result<()> { +fn shell_filter_behaviour(#[case] shell_case: ShellCase) -> Result<()> { + let ShellCase { + compiler, + binary, + template_name, + template_src, + expectation, + } = shell_case; let mut fixture = CommandFixture::new(compiler, binary)?; { let env = fixture.env(); @@ -55,13 +63,14 @@ fn shell_filter_behaviour( let rendered = template .render(context!(cmd => command.clone())) .context("render shell template")?; - ensure!(rendered == expected, "expected '{expected}' but rendered {rendered}"); + ensure!( + rendered == expected, + "expected '{expected}' but rendered {rendered}" + ); } ShellExpectation::Failure { substrings } => { let err = match template.render(context!(cmd => command.clone())) { - Ok(output) => bail!( - "expected shell to propagate failures but rendered {output}" - ), + Ok(output) => bail!("expected shell to propagate failures but rendered {output}"), Err(err) => err, }; ensure!( @@ -89,16 +98,14 @@ fn shell_filter_behaviour( #[cfg(unix)] #[rstest] fn shell_filter_times_out_long_commands() -> Result<()> { - let (mut env, mut state) = fallible::stdlib_env_with_state()?; + let (mut env, state) = fallible::stdlib_env_with_state()?; state.reset_impure(); fallible::register_template(&mut env, "shell_timeout", "{{ '' | shell('sleep 10') }}")?; let template = env .get_template("shell_timeout") .context("fetch template 'shell_timeout'")?; let err = match template.render(context! {}) { - Ok(output) => bail!( - "expected shell timeout but command completed with output {output}" - ), + Ok(output) => bail!("expected shell timeout but command completed with output {output}"), Err(err) => err, }; ensure!( @@ -117,7 +124,7 @@ fn shell_filter_times_out_long_commands() -> Result<()> { #[cfg(unix)] #[rstest] fn shell_filter_tolerates_commands_that_close_stdin() -> Result<()> { - let (mut env, mut state) = fallible::stdlib_env_with_state()?; + let (mut env, state) = fallible::stdlib_env_with_state()?; state.reset_impure(); fallible::register_template( &mut env, @@ -131,7 +138,10 @@ fn shell_filter_tolerates_commands_that_close_stdin() -> Result<()> { let rendered = template .render(context! {}) .context("render shell head template")?; - ensure!(rendered == "alpha", "expected 'alpha' but rendered {rendered}"); + ensure!( + rendered == "alpha", + "expected 'alpha' but rendered {rendered}" + ); ensure!( state.is_impure(), "head command should mark template impure" @@ -141,7 +151,7 @@ fn shell_filter_tolerates_commands_that_close_stdin() -> Result<()> { #[rstest] fn shell_filter_rejects_empty_command() -> Result<()> { - let (mut env, mut state) = fallible::stdlib_env_with_state()?; + let (mut env, state) = fallible::stdlib_env_with_state()?; state.reset_impure(); fallible::register_template(&mut env, "shell_empty", "{{ 'hi' | shell(' ') }}")?; let template = env @@ -157,7 +167,7 @@ fn shell_filter_rejects_empty_command() -> Result<()> { err.kind() ); ensure!( - err.to_string().contains("requires a non-empty command"), + normalize_fluent_isolates(&err.to_string()).contains("Shell command must not be empty"), "error should mention validation message: {err}" ); ensure!(state.is_impure(), "shell should still mark template impure"); @@ -166,10 +176,10 @@ fn shell_filter_rejects_empty_command() -> Result<()> { #[rstest] fn shell_filter_enforces_output_limit() -> Result<()> { - let config = StdlibConfig::from_current_dir()? - .with_command_max_output_bytes(1024)?; let mut fixture = - CommandFixture::with_config(compile_large_output_helper, "cmd_large", config)?; + CommandFixture::with_config(compile_large_output_helper, "cmd_large", |config| { + config.with_command_max_output_bytes(1024) + })?; { let env = fixture.env(); fallible::register_template(env, "shell_large", "{{ '' | shell(cmd) }}")?; @@ -190,20 +200,25 @@ fn shell_filter_enforces_output_limit() -> Result<()> { err.kind() ); ensure!( - err.to_string().contains("exceeded capture stdout limit of 1024 bytes"), + normalize_fluent_isolates(&err.to_string()) + .contains("exceeded capture stdout limit of 1024 bytes"), "limit error should mention configured budget: {err}" ); - ensure!(fixture.state().is_impure(), "limit error should mark template impure"); + ensure!( + fixture.state().is_impure(), + "limit error should mark template impure" + ); Ok(()) } #[rstest] fn shell_filter_streams_to_tempfiles() -> Result<()> { - let config = StdlibConfig::from_current_dir()? - .with_command_max_output_bytes(512)? - .with_command_max_stream_bytes(200_000)?; let mut fixture = - CommandFixture::with_config(compile_large_output_helper, "cmd_stream", config)?; + CommandFixture::with_config(compile_large_output_helper, "cmd_stream", |config| { + config + .with_command_max_output_bytes(512)? + .with_command_max_stream_bytes(200_000) + })?; { let env = fixture.env(); fallible::register_template( @@ -221,11 +236,13 @@ fn shell_filter_streams_to_tempfiles() -> Result<()> { let rendered = template .render(context!(cmd => command)) .context("render shell streaming template")?; - ensure!(fixture.state().is_impure(), "streaming should mark template impure"); + ensure!( + fixture.state().is_impure(), + "streaming should mark template impure" + ); let path = camino::Utf8Path::new(&rendered); - let data = fs::read(path.as_std_path()).with_context(|| { - format!("read streamed output from {}", path.as_str()) - })?; + let data = fs::read(path.as_std_path()) + .with_context(|| format!("read streamed output from {}", path.as_str()))?; ensure!( data.len() >= 65_000, "expected streamed output to contain command data" @@ -239,11 +256,12 @@ fn shell_filter_streams_to_tempfiles() -> Result<()> { #[rstest] fn shell_streaming_honours_size_limit() -> Result<()> { - let config = StdlibConfig::from_current_dir()? - .with_command_max_output_bytes(256)? - .with_command_max_stream_bytes(1024)?; let mut fixture = - CommandFixture::with_config(compile_large_output_helper, "cmd_stream_limit", config)?; + CommandFixture::with_config(compile_large_output_helper, "cmd_stream_limit", |config| { + config + .with_command_max_output_bytes(256)? + .with_command_max_stream_bytes(1024) + })?; { let env = fixture.env(); fallible::register_template( @@ -268,9 +286,13 @@ fn shell_streaming_honours_size_limit() -> Result<()> { err.kind() ); ensure!( - err.to_string().contains("exceeded streaming stdout limit of 1024 bytes"), + normalize_fluent_isolates(&err.to_string()) + .contains("exceeded streaming stdout limit of 1024 bytes"), "streaming limit error should mention configured budget: {err}" ); - ensure!(fixture.state().is_impure(), "streaming limit should mark template impure"); + ensure!( + fixture.state().is_impure(), + "streaming limit should mark template impure" + ); Ok(()) } diff --git a/tests/std_filter_tests/command_filters/windows_filter_tests.rs b/tests/std_filter_tests/command_filters/windows_filter_tests.rs index 4b9206d19..d1159e427 100644 --- a/tests/std_filter_tests/command_filters/windows_filter_tests.rs +++ b/tests/std_filter_tests/command_filters/windows_filter_tests.rs @@ -4,7 +4,7 @@ //! interpretation), streaming and handling of large outputs, and preservation of //! shell metacharacters when invoking external processes. -use anyhow::{anyhow, ensure, Context, Result}; +use anyhow::{Context, Result, anyhow, ensure}; use camino::Utf8PathBuf; use cap_std::{ambient_authority, fs_utf8::Dir}; use minijinja::context; @@ -73,7 +73,12 @@ impl WindowsSetupContext { dir: &'static str, compile: &'static str, ) -> Self { - Self { tempdir, root, dir, compile } + Self { + tempdir, + root, + dir, + compile, + } } } @@ -86,11 +91,12 @@ fn windows_command_setup( let root = Utf8PathBuf::from_path_buf(temp.path().to_path_buf()) .map_err(|path| anyhow!("{}: {path:?}", ctx.root))?; let dir = Dir::open_ambient_dir(&root, ambient_authority()).context(ctx.dir)?; - let helper = compile_rust_helper(&dir, &root, helper_name, helper_source) - .context(ctx.compile)?; + let helper = + compile_rust_helper(&dir, &root, helper_name, helper_source).context(ctx.compile)?; let process_env = DefaultEnv; - let path_value = prepend_path_value(process_env.os_string("PATH").as_deref(), root.as_std_path())?; + let path_value = + prepend_path_value(process_env.os_string("PATH").as_deref(), root.as_std_path())?; Ok((temp, path_value, helper)) } @@ -124,7 +130,10 @@ line2 let rendered = template .render(context! {}) .context("render windows grep template")?; - ensure!(rendered == "line2", "expected 'line2' but rendered {rendered}"); + ensure!( + rendered == "line2", + "expected 'line2' but rendered {rendered}" + ); ensure!(state.is_impure(), "grep should mark template impure"); Ok(()) } @@ -160,7 +169,10 @@ fn grep_streams_large_output_on_windows() -> Result<()> { let rendered = template .render(context!(text => payload.clone())) .context("render windows grep streaming template")?; - ensure!(state.is_impure(), "grep streaming should mark template impure"); + ensure!( + state.is_impure(), + "grep streaming should mark template impure" + ); let path = camino::Utf8Path::new(rendered.as_str()); let metadata = fs::metadata(path.as_std_path()) .with_context(|| format!("stat streamed windows grep output {}", path))?; @@ -201,7 +213,10 @@ fn shell_preserves_cmd_meta_characters() -> Result<()> { let rendered = template .render(context!(cmd => command)) .context("render shell meta template")?; - ensure!(rendered.trim() == "literal %^!", "expected literal %^! but rendered {rendered}"); + ensure!( + rendered.trim() == "literal %^!", + "expected literal %^! but rendered {rendered}" + ); ensure!( state.is_impure(), "shell filter should mark template impure" diff --git a/tests/std_filter_tests/hash_filters.rs b/tests/std_filter_tests/hash_filters.rs index 2723d7bf1..ce27a2660 100644 --- a/tests/std_filter_tests/hash_filters.rs +++ b/tests/std_filter_tests/hash_filters.rs @@ -2,10 +2,11 @@ //! //! Validates SHA-256, SHA-512, and optionally SHA-1 and MD5 (under the //! `legacy-digests` feature) for both the `hash` and `digest` filters. -use anyhow::{bail, ensure, Context, Result}; +use anyhow::{Context, Result, bail, ensure}; use cap_std::{ambient_authority, fs_utf8::Dir}; -use minijinja::{context, ErrorKind}; +use minijinja::{ErrorKind, context}; use rstest::rstest; +use test_support::fluent::normalize_fluent_isolates; use super::support::fallible; @@ -50,9 +51,8 @@ fn hash_and_digest_filters( #[case] alg: &str, #[case] expected_hash: &str, #[case] expected_digest: &str, - ) -> Result<()> { - let ( _temp, root) = fallible::filter_workspace()?; + let (_temp, root) = fallible::filter_workspace()?; let mut env = fallible::stdlib_env()?; let dir = Dir::open_ambient_dir(&root, ambient_authority()) .context("open workspace root for hashing tests")?; @@ -106,9 +106,9 @@ fn hash_filter_legacy_algorithms_disabled() -> Result<()> { .context("fetch template 'hash_sha1'")?; let result = template.render(context!(path => root.join("file").as_str())); let err = match result { - Ok(output) => bail!( - "expected hash to require legacy digests for sha1 but rendered {output}" - ), + Ok(output) => { + bail!("expected hash to require legacy digests for sha1 but rendered {output}") + } Err(err) => err, }; ensure!( @@ -117,7 +117,7 @@ fn hash_filter_legacy_algorithms_disabled() -> Result<()> { err.kind() ); ensure!( - err.to_string().contains("enable feature 'legacy-digests'"), + normalize_fluent_isolates(&err.to_string()).contains("enable feature 'legacy-digests'"), "error should mention legacy feature: {err}" ); Ok(()) @@ -135,9 +135,7 @@ fn hash_filter_rejects_unknown_algorithm() -> Result<()> { .context("fetch template 'hash_unknown'")?; let hash_result = hash_template.render(context!(path => file.as_str())); let hash_err = match hash_result { - Ok(output) => bail!( - "expected hash to reject unsupported algorithm but rendered {output}" - ), + Ok(output) => bail!("expected hash to reject unsupported algorithm but rendered {output}"), Err(err) => err, }; ensure!( @@ -146,9 +144,8 @@ fn hash_filter_rejects_unknown_algorithm() -> Result<()> { hash_err.kind() ); ensure!( - hash_err - .to_string() - .contains("unsupported hash algorithm 'whirlpool'"), + normalize_fluent_isolates(&hash_err.to_string()) + .contains("Unsupported hash algorithm 'whirlpool'"), "error should mention unsupported algorithm: {hash_err}" ); @@ -162,9 +159,9 @@ fn hash_filter_rejects_unknown_algorithm() -> Result<()> { .context("fetch template 'digest_unknown'")?; let digest_result = digest_template.render(context!(path => file.as_str())); let digest_err = match digest_result { - Ok(output) => bail!( - "expected digest to reject unsupported algorithms but rendered {output}" - ), + Ok(output) => { + bail!("expected digest to reject unsupported algorithms but rendered {output}") + } Err(err) => err, }; ensure!( @@ -173,9 +170,8 @@ fn hash_filter_rejects_unknown_algorithm() -> Result<()> { digest_err.kind() ); ensure!( - digest_err - .to_string() - .contains("unsupported hash algorithm 'whirlpool'"), + normalize_fluent_isolates(&digest_err.to_string()) + .contains("Unsupported hash algorithm 'whirlpool'"), "error should mention unsupported algorithm: {digest_err}" ); Ok(()) diff --git a/tests/std_filter_tests/io_filters.rs b/tests/std_filter_tests/io_filters.rs index 7abe82a0b..4dcef312c 100644 --- a/tests/std_filter_tests/io_filters.rs +++ b/tests/std_filter_tests/io_filters.rs @@ -1,9 +1,10 @@ //! Exercises standard library I/O filters to ensure they render file contents, //! line counts, and error paths correctly in end-to-end scenarios. -use anyhow::{bail, ensure, Context, Result}; +use anyhow::{Context, Result, bail, ensure}; use cap_std::{ambient_authority, fs_utf8::Dir}; -use minijinja::{context, ErrorKind}; +use minijinja::{ErrorKind, context}; use rstest::rstest; +use test_support::fluent::normalize_fluent_isolates; use super::support::fallible; @@ -14,7 +15,10 @@ fn contents_and_linecount_filters() -> Result<()> { let file = root.join("file"); let text = fallible::render(&mut env, "contents", "{{ path | contents }}", &file) .context("render contents filter")?; - ensure!(text == "data", "expected file contents 'data' but rendered {text}"); + ensure!( + text == "data", + "expected file contents 'data' but rendered {text}" + ); let lines_output = fallible::render( &mut env, "linecount", @@ -22,9 +26,7 @@ fn contents_and_linecount_filters() -> Result<()> { &root.join("lines.txt"), ) .context("render linecount filter")?; - let linecount: usize = lines_output - .parse() - .context("parse linecount result")?; + let linecount: usize = lines_output.parse().context("parse linecount result")?; ensure!(linecount == 3, "expected 3 lines but counted {linecount}"); Dir::open_ambient_dir(&root, ambient_authority()) @@ -42,7 +44,10 @@ fn contents_and_linecount_filters() -> Result<()> { let empty_count: usize = empty_lines .parse() .context("parse empty linecount result")?; - ensure!(empty_count == 0, "expected zero lines but counted {empty_count}"); + ensure!( + empty_count == 0, + "expected zero lines but counted {empty_count}" + ); Ok(()) } @@ -60,9 +65,9 @@ fn contents_filter_unsupported_encoding() -> Result<()> { .context("fetch template 'contents_bad_encoding'")?; let file = root.join("file"); let err = match template.render(context!(path => file.as_str())) { - Ok(output) => bail!( - "expected contents to reject unsupported encoding but rendered {output}" - ), + Ok(output) => { + bail!("expected contents to reject unsupported encoding but rendered {output}") + } Err(err) => err, }; ensure!( @@ -71,8 +76,8 @@ fn contents_filter_unsupported_encoding() -> Result<()> { err.kind() ); ensure!( - err.to_string().contains("unsupported encoding"), - "error should mention unsupported encoding" + normalize_fluent_isolates(&err.to_string()).contains("Unsupported encoding"), + "error should mention unsupported encoding: {err}" ); Ok(()) } @@ -99,9 +104,7 @@ fn size_filter_missing_file() -> Result<()> { .context("fetch template 'size_missing'")?; let missing = root.join("does_not_exist"); let err = match template.render(context!(path => missing.as_str())) { - Ok(output) => bail!( - "expected size to error for missing file but rendered {output}" - ), + Ok(output) => bail!("expected size to error for missing file but rendered {output}"), Err(err) => err, }; ensure!( diff --git a/tests/std_filter_tests/network_functions.rs b/tests/std_filter_tests/network_functions.rs index 8ded85f2b..04e254973 100644 --- a/tests/std_filter_tests/network_functions.rs +++ b/tests/std_filter_tests/network_functions.rs @@ -1,46 +1,31 @@ //! Tests for stdlib network helpers covering fetch caching and failure paths. -use std::{any, fs, io}; +use std::{any, io}; -use anyhow::{anyhow, bail, ensure, Context, Result}; -use camino::Utf8PathBuf; -use cap_std::{ambient_authority, fs_utf8::Dir}; -use minijinja::{context, Environment, ErrorKind}; +use anyhow::{Context, Result, anyhow, bail, ensure}; +use minijinja::{Environment, ErrorKind, context}; use netsuke::stdlib::{NetworkPolicy, StdlibConfig, StdlibState}; use rstest::{fixture, rstest}; -use tempfile::tempdir; use super::support::fallible; -use test_support::{hash, http}; +use test_support::http; #[fixture] fn http_policy() -> Result { - NetworkPolicy::default().allow_scheme("http") + Ok(NetworkPolicy::default().allow_scheme("http")?) } fn env_with_policy(policy: NetworkPolicy) -> Result<(Environment<'static>, StdlibState)> { fallible::stdlib_env_with_config(StdlibConfig::from_current_dir()?.with_network_policy(policy)) } -fn env_with_workspace_policy( - workspace: Dir, - workspace_path: Utf8PathBuf, - policy: NetworkPolicy, -) -> Result<(Environment<'static>, StdlibState)> { - fallible::stdlib_env_with_config( - StdlibConfig::new(workspace)? - .with_workspace_root_path(workspace_path)? - .with_network_policy(policy), - ) -} - struct FetchTestContext<'env> { env: &'env mut Environment<'static>, state: &'env mut StdlibState, } impl<'env> FetchTestContext<'env> { - fn new(env: &'env mut Environment<'static>, state: &'env mut StdlibState) -> Self { + const fn new(env: &'env mut Environment<'static>, state: &'env mut StdlibState) -> Self { Self { env, state } } @@ -103,12 +88,12 @@ impl<'a> FetchErrorExpectation<'a> { } } -fn identity_policy(policy: NetworkPolicy) -> Result { - Ok(policy) +const fn identity_policy(policy: NetworkPolicy) -> NetworkPolicy { + policy } -fn deny_all_policy(policy: NetworkPolicy) -> Result { - Ok(policy.deny_all_hosts()) +fn deny_all_policy(policy: NetworkPolicy) -> NetworkPolicy { + policy.deny_all_hosts() } fn test_fetch_with_policy( @@ -130,9 +115,7 @@ where let (url, server) = match http::spawn_http_server(content) { Ok(pair) => pair, Err(err) if err.kind() == io::ErrorKind::PermissionDenied => { - tracing::warn!( - "Skipping {test_name}: cannot bind HTTP listener ({err})" - ); + tracing::warn!("Skipping {test_name}: cannot bind HTTP listener ({err})"); return Ok(()); } Err(err) => bail!("failed to spawn HTTP server: {err}"), @@ -149,8 +132,14 @@ where let rendered = tmpl .render(context!(url => url.clone())) .context("render fetch template")?; - ensure!(rendered == expected, "expected {expected} but rendered {rendered}"); - ensure!(ctx.state.is_impure(), "network fetch should mark template impure"); + ensure!( + rendered == expected, + "expected {expected} but rendered {rendered}" + ); + ensure!( + ctx.state.is_impure(), + "network fetch should mark template impure" + ); server .join() .map_err(|err| anyhow!("HTTP server thread panicked: {err:?}"))?; @@ -162,7 +151,7 @@ fn fetch_function_downloads_content(http_policy: Result) -> Resul test_fetch_with_policy( http_policy, "payload", - |policy| policy.block_host("169.254.169.254"), + |policy| Ok(policy.block_host("169.254.169.254")?), "payload", ) } @@ -172,14 +161,14 @@ fn fetch_function_allows_wildcard_hosts(http_policy: Result) -> R test_fetch_with_policy( http_policy, "wildcard", - |policy| policy.deny_all_hosts().allow_hosts(["*.0.0.1"]), + |policy| Ok(policy.deny_all_hosts().allow_hosts(["*.0.0.1"])?), "wildcard", ) } #[rstest] #[case::not_allowlisted( - deny_all_policy as fn(NetworkPolicy) -> Result, + deny_all_policy as fn(NetworkPolicy) -> NetworkPolicy, FetchErrorExpectation::new( "http://127.0.0.1", "not on the allowlist", @@ -188,7 +177,7 @@ fn fetch_function_allows_wildcard_hosts(http_policy: Result) -> R ), )] #[case::connection_failure( - identity_policy as fn(NetworkPolicy) -> Result, + identity_policy as fn(NetworkPolicy) -> NetworkPolicy, FetchErrorExpectation::new( "http://127.0.0.1:9", "Failed to fetch", @@ -198,74 +187,19 @@ fn fetch_function_allows_wildcard_hosts(http_policy: Result) -> R )] fn fetch_function_reports_errors( http_policy: Result, - #[case] transform: fn(NetworkPolicy) -> Result, + #[case] transform: fn(NetworkPolicy) -> NetworkPolicy, #[case] expectation: FetchErrorExpectation<'static>, ) -> Result<()> { - let policy = transform(http_policy?)?; + let policy = transform(http_policy?); let (mut env, mut state) = env_with_policy(policy)?; let mut ctx = FetchTestContext::new(&mut env, &mut state); ctx.prepare_fetch_template()?; ctx.assert_error(expectation) } -#[rstest] -fn fetch_function_respects_cache(http_policy: Result) -> Result<()> { - let temp_dir = tempdir().context("create fetch cache tempdir")?; - let temp_root = Utf8PathBuf::from_path_buf(temp_dir.path().to_path_buf()) - .map_err(|path| anyhow!("temporary root is not valid UTF-8: {path:?}"))?; - let (url, server) = match http::spawn_http_server("cached") { - Ok(pair) => pair, - Err(err) if err.kind() == io::ErrorKind::PermissionDenied => { - tracing::warn!( - "Skipping fetch_function_respects_cache: cannot bind HTTP listener ({err})" - ); - return Ok(()); - } - Err(err) => bail!("failed to spawn HTTP server: {err}"), - }; - let workspace = Dir::open_ambient_dir(&temp_root, ambient_authority()) - .context("open fetch cache workspace")?; - let (mut env, mut state) = env_with_workspace_policy(workspace, temp_root.clone(), http_policy?)?; - state.reset_impure(); - fallible::register_template(&mut env, "fetch_cache", "{{ fetch(url, cache=true) }}")?; - let tmpl = env - .get_template("fetch_cache") - .context("fetch template 'fetch_cache'")?; - let rendered = tmpl - .render(context!(url => url.clone())) - .context("render fetch with caching")?; - ensure!(rendered == "cached", "expected 'cached' but rendered {rendered}"); - ensure!( - state.is_impure(), - "network-backed cache fill should mark template impure" - ); - state.reset_impure(); - server - .join() - .map_err(|err| anyhow!("HTTP server thread panicked: {err:?}"))?; - - // Drop the listener and verify the cached response is returned. - let rendered_again = tmpl - .render(context!(url => url.clone())) - .context("render cached fetch")?; - ensure!(rendered_again == "cached", "expected cached response but rendered {rendered_again}"); - ensure!( - state.is_impure(), - "cached responses should mark template impure", - ); - - let cache_key = hash::sha256_hex(url.as_bytes()); - let cache_path = temp_root.join(".netsuke").join("fetch").join(cache_key); - ensure!( - fs::metadata(cache_path.as_std_path()).is_ok(), - "cache file should exist inside the workspace" - ); - Ok(()) -} - #[rstest] fn fetch_function_rejects_template_cache_dir(http_policy: Result) -> Result<()> { - let (mut env, mut state) = env_with_policy(http_policy?)?; + let (mut env, state) = env_with_policy(http_policy?)?; state.reset_impure(); fallible::register_template( &mut env, @@ -276,9 +210,7 @@ fn fetch_function_rejects_template_cache_dir(http_policy: Result) .get_template("fetch_cache_dir") .context("fetch template 'fetch_cache_dir'")?; let err = match tmpl.render(context!(url => "http://127.0.0.1:9")) { - Ok(output) => bail!( - "expected fetch to reject cache_dir override but rendered {output}" - ), + Ok(output) => bail!("expected fetch to reject cache_dir override but rendered {output}"), Err(err) => err, }; ensure!( diff --git a/tests/std_filter_tests/path_filters.rs b/tests/std_filter_tests/path_filters.rs index df9c9d6de..73c7ff948 100644 --- a/tests/std_filter_tests/path_filters.rs +++ b/tests/std_filter_tests/path_filters.rs @@ -1,11 +1,11 @@ //! Path filter tests for the standard filter library. //! -//! Tests for path manipulation filters including dirname, relative_to, -//! with_suffix, realpath, and expanduser. Each test validates filter +//! Tests for path manipulation filters including `dirname`, `relative_to`, +//! `with_suffix`, `realpath`, and `expanduser`. Each test validates filter //! behaviour with various inputs and error conditions. -use anyhow::{anyhow, bail, ensure, Context, Result}; -use camino::{Utf8Path, Utf8PathBuf}; +use anyhow::{Context, Result, anyhow, bail, ensure}; +use camino::Utf8Path; use cap_std::{ambient_authority, fs_utf8::Dir}; use minijinja::{Environment, ErrorKind}; use rstest::rstest; @@ -13,6 +13,9 @@ use serde_json::{Value, json}; use super::support::{Workspace, fallible}; +#[path = "path_filters/with_suffix.rs"] +mod with_suffix; + /// Helper for standard filter environment setup fn setup_filter_env() -> Result> { fallible::stdlib_env() @@ -39,16 +42,6 @@ struct TemplateErrorSpec<'a> { expectation: TemplateErrorExpectation<'a>, } -/// Specification for a filter success test case. -struct FilterSuccessSpec<'a> { - /// Template name for registration. - name: &'static str, - /// Template source code. - template: &'static str, - /// Path value to pass to the template. - path: &'a Utf8PathBuf, -} - /// Helper for error testing with custom template fn assert_template_error(env: &mut Environment<'_>, spec: TemplateErrorSpec<'_>) -> Result<()> { fallible::register_template(env, spec.name, spec.template)?; @@ -63,9 +56,7 @@ fn assert_template_error(env: &mut Environment<'_>, spec: TemplateErrorSpec<'_>) } = spec; let TemplateErrorExpectation { kind, contains } = expectation; let err = match template.render(context) { - Ok(output) => bail!( - "expected template '{name}' to fail but rendered {output}" - ), + Ok(output) => bail!("expected template '{name}' to fail but rendered {output}"), Err(err) => err, }; ensure!( @@ -111,38 +102,6 @@ where }) } -fn assert_filter_success_with_env( - filter_workspace: Workspace, - home_value: Option<&str>, - spec: FilterSuccessSpec<'_>, - expected: F, -) -> Result<()> -where - F: FnOnce(&Utf8Path) -> String, -{ - let (_temp, root) = filter_workspace; - let home = home_value.map(|value| { - if value.is_empty() { - root.as_str() - } else { - value - } - }); - let mut env = fallible::stdlib_env_with_home(&root, home.map(str::to_owned))?; - let FilterSuccessSpec { - name, - template, - path, - } = spec; - let result = fallible::render(&mut env, name, template, path)?; - let expected_value = expected(&root); - ensure!( - result == expected_value, - "expected '{expected_value}' but rendered {result}" - ); - Ok(()) -} - /// Test data for filter error tests struct FilterErrorTest { name: &'static str, @@ -171,26 +130,30 @@ fn test_filter_error(filter_workspace: Workspace, test: FilterErrorTest) -> Resu } = test; if let Some(EnvironmentSetup::SetHome) = env_setup { - return assert_filter_error_with_env(filter_workspace, Some(""), move |_root| TemplateErrorSpec { - name, - template, - context: context.clone(), - expectation: TemplateErrorExpectation { - kind: error_kind, - contains: error_contains, - }, + return assert_filter_error_with_env(filter_workspace, Some(""), move |_root| { + TemplateErrorSpec { + name, + template, + context: context.clone(), + expectation: TemplateErrorExpectation { + kind: error_kind, + contains: error_contains, + }, + } }); } if let Some(EnvironmentSetup::RemoveHome) = env_setup { - return assert_filter_error_with_env(filter_workspace, None, move |_root| TemplateErrorSpec { - name, - template, - context: context.clone(), - expectation: TemplateErrorExpectation { - kind: error_kind, - contains: error_contains, - }, + return assert_filter_error_with_env(filter_workspace, None, move |_root| { + TemplateErrorSpec { + name, + template, + context: context.clone(), + expectation: TemplateErrorExpectation { + kind: error_kind, + contains: error_contains, + }, + } }); } @@ -225,8 +188,8 @@ fn dirname_filter() -> Result<()> { fn relative_to_filter() -> Result<()> { let workspace = fallible::filter_workspace()?; with_filter_env(workspace, |root, env| { - let dir = Dir::open_ambient_dir(root, ambient_authority()) - .context("open workspace root")?; + let dir = + Dir::open_ambient_dir(root, ambient_authority()).context("open workspace root")?; dir.create_dir_all("nested") .context("create nested directory")?; dir.write("nested/file.txt", b"data") @@ -239,7 +202,10 @@ fn relative_to_filter() -> Result<()> { &nested, ) .context("render relative_to filter")?; - ensure!(output == "file.txt", "expected 'file.txt' but rendered {output}"); + ensure!( + output == "file.txt", + "expected 'file.txt' but rendered {output}" + ); Ok(()) })?; Ok(()) @@ -264,108 +230,6 @@ fn relative_to_filter_outside_root() -> Result<()> { ) } -#[rstest] -fn with_suffix_filter() -> Result<()> { - let workspace = fallible::filter_workspace()?; - with_filter_env(workspace, |root, env| { - let file = root.join("file.tar.gz"); - Dir::open_ambient_dir(root, ambient_authority()) - .context("open workspace root")? - .write("file.tar.gz", b"data") - .context("write archive fixture")?; - let first = fallible::render(env, "suffix", "{{ path | with_suffix('.log') }}", &file) - .context("render with_suffix(.log)")?; - ensure!( - first == root.join("file.tar.log").as_str(), - "expected '.log' suffix to replace final component but rendered {first}" - ); - let second = fallible::render( - env, - "suffix_alt", - "{{ path | with_suffix('.zip', 2) }}", - &file, - ) - .context("render with_suffix(.zip, 2)")?; - ensure!( - second == root.join("file.zip").as_str(), - "expected two extensions to be replaced but rendered {second}" - ); - let third = fallible::render( - env, - "suffix_count_zero", - "{{ path | with_suffix('.bak', 0) }}", - &file, - ) - .context("render with_suffix(.bak, 0)")?; - ensure!( - third == root.join("file.tar.gz.bak").as_str(), - "expected zero count to append suffix but rendered {third}" - ); - Ok(()) - })?; - Ok(()) -} - -#[rstest] -fn with_suffix_filter_without_separator() -> Result<()> { - let workspace = fallible::filter_workspace()?; - with_filter_env(workspace, |root, env| { - let file = root.join("file"); - let output = fallible::render( - env, - "suffix_plain", - "{{ path | with_suffix('.log') }}", - &file, - ) - .context("render with_suffix on filename without separator")?; - ensure!( - output == root.join("file.log").as_str(), - "expected '.log' to be appended but rendered {output}" - ); - Ok(()) - })?; - Ok(()) -} - -#[rstest] -fn with_suffix_filter_empty_separator() -> Result<()> { - let workspace = fallible::filter_workspace()?; - test_filter_error( - workspace, - FilterErrorTest { - name: "suffix_empty_sep", - template: "{{ path | with_suffix('.log', 1, '') }}", - context: json!({ - "path": "file.tar.gz", - }), - error_kind: ErrorKind::InvalidOperation, - error_contains: "non-empty separator", - env_setup: None, - }, - ) -} - -#[rstest] -fn with_suffix_filter_excessive_count() -> Result<()> { - let workspace = fallible::filter_workspace()?; - with_filter_env(workspace, |root, env| { - let file = root.join("file.tar.gz"); - let output = fallible::render( - env, - "suffix_excessive", - "{{ path | with_suffix('.bak', 5) }}", - &file, - ) - .context("render with_suffix(.bak, 5)")?; - ensure!( - output == root.join("file.bak").as_str(), - "expected excessive count to collapse extensions but rendered {output}" - ); - Ok(()) - })?; - Ok(()) -} - #[cfg(unix)] #[rstest] fn realpath_filter() -> Result<()> { @@ -427,22 +291,6 @@ fn realpath_filter_root_path() -> Result<()> { Ok(()) } -#[rstest] -fn expanduser_filter() -> Result<()> { - let workspace = fallible::filter_workspace()?; - let path = Utf8PathBuf::from("~/workspace"); - assert_filter_success_with_env( - workspace, - Some(""), - FilterSuccessSpec { - name: "expanduser", - template: "{{ path | expanduser }}", - path: &path, - }, - |root| root.join("workspace").as_str().to_owned(), - ) -} - #[rstest] fn expanduser_filter_non_tilde_path() -> Result<()> { let workspace = fallible::filter_workspace()?; @@ -450,7 +298,10 @@ fn expanduser_filter_non_tilde_path() -> Result<()> { let file = root.join("file"); let output = fallible::render(env, "expanduser_plain", "{{ path | expanduser }}", &file) .context("render expanduser on non-tilde path")?; - ensure!(output == file.as_str(), "expected path to remain unchanged but rendered {output}"); + ensure!( + output == file.as_str(), + "expected path to remain unchanged but rendered {output}" + ); Ok(()) })?; Ok(()) @@ -486,7 +337,7 @@ fn expanduser_filter_user_specific() -> Result<()> { "path": "~otheruser/workspace", }), error_kind: ErrorKind::InvalidOperation, - error_contains: "user-specific ~ expansion is unsupported", + error_contains: "User-specific ~ expansion is unsupported", env_setup: Some(EnvironmentSetup::SetHome), }, ) diff --git a/tests/std_filter_tests/path_filters/with_suffix.rs b/tests/std_filter_tests/path_filters/with_suffix.rs new file mode 100644 index 000000000..bb31c661c --- /dev/null +++ b/tests/std_filter_tests/path_filters/with_suffix.rs @@ -0,0 +1,116 @@ +//! Integration coverage for the `with_suffix` path filter. + +use anyhow::{Context, Result, ensure}; +use cap_std::{ambient_authority, fs_utf8::Dir}; +use minijinja::ErrorKind; +use rstest::{fixture, rstest}; +use serde_json::json; + +use super::{FilterErrorTest, Workspace, fallible, test_filter_error, with_filter_env}; + +/// Yield a fresh, isolated filter workspace. +/// +/// rstest invokes this per test case, so each test owns its own `TempDir` and +/// no fixture state leaks between them. +#[fixture] +fn workspace() -> Result { + fallible::filter_workspace() +} + +#[rstest] +fn with_suffix_filter(workspace: Result) -> Result<()> { + with_filter_env(workspace?, |root, env| { + let file = root.join("file.tar.gz"); + Dir::open_ambient_dir(root, ambient_authority()) + .context("open workspace root")? + .write("file.tar.gz", b"data") + .context("write archive fixture")?; + let first = fallible::render(env, "suffix", "{{ path | with_suffix('.log') }}", &file) + .context("render with_suffix(.log)")?; + ensure!( + first == root.join("file.tar.log").as_str(), + "expected '.log' suffix to replace final component but rendered {first}" + ); + let second = fallible::render( + env, + "suffix_alt", + "{{ path | with_suffix('.zip', 2) }}", + &file, + ) + .context("render with_suffix(.zip, 2)")?; + ensure!( + second == root.join("file.zip").as_str(), + "expected two extensions to be replaced but rendered {second}" + ); + let third = fallible::render( + env, + "suffix_count_zero", + "{{ path | with_suffix('.bak', 0) }}", + &file, + ) + .context("render with_suffix(.bak, 0)")?; + ensure!( + third == root.join("file.tar.gz.bak").as_str(), + "expected zero count to append suffix but rendered {third}" + ); + Ok(()) + })?; + Ok(()) +} + +#[rstest] +fn with_suffix_filter_without_separator(workspace: Result) -> Result<()> { + with_filter_env(workspace?, |root, env| { + let file = root.join("file"); + let output = fallible::render( + env, + "suffix_plain", + "{{ path | with_suffix('.log') }}", + &file, + ) + .context("render with_suffix on filename without separator")?; + ensure!( + output == root.join("file.log").as_str(), + "expected '.log' to be appended but rendered {output}" + ); + Ok(()) + })?; + Ok(()) +} + +#[rstest] +fn with_suffix_filter_empty_separator(workspace: Result) -> Result<()> { + test_filter_error( + workspace?, + FilterErrorTest { + name: "suffix_empty_sep", + template: "{{ path | with_suffix('.log', 1, '') }}", + context: json!({ + "path": "file.tar.gz", + }), + error_kind: ErrorKind::InvalidOperation, + error_contains: "non-empty separator", + env_setup: None, + }, + ) +} + +#[rstest] +fn with_suffix_filter_excessive_count(workspace: Result) -> Result<()> { + with_filter_env(workspace?, |root, env| { + let file = root.join("file.tar.gz"); + let output = fallible::render( + env, + "suffix_excessive", + "{{ path | with_suffix('.bak', 5) }}", + &file, + ) + .context("render with_suffix(.bak, 5)")?; + ensure!( + output == root.join("file.bak").as_str(), + "expected excessive count to collapse extensions but rendered {output}" + ); + Ok(()) + })?; + Ok(()) +} diff --git a/tests/std_filter_tests/support.rs b/tests/std_filter_tests/support.rs index 115e546e3..a79c1c6b7 100644 --- a/tests/std_filter_tests/support.rs +++ b/tests/std_filter_tests/support.rs @@ -6,22 +6,15 @@ //! isolated workspace used by each test. use camino::Utf8PathBuf; -use cap_std::{ambient_authority, fs_utf8::Dir}; -use minijinja::{Environment, context}; -use netsuke::stdlib::{self, StdlibConfig, StdlibState}; +use netsuke::stdlib; pub(crate) type Workspace = (tempfile::TempDir, Utf8PathBuf); pub(crate) mod fallible { - //! Fallible variants of the `support` helpers, returning `anyhow::Result` - //! instead of panicking on setup failure. - //! - //! The parent module re-exports these functions so most call sites reach - //! them via `support::*`; keeping them namespaced here separates the - //! error-propagating implementations from the workspace types they - //! operate on. + //! Fallible fixture builders that preserve setup diagnostics for callers. + use super::{Workspace, stdlib}; - use anyhow::{anyhow, Context, Result}; + use anyhow::{Context, Result, anyhow}; use camino::Utf8PathBuf; use cap_std::{ambient_authority, fs_utf8::Dir}; use minijinja::{Environment, context}; @@ -80,7 +73,8 @@ pub(crate) mod fallible { .map_err(|path| anyhow!("workspace path is not valid UTF-8: {path:?}"))?; let dir = Dir::open_ambient_dir(&root, ambient_authority()) .context("open filter workspace directory")?; - dir.write("file", b"data").context("write fixture file 'file'")?; + dir.write("file", b"data") + .context("write fixture file 'file'")?; #[cfg(unix)] dir.symlink("file", "link") .context("create fixture symlink")?; @@ -100,20 +94,13 @@ pub(crate) mod fallible { ) -> Result { env.add_template(name, template) .with_context(|| format!("register template '{name}'"))?; - let template = env + let registered_template = env .get_template(name) .with_context(|| format!("fetch template '{name}'"))?; - template + registered_template .render(context!(path => path.as_str())) .with_context(|| format!("render template '{name}'")) } } -pub(crate) use fallible::{ - filter_workspace, - register_template, - render, - stdlib_env, - stdlib_env_with_config, - stdlib_env_with_state, -}; +pub(crate) use fallible::filter_workspace; diff --git a/tests/std_filter_tests/which_filter_common.rs b/tests/std_filter_tests/which_filter_common.rs index 593fb354c..7294176e6 100644 --- a/tests/std_filter_tests/which_filter_common.rs +++ b/tests/std_filter_tests/which_filter_common.rs @@ -2,9 +2,9 @@ use anyhow::{Context, Result, anyhow}; use camino::{Utf8Path, Utf8PathBuf}; -use minijinja::{context, Environment}; +use minijinja::{Environment, context}; use std::ffi::{OsStr, OsString}; - +use test_support::fs; use super::support::{self, fallible}; @@ -12,52 +12,72 @@ use super::support::{self, fallible}; pub(crate) struct ToolName(String); impl ToolName { - pub(crate) fn new(name: impl Into) -> Self { Self(name.into()) } - pub(crate) fn as_str(&self) -> &str { &self.0 } + pub(crate) fn as_str(&self) -> &str { + &self.0 + } } impl From<&str> for ToolName { - fn from(s: &str) -> Self { Self(s.to_owned()) } + fn from(s: &str) -> Self { + Self(s.to_owned()) + } } impl AsRef for ToolName { - fn as_ref(&self) -> &str { &self.0 } + fn as_ref(&self) -> &str { + &self.0 + } } #[derive(Debug, Clone)] pub(crate) struct DirName(String); impl DirName { - pub(crate) fn new(name: impl Into) -> Self { Self(name.into()) } - pub(crate) fn as_str(&self) -> &str { &self.0 } + pub(crate) fn as_str(&self) -> &str { + &self.0 + } } impl From<&str> for DirName { - fn from(s: &str) -> Self { Self(s.to_owned()) } + fn from(s: &str) -> Self { + Self(s.to_owned()) + } } impl AsRef for DirName { - fn as_ref(&self) -> &str { &self.0 } + fn as_ref(&self) -> &str { + &self.0 + } } impl AsRef for DirName { - fn as_ref(&self) -> &OsStr { OsStr::new(&self.0) } + fn as_ref(&self) -> &OsStr { + OsStr::new(&self.0) + } } #[derive(Debug, Clone)] pub(crate) struct Template(String); impl Template { - pub(crate) fn new(template: impl Into) -> Self { Self(template.into()) } - pub(crate) fn as_str(&self) -> &str { &self.0 } + pub(crate) fn new(template: impl Into) -> Self { + Self(template.into()) + } + pub(crate) fn as_str(&self) -> &str { + &self.0 + } } impl From<&str> for Template { - fn from(s: &str) -> Self { Self(s.to_owned()) } + fn from(s: &str) -> Self { + Self(s.to_owned()) + } } impl AsRef for Template { - fn as_ref(&self) -> &str { &self.0 } + fn as_ref(&self) -> &str { + &self.0 + } } pub(crate) struct PathEnv(OsString); @@ -84,39 +104,41 @@ pub(crate) fn write_tool(dir: &Utf8Path, name: &ToolName) -> Result let parent = path .parent() .context("tool path should have a parent directory")?; - std::fs::create_dir_all(parent.as_std_path()) - .with_context(|| format!("create parent for {path:?}"))?; - std::fs::write(path.as_std_path(), script_contents()) - .with_context(|| format!("write fixture {path:?}"))?; + fs::create_dir_all(parent).with_context(|| format!("create parent for {path:?}"))?; + fs::write(&path, script_contents()).with_context(|| format!("write fixture {path:?}"))?; mark_executable(&path)?; Ok(path) } #[cfg(unix)] fn mark_executable(path: &Utf8Path) -> Result<()> { - use std::os::unix::fs::PermissionsExt; - let mut perms = std::fs::metadata(path.as_std_path()) - .with_context(|| format!("stat {path:?}"))? - .permissions(); - perms.set_mode(0o755); - std::fs::set_permissions(path.as_std_path(), perms) - .with_context(|| format!("chmod {path:?}")) + fs::set_mode(path, 0o755).with_context(|| format!("chmod {path:?}")) } #[cfg(not(unix))] -fn mark_executable(_path: &Utf8Path) -> Result<()> { Ok(()) } +fn mark_executable(_path: &Utf8Path) -> Result<()> { + Ok(()) +} #[cfg(windows)] -fn tool_name(base: &ToolName) -> String { format!("{}.cmd", base.as_str()) } +fn tool_name(base: &ToolName) -> String { + format!("{}.cmd", base.as_str()) +} #[cfg(not(windows))] -fn tool_name(base: &ToolName) -> String { base.as_str().to_owned() } +fn tool_name(base: &ToolName) -> String { + base.as_str().to_owned() +} -fn script_contents() -> &'static [u8] { +const fn script_contents() -> &'static [u8] { #[cfg(windows)] - { b"@echo off\r\n" } + { + b"@echo off\r\n" + } #[cfg(not(windows))] - { b"#!/bin/sh\nexit 0\n" } + { + b"#!/bin/sh\nexit 0\n" + } } pub(crate) fn render(env: &mut Environment<'_>, template: &Template) -> Result { @@ -138,8 +160,7 @@ impl WhichTestFixture { let mut tool_paths = Vec::new(); for dir_name in dir_names { let dir = root.join(dir_name.as_str()); - std::fs::create_dir_all(dir.as_std_path()) - .with_context(|| format!("create directory {}", dir))?; + fs::create_dir_all(&dir).with_context(|| format!("create directory {dir}"))?; let tool_path = write_tool(&dir, tool_name)?; dirs.push(dir); tool_paths.push(tool_path); diff --git a/tests/std_filter_tests/which_filter_tests.rs b/tests/std_filter_tests/which_filter_tests.rs index 20964df6d..807f90f00 100644 --- a/tests/std_filter_tests/which_filter_tests.rs +++ b/tests/std_filter_tests/which_filter_tests.rs @@ -1,52 +1,23 @@ //! Integration tests for the `which` filter/function covering PATH resolution, //! canonicalization, cwd behaviour, workspace fallback, and diagnostic output. -use anyhow::{Context, Result}; +use anyhow::{Context, Result, ensure}; +use cap_std::{ambient_authority, fs_utf8::Dir}; +use minijinja::context; use netsuke::stdlib::StdlibConfig; use rstest::rstest; -use serde_json::Value; -use std::{env, path::PathBuf}; -use tempfile::tempdir; -use test_support::env_lock::EnvLock; +use test_support::fs; use super::support::{self, fallible}; use super::which_filter_common::*; -struct CurrentDirGuard { - original: PathBuf, - _lock: EnvLock, -} - -impl CurrentDirGuard { - fn change_to(path: &std::path::Path) -> Result { - let lock = EnvLock::acquire(); - let original = env::current_dir().context("capture current working directory")?; - env::set_current_dir(path).context("switch cwd")?; - Ok(Self { - original, - _lock: lock, - }) - } -} - -impl Drop for CurrentDirGuard { - fn drop(&mut self) { - if let Err(err) = env::set_current_dir(&self.original) { - tracing::warn!( - "failed to restore working directory to {}: {err}", - self.original.display() - ); - } - } -} - -fn render_and_assert_pure( - fixture: &mut WhichTestFixture, - template: &Template, -) -> Result { +fn render_and_assert_pure(fixture: &mut WhichTestFixture, template: &Template) -> Result { fixture.state.reset_impure(); let output = fixture.render(template)?; - assert!(!fixture.state.is_impure()); + ensure!( + !fixture.state.is_impure(), + "which lookup should leave the template pure" + ); Ok(output) } @@ -57,21 +28,36 @@ fn test_cache_after_removal( expect_second_err: bool, ) -> Result<()> { fixture.state.reset_impure(); - let removed_path = &fixture.paths[0]; let first = fixture.render(first_template)?; - assert_eq!(first, removed_path.as_str()); + let removed_path = fixture + .paths + .first() + .context("cache-removal fixture should contain a tool path")? + .clone(); + ensure!( + first == removed_path.as_str(), + "initial which lookup should resolve {removed_path}, got {first}" + ); - std::fs::remove_file(removed_path)?; + fs::remove_file(&removed_path)?; fixture.state.reset_impure(); let second_result = fixture.render(second_template); if expect_second_err { - let err = second_result.expect_err("expected fresh which lookup to fail after removal"); - assert!(err.to_string().contains("not_found")); + let err = second_result + .err() + .context("expected fresh which lookup to fail after removal")?; + ensure!( + err.to_string().contains("not_found"), + "fresh lookup error should report not_found: {err}" + ); } else { let second = second_result?; - assert_eq!(second, removed_path.as_str()); + ensure!( + second == removed_path.as_str(), + "cached lookup should preserve {removed_path}, got {second}" + ); } Ok(()) @@ -82,10 +68,8 @@ fn test_cache_behaviour_after_removal( second_template_str: &str, expect_second_err: bool, ) -> Result<()> { - let mut fixture = WhichTestFixture::with_tool_in_dirs( - &ToolName::from("helper"), - &[DirName::from("bin_first"), DirName::from("bin_second")], - )?; + let mut fixture = + WhichTestFixture::with_tool_in_dirs(&ToolName::from("helper"), &[DirName::from("bin")])?; test_cache_after_removal( &mut fixture, @@ -108,35 +92,45 @@ fn test_duplicate_paths( let output = render_and_assert_pure(fixture, &template)?; let parts: Vec<&str> = output.split('|').collect(); - - assert_eq!(parts.len(), expected_count); + let expected_path = fixture + .paths + .first() + .context("duplicate-path fixture should contain a tool path")?; + + ensure!( + parts.len() == expected_count, + "expected {expected_count} which results, got {}", + parts.len() + ); for part in &parts { - assert_eq!(*part, fixture.paths[0].as_str()); + ensure!( + *part == expected_path.as_str(), + "expected duplicate path {expected_path}, got {part}" + ); } Ok(()) } -/// Helper to test cwd_mode resolution with a given case variant. +/// Helper to test `cwd_mode` resolution with a given case variant. fn test_cwd_mode_resolution(cwd_mode_value: &str) -> Result<()> { let (_temp, root) = support::filter_workspace()?; let tool = write_tool(&root, &ToolName::from("local"))?; let path = PathEnv::new(&[])?; - let (mut env, _state) = fallible::stdlib_env_with_path(path.into_inner())?; - let template = Template::from(format!("{{{{ which('local', cwd_mode='{cwd_mode_value}') }}}}")); + let config = StdlibConfig::new( + Dir::open_ambient_dir(&root, ambient_authority()).context("open workspace")?, + )? + .with_workspace_root_path(root)?; + let (mut env, _state) = + fallible::stdlib_env_with_config(config.with_path_override(path.into_inner()))?; + let template = Template::new(format!( + "{{{{ which('local', cwd_mode='{cwd_mode_value}') }}}}" + )); let output = render(&mut env, &template)?; - assert_eq!(output, tool.as_str()); - Ok(()) -} - -#[rstest] -fn which_filter_returns_first_match() -> Result<()> { - let mut fixture = WhichTestFixture::with_tool_in_dirs( - &ToolName::from("helper"), - &[DirName::from("bin_first"), DirName::from("bin_second")], - )?; - let output = render_and_assert_pure(&mut fixture, &Template::from("{{ 'helper' | which }}"))?; - assert_eq!(output, fixture.paths[0].as_str()); + ensure!( + output == tool.as_str(), + "cwd_mode {cwd_mode_value} should resolve {tool}, got {output}" + ); Ok(()) } @@ -150,42 +144,6 @@ fn which_filter_fresh_bypasses_cache_after_executable_removed() -> Result<()> { test_cache_behaviour_after_removal("{{ 'helper' | which(fresh=true) }}", true) } -#[rstest] -fn which_filter_all_returns_all_matches() -> Result<()> { - let mut fixture = WhichTestFixture::with_tool_in_dirs( - &ToolName::from("helper"), - &[DirName::from("bin_a"), DirName::from("bin_b")], - )?; - let output = render_and_assert_pure( - &mut fixture, - &Template::from("{{ 'helper' | which(all=true) | join('|') }}"), - )?; - let expected = format!( - "{}|{}", - fixture.paths[0].as_str(), - fixture.paths[1].as_str() - ); - assert_eq!(output, expected); - Ok(()) -} - -#[rstest] -fn which_filter_all_returns_list() -> Result<()> { - let mut fixture = WhichTestFixture::with_tool_in_dirs( - &ToolName::from("helper"), - &[DirName::from("bin_a"), DirName::from("bin_b")], - )?; - - let output = fixture.render(&Template::from("{{ which('helper', all=true) | tojson }}"))?; - let value: Value = serde_json::from_str(&output)?; - assert!( - value.is_array(), - "expected which(..., all=true) to return a JSON array, got {value:?}", - ); - - Ok(()) -} - #[rstest] fn which_filter_all_with_duplicates_respects_canonical_false() -> Result<()> { let mut fixture = WhichTestFixture::with_tool_in_dirs( @@ -204,11 +162,6 @@ fn which_filter_all_with_duplicates_deduplicates_canonicalized_paths() -> Result test_duplicate_paths(&mut fixture, true, 1) } -#[rstest] -fn which_function_honours_cwd_mode() -> Result<()> { - test_cwd_mode_resolution("always") -} - #[rstest] fn which_function_rejects_invalid_cwd_mode() -> Result<()> { let (_temp, _root) = support::filter_workspace()?; @@ -217,14 +170,15 @@ fn which_function_rejects_invalid_cwd_mode() -> Result<()> { let template = Template::from("{{ which('local', cwd_mode='invalid') }}"); let err = render(&mut env, &template) - .expect_err("expected invalid cwd_mode to fail"); + .err() + .context("expected invalid cwd_mode to fail")?; let message = err.to_string(); - assert!( + ensure!( message.contains("netsuke::jinja::which::args"), "expected which args error, got: {message}", ); - assert!( + ensure!( message.contains("cwd_mode"), "expected message to mention cwd_mode, got: {message}", ); @@ -237,53 +191,37 @@ fn which_function_accepts_case_insensitive_cwd_mode() -> Result<()> { test_cwd_mode_resolution("ALWAYS") } -#[rstest] -fn which_filter_reports_missing_command() -> Result<()> { - let (_temp, _root) = support::filter_workspace()?; - let path = PathEnv::new(&[])?; - let (mut env, _state) = fallible::stdlib_env_with_path(path.into_inner())?; - let err = env - .render_str("{{ 'absent' | which }}", context! {}) - .expect_err("render should fail for missing command"); - let message = err.to_string(); - assert!(message.contains("netsuke::jinja::which::not_found")); - Ok(()) -} - -#[rstest] -fn which_filter_falls_back_to_workspace_when_path_empty() -> Result<()> { - let (_temp, root) = support::filter_workspace()?; - let tool = write_tool(&root, &ToolName::from("helper"))?; - let path = PathEnv::new(&[])?; - let (mut env, _state) = fallible::stdlib_env_with_path(path.into_inner())?; - let output = render(&mut env, &Template::from("{{ 'helper' | which }}"))?; - assert_eq!(output, tool.as_str()); - Ok(()) -} - #[rstest] fn which_filter_skips_heavy_directories() -> Result<()> { let (_temp, root) = support::filter_workspace()?; let target = root.join("target"); - std::fs::create_dir_all(target.as_std_path())?; + fs::create_dir_all(&target)?; write_tool(&target, &ToolName::from("helper"))?; + // Root the resolver at the fixture workspace; otherwise the lookup searches + // the process working directory, `target/helper` is never a candidate, and + // the `not_found` assertion below would hold regardless of the skip policy. + let config = StdlibConfig::new( + Dir::open_ambient_dir(&root, ambient_authority()).context("open workspace")?, + )? + .with_workspace_root_path(root)?; let path = PathEnv::new(&[])?; - let (mut env, _state) = fallible::stdlib_env_with_path(path.into_inner())?; + let (env, _state) = + fallible::stdlib_env_with_config(config.with_path_override(path.into_inner()))?; let err = env .render_str("{{ 'helper' | which }}", context! {}) - .expect_err("render should fail when tool is in skipped directory"); - assert!(err.to_string().contains("not_found")); + .err() + .context("render should fail when tool is in skipped directory")?; + ensure!( + err.to_string().contains("not_found"), + "skipped-directory lookup should report not_found: {err}" + ); Ok(()) } #[rstest] fn which_resolver_honours_workspace_root_override() -> Result<()> { - use cap_std::{ambient_authority, fs_utf8::Dir}; let (_temp, root) = support::filter_workspace()?; let tool = write_tool(&root, &ToolName::from("helper"))?; - let alt = tempdir().context("create alternate cwd")?; - let _cwd_guard = CurrentDirGuard::change_to(alt.path())?; - let config = StdlibConfig::new( Dir::open_ambient_dir(&root, ambient_authority()).context("open workspace")?, )? @@ -292,6 +230,9 @@ fn which_resolver_honours_workspace_root_override() -> Result<()> { let (mut env, _state) = fallible::stdlib_env_with_config(config.with_path_override(path.into_inner()))?; let output = render(&mut env, &Template::from("{{ 'helper' | which }}"))?; - assert_eq!(output, tool.as_str()); + ensure!( + output == tool.as_str(), + "workspace override should resolve {tool}, got {output}" + ); Ok(()) }