From 2492d031d84b85a651875cd894752cc5a86df42f Mon Sep 17 00:00:00 2001 From: Payton McIntosh Date: Wed, 5 Aug 2026 23:36:20 +0100 Subject: [PATCH 1/5] Bump Cargo.lock --- Cargo.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index 6e6e35ed6..ce28d767c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1465,7 +1465,7 @@ dependencies = [ [[package]] name = "netsuke-build" -version = "0.1.0" +version = "0.1.0-beta1" dependencies = [ "anyhow", "assert_cmd", From 19f8c9ec3292e57c63a443aa4aaac5ed27f594ac Mon Sep 17 00:00:00 2001 From: Payton McIntosh Date: Wed, 5 Aug 2026 23:56:47 +0100 Subject: [PATCH 2/5] Derive version-sensitive test contracts from the crate version MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The v0.1.0-beta1 bump broke the `diagnostic_json` snapshots, which baked the generator version into the stored JSON, and exposed two hardcoded literals in the documentation contract: - The PowerShell help-directory fragment asserted `Netsuke\0.1.0` while the docs correctly moved to `Netsuke\0.1.0-beta1`. - The release-tag assertion was a `contains` check on `releases/tag/v0.1.0`, which only kept passing because `v0.1.0` is a prefix of `v0.1.0-beta1` — it would have silently asserted the wrong release forever. Redact the generator version in the snapshots with an insta filter (enabling the `filters` feature), and derive both documentation contract literals from `CARGO_PKG_VERSION` so `Cargo.toml` remains the single source of truth and future bumps cannot reproduce the drift. Also teach `test_support`'s binary locator to fall back to `CARGO_TARGET_DIR` when Cargo's `build.build-dir` splits intermediate artefacts from final ones: test executables then run from the build dir while the uplifted `netsuke` binary is placed under the target dir, so the current-exe-derived path alone cannot find it. --- Cargo.lock | 2 ++ Cargo.toml | 2 +- src/diagnostic_json_tests.rs | 3 ++ ...json__tests__circular_dependency_json.snap | 2 +- ...tic_json__tests__manifest_parse_error.snap | 3 +- test_support/src/netsuke.rs | 29 +++++++++++++++---- tests/documentation_examples_tests.rs | 16 ++++++++-- 7 files changed, 44 insertions(+), 13 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index ce28d767c..0d0710807 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1135,8 +1135,10 @@ checksum = "86f0f8fee8c926415c58d6ae43a08523a26faccb2323f5e6b644fe7dd4ef6b82" dependencies = [ "console", "once_cell", + "regex", "serde", "similar", + "strip-ansi-escapes", "tempfile", ] diff --git a/Cargo.toml b/Cargo.toml index 857f42875..5447b1083 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -162,7 +162,7 @@ rstest = "0.26.1" rstest-bdd = "0.5.0" rstest-bdd-macros = { version = "0.5.0", features = ["strict-compile-time-validation"] } tokio = { version = "1", features = ["macros", "rt-multi-thread"], default-features = false } -insta = { version = "1", features = ["yaml"] } +insta = { version = "1", features = ["yaml", "filters"] } assert_cmd = "2.0.0" predicates = "3" # Provides the DebuggingRecorder used to assert bounded telemetry without a diff --git a/src/diagnostic_json_tests.rs b/src/diagnostic_json_tests.rs index 479bc3284..da9c8c805 100644 --- a/src/diagnostic_json_tests.rs +++ b/src/diagnostic_json_tests.rs @@ -20,12 +20,15 @@ fn parse_json_value(document: &str) -> Result { } /// Builds insta [`Settings`] pointing at the `src/snapshots/diagnostic_json` directory. +/// +/// The generator version is redacted so snapshots survive version bumps. fn snapshot_settings() -> Settings { let mut settings = Settings::new(); settings.set_snapshot_path(concat!( env!("CARGO_MANIFEST_DIR"), "/src/snapshots/diagnostic_json" )); + settings.add_filter(r#""version": "[^"]+""#, r#""version": "[version]""#); settings } diff --git a/src/snapshots/diagnostic_json/netsuke__diagnostic_json__tests__circular_dependency_json.snap b/src/snapshots/diagnostic_json/netsuke__diagnostic_json__tests__circular_dependency_json.snap index 879a88825..ddba5dd82 100644 --- a/src/snapshots/diagnostic_json/netsuke__diagnostic_json__tests__circular_dependency_json.snap +++ b/src/snapshots/diagnostic_json/netsuke__diagnostic_json__tests__circular_dependency_json.snap @@ -6,7 +6,7 @@ expression: rendered "schema_version": 1, "generator": { "name": "netsuke", - "version": "0.1.0" + "version": "[version]" }, "diagnostics": [ { diff --git a/src/snapshots/diagnostic_json/netsuke__diagnostic_json__tests__manifest_parse_error.snap b/src/snapshots/diagnostic_json/netsuke__diagnostic_json__tests__manifest_parse_error.snap index 6083e650c..1f28a261e 100644 --- a/src/snapshots/diagnostic_json/netsuke__diagnostic_json__tests__manifest_parse_error.snap +++ b/src/snapshots/diagnostic_json/netsuke__diagnostic_json__tests__manifest_parse_error.snap @@ -1,13 +1,12 @@ --- source: src/diagnostic_json_tests.rs -assertion_line: 151 expression: rendered --- { "schema_version": 1, "generator": { "name": "netsuke", - "version": "0.1.0" + "version": "[version]" }, "diagnostics": [ { diff --git a/test_support/src/netsuke.rs b/test_support/src/netsuke.rs index cac9f2e46..2ee2424ee 100644 --- a/test_support/src/netsuke.rs +++ b/test_support/src/netsuke.rs @@ -5,20 +5,37 @@ //! capturing stdout/stderr for assertions. use anyhow::{Context, Result, ensure}; +use mockable::{DefaultEnv, Env}; use std::path::Path; use std::path::PathBuf; /// Locate the built `netsuke` executable for integration-style tests. /// -/// Derive the path from the current test executable's target directory. +/// Derive the path from the current test executable's target directory. When +/// Cargo's `build.build-dir` splits intermediate artefacts from final ones, +/// test executables run from the build dir while the uplifted binary lands +/// under `CARGO_TARGET_DIR`, so fall back to that location. fn netsuke_executable() -> Result { - let mut target_dir = std::env::current_exe().context("locate current test executable")?; - target_dir.pop(); - if target_dir.ends_with("deps") { - target_dir.pop(); + let mut exe_dir = std::env::current_exe().context("locate current test executable")?; + exe_dir.pop(); + if exe_dir.ends_with("deps") { + exe_dir.pop(); } - let path = target_dir.join(format!("netsuke{}", std::env::consts::EXE_SUFFIX)); + let binary_name = format!("netsuke{}", std::env::consts::EXE_SUFFIX); + let path = exe_dir.join(&binary_name); + if path.is_file() { + return Ok(path); + } + if let (Some(target_dir), Some(profile)) = ( + DefaultEnv.os_string("CARGO_TARGET_DIR"), + exe_dir.file_name(), + ) { + let fallback = Path::new(&target_dir).join(profile).join(&binary_name); + if fallback.is_file() { + return Ok(fallback); + } + } ensure!( path.is_file(), "locate netsuke binary at {}", diff --git a/tests/documentation_examples_tests.rs b/tests/documentation_examples_tests.rs index 90e16d6e8..a78201428 100644 --- a/tests/documentation_examples_tests.rs +++ b/tests/documentation_examples_tests.rs @@ -255,8 +255,13 @@ fn assert_release_installation_contract() -> Result<()> { ); ensure!(readme_release.body == expected_release, "README drifted"); ensure!(guide_release.body == expected_release, "user guide drifted"); + // Derive version literals from the crate version so docs contracts track + // release bumps instead of drifting behind them. let expected_release_details = [ - "https://github.com/leynos/netsuke/releases/tag/v0.1.0", + concat!( + "https://github.com/leynos/netsuke/releases/tag/v", + env!("CARGO_PKG_VERSION") + ), "Debian (`.deb`) and RPM (`.rpm`)", "Installer package (`.pkg`)", "Windows Installer (`.msi`)", @@ -269,7 +274,8 @@ fn assert_release_installation_contract() -> Result<()> { for expected in expected_release_details { ensure!( document.contains(expected), - "{path} should document v0.1.0 release detail: {expected}" + "{path} should document v{} release detail: {expected}", + env!("CARGO_PKG_VERSION") ); } } @@ -295,7 +301,11 @@ fn assert_windows_setup_examples() -> Result<()> { let windows_help_install = documented_example("guide-windows-help-install")?; let windows_help_fragments = [ "Import-Module", - "$moduleDirectory = Join-Path $moduleRoot 'Netsuke\\0.1.0'", + concat!( + "$moduleDirectory = Join-Path $moduleRoot 'Netsuke\\", + env!("CARGO_PKG_VERSION"), + "'" + ), "Import-Module (Join-Path $moduleDirectory 'Netsuke.psd1')", "*windows-$architecture*", ]; From 30fd86d355be0fdca818a903d8e8464d58b737dc Mon Sep 17 00:00:00 2001 From: Payton McIntosh Date: Thu, 6 Aug 2026 00:59:56 +0100 Subject: [PATCH 3/5] Address review feedback on version-contract fixes Narrow the diagnostic-JSON snapshot filter so it anchors on the generator block's `"name": "netsuke"` line: only the generator's version is redacted, and any other `version` field that later appears in a diagnostic document stays visible in snapshot diffs. Rework the `test_support` binary locator for injectability and robustness: - Split the lookup into `netsuke_executable_from(env, current_exe)` with an injected `mockable::Env`, mirroring the `compile_rust_helper_with_env` pattern, so the fallback logic is unit-testable with `MockEnv`. - Use `camino::Utf8PathBuf` throughout, converting at the `current_exe()` boundary, consistent with the rest of the crate. - Add a third candidate, `CARGO_TARGET_DIR///`, so `--target` builds resolve; the reviewer's suggestion to insert the triple unconditionally would have broken the ordinary no-`--target` layout, so the triple path is an additional candidate instead. - Surface filesystem errors other than not-found through a new `test_support::fs::try_is_file` wrapper (the crate's sanctioned ambient boundary under the Whitaker `no_std_fs_operations` policy), and list every attempted candidate when the binary is missing. Add unit tests covering the primary, profile-fallback, triple-fallback, and missing-binary paths, and document the locator and the snapshot-redaction policy in the developers' guide and the insta guide. --- docs/developers-guide.md | 28 +++ ...snapshot-testing-in-netsuke-using-insta.md | 21 ++ src/diagnostic_json_tests.rs | 10 +- test_support/src/fs.rs | 28 +++ test_support/src/netsuke.rs | 210 +++++++++++++++--- 5 files changed, 265 insertions(+), 32 deletions(-) diff --git a/docs/developers-guide.md b/docs/developers-guide.md index d433e67d2..84f1a2cb4 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -2198,6 +2198,34 @@ only an empty `PATH`, isolated `HOME` and `XDG_CONFIG_HOME` values, plus the variables supplied in `extra_env`. Use it for configuration-layering tests or any scenario that requires a hermetic child environment. +#### Locating the netsuke binary + +Both `run_netsuke_in` and `run_netsuke_in_with_env` depend on a private +locator, `netsuke_executable()`, to find the built `netsuke` binary. +`netsuke_executable()` converts `std::env::current_exe()` to a +`camino::Utf8PathBuf` and delegates to `netsuke_executable_from`, which takes +an injected `mockable::Env` — the same injectable-environment pattern used by +`compile_rust_helper_with_env` in `command_helper.rs` — so the lookup logic is +unit-testable with `MockEnv` rather than depending on the real process +environment. + +The locator checks candidate paths in order: + +1. beside the test executable, using its directory with any trailing `deps` + component stripped; +2. `CARGO_TARGET_DIR//`, needed when Cargo's `build.build-dir` + configuration splits intermediate artefacts — where test executables run — + from the uplifted binary, which lands under the target directory; +3. `CARGO_TARGET_DIR///`, for `--target` builds where the + profile directory nests under the target triple. + +Filesystem errors other than "not found" are surfaced rather than treated as +a missing candidate. When every candidate misses, the resulting error lists +all attempted paths. + +The locator's unit tests live in `test_support/src/netsuke.rs` and cover the +primary lookup, both fallback paths, and the missing-binary case. + ## Digest rendering `src/hex.rs` (`netsuke::hex`) is the single owner of lowercase hexadecimal diff --git a/docs/snapshot-testing-in-netsuke-using-insta.md b/docs/snapshot-testing-in-netsuke-using-insta.md index 17bda652b..2b5dff258 100644 --- a/docs/snapshot-testing-in-netsuke-using-insta.md +++ b/docs/snapshot-testing-in-netsuke-using-insta.md @@ -276,6 +276,27 @@ function, serializing locale state across the test suite. - `test_support::set_en_localizer()` — installs `en-US` as the active locale and returns a `LocalizerGuard`. +## Redacting the generator version in diagnostic JSON snapshots + +The diagnostics JSON document embeds the generator's crate version. Left +unredacted, that field changes on every version bump and would churn every +diagnostic-JSON snapshot — this is exactly what happened on the v0.1.0-beta1 +bump. + +The shared `snapshot_settings()` helper in `src/diagnostic_json_tests.rs` +adds an insta filter that rewrites the generator's version to `[version]`. +The filter anchors on the preceding `"name": "netsuke"` line, so it redacts +only the generator block's version; any other field named `version` +elsewhere in a diagnostic document remains visible in snapshot diffs. + +New diagnostic-JSON snapshot tests must bind through this shared +`snapshot_settings()` helper rather than asserting raw output, so the +redaction is applied consistently. + +`schema_version` and the generator name are deliberately excluded from this +redaction: they are asserted structurally in dedicated tests, separate from +the redacted version string. + ## Running and Updating Snapshot Tests > In this repository the canonical runner is cargo-nextest: `make test`, or diff --git a/src/diagnostic_json_tests.rs b/src/diagnostic_json_tests.rs index da9c8c805..75474cbaf 100644 --- a/src/diagnostic_json_tests.rs +++ b/src/diagnostic_json_tests.rs @@ -21,14 +21,20 @@ fn parse_json_value(document: &str) -> Result { /// Builds insta [`Settings`] pointing at the `src/snapshots/diagnostic_json` directory. /// -/// The generator version is redacted so snapshots survive version bumps. +/// The generator version is redacted so snapshots survive version bumps. The +/// filter anchors on the preceding `"name": "netsuke"` line so only the +/// generator's version is redacted; any other `version` field stays +/// detectable in snapshot diffs. fn snapshot_settings() -> Settings { let mut settings = Settings::new(); settings.set_snapshot_path(concat!( env!("CARGO_MANIFEST_DIR"), "/src/snapshots/diagnostic_json" )); - settings.add_filter(r#""version": "[^"]+""#, r#""version": "[version]""#); + settings.add_filter( + r#"("name": "netsuke",\s*\n\s*"version": ")[^"]+(")"#, + r"${1}[version]${2}", + ); settings } diff --git a/test_support/src/fs.rs b/test_support/src/fs.rs index aec32022b..77fc9e2cf 100644 --- a/test_support/src/fs.rs +++ b/test_support/src/fs.rs @@ -117,6 +117,34 @@ pub fn is_dir(path: impl AsRef) -> bool { fs::metadata(path).is_ok_and(|metadata| metadata.is_dir()) } +/// Return `true` when `path` is a regular file, surfacing unexpected errors. +/// +/// Unlike `Path::is_file`, an I/O failure other than `NotFound` is propagated +/// rather than reported as `false`, so callers can distinguish "the file is +/// absent" from "the file could not be inspected". +/// +/// # Errors +/// +/// Propagates the underlying metadata failure for any error other than +/// `NotFound`. +/// +/// # Examples +/// +/// ``` +/// let dir = tempfile::tempdir().expect("create tempdir"); +/// let file = dir.path().join("file"); +/// test_support::fs::write(&file, "contents").expect("write file"); +/// assert!(test_support::fs::try_is_file(&file).expect("inspect file")); +/// assert!(!test_support::fs::try_is_file(dir.path().join("absent")).expect("inspect absent")); +/// ``` +pub fn try_is_file(path: impl AsRef) -> io::Result { + match fs::metadata(path) { + Ok(metadata) => Ok(metadata.is_file()), + Err(error) if error.kind() == io::ErrorKind::NotFound => Ok(false), + Err(error) => Err(error), + } +} + /// Copy `from` to `to`, returning the number of bytes copied. /// /// # Errors diff --git a/test_support/src/netsuke.rs b/test_support/src/netsuke.rs index 2ee2424ee..08f024dc7 100644 --- a/test_support/src/netsuke.rs +++ b/test_support/src/netsuke.rs @@ -4,44 +4,79 @@ //! `netsuke` executable and run it in a controlled working directory, //! capturing stdout/stderr for assertions. -use anyhow::{Context, Result, ensure}; +use anyhow::{Context, Result, bail}; +use camino::{Utf8Path, Utf8PathBuf}; use mockable::{DefaultEnv, Env}; use std::path::Path; -use std::path::PathBuf; /// Locate the built `netsuke` executable for integration-style tests. /// -/// Derive the path from the current test executable's target directory. When -/// Cargo's `build.build-dir` splits intermediate artefacts from final ones, -/// test executables run from the build dir while the uplifted binary lands -/// under `CARGO_TARGET_DIR`, so fall back to that location. -fn netsuke_executable() -> Result { - let mut exe_dir = std::env::current_exe().context("locate current test executable")?; - exe_dir.pop(); - if exe_dir.ends_with("deps") { - exe_dir.pop(); +/// Derive the primary path from the current test executable's directory, then +/// fall back to `CARGO_TARGET_DIR` when Cargo's `build.build-dir` splits +/// intermediate artefacts from final ones: test executables then run from the +/// build dir while the uplifted binary lands under the target dir. +fn netsuke_executable() -> Result { + let raw_exe = std::env::current_exe().context("locate current test executable")?; + let current_exe = Utf8PathBuf::from_path_buf(raw_exe) + .map_err(|path| anyhow::anyhow!("test executable path {} is not UTF-8", path.display()))?; + netsuke_executable_from(&DefaultEnv, ¤t_exe) +} + +/// Locate the `netsuke` binary from an injected environment and test path. +/// +/// Candidates are checked in order: +/// 1. beside the test executable (its directory, minus a trailing `deps`); +/// 2. `CARGO_TARGET_DIR//` for split `build.build-dir` layouts; +/// 3. `CARGO_TARGET_DIR///` for `--target` builds, where the +/// profile directory nests under the target triple. +/// +/// Filesystem errors other than "not found" are surfaced rather than treated +/// as a missing candidate. +fn netsuke_executable_from(env: &impl Env, current_exe: &Utf8Path) -> Result { + let mut exe_dir = current_exe + .parent() + .context("test executable should have a parent directory")?; + if exe_dir.file_name() == Some("deps") { + exe_dir = exe_dir + .parent() + .context("deps directory should have a parent")?; } let binary_name = format!("netsuke{}", std::env::consts::EXE_SUFFIX); - let path = exe_dir.join(&binary_name); - if path.is_file() { - return Ok(path); - } - if let (Some(target_dir), Some(profile)) = ( - DefaultEnv.os_string("CARGO_TARGET_DIR"), - exe_dir.file_name(), - ) { - let fallback = Path::new(&target_dir).join(profile).join(&binary_name); - if fallback.is_file() { - return Ok(fallback); + let candidates = candidate_paths(env, exe_dir, &binary_name); + for candidate in &candidates { + let is_file = crate::fs::try_is_file(candidate) + .with_context(|| format!("inspect candidate netsuke binary at {candidate}"))?; + if is_file { + return Ok(candidate.clone()); } } - ensure!( - path.is_file(), - "locate netsuke binary at {}", - path.display() - ); - Ok(path) + let attempted = candidates + .iter() + .map(|candidate| candidate.as_str()) + .collect::>() + .join(", "); + bail!("locate netsuke binary; tried: {attempted}"); +} + +/// Build the ordered candidate paths for the `netsuke` binary. +fn candidate_paths(env: &impl Env, exe_dir: &Utf8Path, binary_name: &str) -> Vec { + let mut candidates = vec![exe_dir.join(binary_name)]; + let (Some(target_dir), Some(profile)) = (env.string("CARGO_TARGET_DIR"), exe_dir.file_name()) + else { + return candidates; + }; + let target_root = Utf8PathBuf::from(target_dir); + candidates.push(target_root.join(profile).join(binary_name)); + // `--target` builds nest the profile directory under the target triple in + // both the build dir and the target dir, so reinsert the component above + // the profile when one exists. For no-`--target` builds that component is + // the build-dir root, which never exists under the target dir, so the + // extra candidate is harmless. + if let Some(triple) = exe_dir.parent().and_then(Utf8Path::file_name) { + candidates.push(target_root.join(triple).join(profile).join(binary_name)); + } + candidates } /// Captured output from a `netsuke` invocation. @@ -67,7 +102,8 @@ pub struct NetsukeRun { /// spawned. pub fn run_netsuke_in(current_dir: &Path, args: &[&str]) -> Result { let isolated_config_home = current_dir.join(".config"); - let mut cmd = assert_cmd::Command::new(netsuke_executable()?); + let executable = netsuke_executable()?; + let mut cmd = assert_cmd::Command::new(executable.as_std_path()); let output = cmd .current_dir(current_dir) .env("PATH", "") @@ -102,7 +138,8 @@ pub fn run_netsuke_in_with_env( args: &[&str], extra_env: &[(&str, &str)], ) -> Result { - let mut cmd = assert_cmd::Command::new(netsuke_executable()?); + let executable = netsuke_executable()?; + let mut cmd = assert_cmd::Command::new(executable.as_std_path()); let isolated_config_home = current_dir.join(".config"); let isolated_path = tempfile::tempdir().context("create isolated executable directory")?; cmd.current_dir(current_dir) @@ -120,3 +157,116 @@ pub fn run_netsuke_in_with_env( success: output.status.success(), }) } + +#[cfg(test)] +mod tests { + //! Unit tests for the netsuke binary locator. + + use super::netsuke_executable_from; + use anyhow::{Context, Result, ensure}; + use camino::{Utf8Path, Utf8PathBuf}; + use mockable::MockEnv; + + fn utf8_root(temp: &tempfile::TempDir) -> Result { + Utf8PathBuf::from_path_buf(temp.path().to_path_buf()) + .map_err(|path| anyhow::anyhow!("temp dir {} is not UTF-8", path.display())) + } + + fn binary_name() -> String { + format!("netsuke{}", std::env::consts::EXE_SUFFIX) + } + + fn touch(path: &Utf8Path) -> Result<()> { + let parent = path.parent().context("path should have a parent")?; + crate::fs::create_dir_all(parent).with_context(|| format!("create {parent}"))?; + crate::fs::write(path, b"stub").with_context(|| format!("write {path}"))?; + Ok(()) + } + + fn env_with_target_dir(target_dir: Option<&Utf8Path>) -> MockEnv { + let mut env = MockEnv::new(); + let value = target_dir.map(Utf8Path::to_string); + env.expect_string() + .withf(|key| key == "CARGO_TARGET_DIR") + .return_const(value); + env + } + + #[test] + fn locates_binary_beside_the_test_executable() -> Result<()> { + let temp = tempfile::tempdir().context("create temp dir")?; + let root = utf8_root(&temp)?; + let exe = root.join("build/debug/deps/test-exe"); + touch(&exe)?; + let binary = root.join("build/debug").join(binary_name()); + touch(&binary)?; + + let located = netsuke_executable_from(&env_with_target_dir(None), &exe)?; + ensure!( + located == binary, + "primary lookup should win; got {located}" + ); + Ok(()) + } + + #[test] + fn falls_back_to_cargo_target_dir_profile() -> Result<()> { + let temp = tempfile::tempdir().context("create temp dir")?; + let root = utf8_root(&temp)?; + let exe = root.join("build/debug/deps/test-exe"); + touch(&exe)?; + let target_dir = root.join("target"); + let binary = target_dir.join("debug").join(binary_name()); + touch(&binary)?; + + let located = netsuke_executable_from(&env_with_target_dir(Some(&target_dir)), &exe)?; + ensure!( + located == binary, + "profile fallback should resolve; got {located}" + ); + Ok(()) + } + + #[test] + fn falls_back_to_target_triple_directory() -> Result<()> { + let temp = tempfile::tempdir().context("create temp dir")?; + let root = utf8_root(&temp)?; + let exe = root.join("build/x86_64-unknown-linux-gnu/debug/deps/test-exe"); + touch(&exe)?; + let target_dir = root.join("target"); + let binary = target_dir + .join("x86_64-unknown-linux-gnu/debug") + .join(binary_name()); + touch(&binary)?; + + let located = netsuke_executable_from(&env_with_target_dir(Some(&target_dir)), &exe)?; + ensure!( + located == binary, + "triple fallback should resolve; got {located}" + ); + Ok(()) + } + + #[test] + fn reports_every_attempted_candidate_when_missing() -> Result<()> { + let temp = tempfile::tempdir().context("create temp dir")?; + let root = utf8_root(&temp)?; + let exe = root.join("build/debug/deps/test-exe"); + touch(&exe)?; + let target_dir = root.join("target"); + + let error = netsuke_executable_from(&env_with_target_dir(Some(&target_dir)), &exe) + .expect_err("no candidate exists"); + let message = error.to_string(); + for expected in [ + root.join("build/debug").join(binary_name()), + target_dir.join("debug").join(binary_name()), + ] { + ensure!( + message.contains(expected.as_str()), + "error should list attempted candidate {expected}; got: {message}" + ); + } + Ok(()) + } +} From f4c77d55bb51655f00ed9418f0e46729f65f54e0 Mon Sep 17 00:00:00 2001 From: Payton McIntosh Date: Thu, 6 Aug 2026 01:04:57 +0100 Subject: [PATCH 4/5] Deduplicate locator test scenarios behind a shared helper The three happy-path locator tests repeated the same stage-and-assert choreography. Extract an `assert_locates` helper that stages the scenario from root-relative paths (test executable, optional `CARGO_TARGET_DIR`, expected binary) and asserts the resolved path, keeping each test to a single scenario-specific call. Coverage, lookup order, and the missing-candidate test are unchanged. --- test_support/src/netsuke.rs | 79 +++++++++++++++++++------------------ 1 file changed, 40 insertions(+), 39 deletions(-) diff --git a/test_support/src/netsuke.rs b/test_support/src/netsuke.rs index 08f024dc7..7620a3fe5 100644 --- a/test_support/src/netsuke.rs +++ b/test_support/src/netsuke.rs @@ -192,59 +192,60 @@ mod tests { env } - #[test] - fn locates_binary_beside_the_test_executable() -> Result<()> { + /// Stage a locator scenario and assert the resolved binary path. + /// + /// Creates the temporary root, touches the test executable at `exe_rel` + /// and the expected binary at `binary_rel`, configures the mock + /// environment with `target_dir_rel` when supplied (all three paths are + /// relative to the root), and asserts that the locator resolves the + /// expected binary, retaining `message` in the diagnostic. + fn assert_locates( + exe_rel: &str, + target_dir_rel: Option<&str>, + binary_rel: &str, + message: &str, + ) -> Result<()> { let temp = tempfile::tempdir().context("create temp dir")?; let root = utf8_root(&temp)?; - let exe = root.join("build/debug/deps/test-exe"); + let exe = root.join(exe_rel); touch(&exe)?; - let binary = root.join("build/debug").join(binary_name()); + let binary = root.join(binary_rel); touch(&binary)?; + let target_dir = target_dir_rel.map(|rel| root.join(rel)); - let located = netsuke_executable_from(&env_with_target_dir(None), &exe)?; - ensure!( - located == binary, - "primary lookup should win; got {located}" - ); + let located = netsuke_executable_from(&env_with_target_dir(target_dir.as_deref()), &exe)?; + ensure!(located == binary, "{message}; got {located}"); Ok(()) } #[test] - fn falls_back_to_cargo_target_dir_profile() -> Result<()> { - let temp = tempfile::tempdir().context("create temp dir")?; - let root = utf8_root(&temp)?; - let exe = root.join("build/debug/deps/test-exe"); - touch(&exe)?; - let target_dir = root.join("target"); - let binary = target_dir.join("debug").join(binary_name()); - touch(&binary)?; + fn locates_binary_beside_the_test_executable() -> Result<()> { + assert_locates( + "build/debug/deps/test-exe", + None, + &format!("build/debug/{}", binary_name()), + "primary lookup should win", + ) + } - let located = netsuke_executable_from(&env_with_target_dir(Some(&target_dir)), &exe)?; - ensure!( - located == binary, - "profile fallback should resolve; got {located}" - ); - Ok(()) + #[test] + fn falls_back_to_cargo_target_dir_profile() -> Result<()> { + assert_locates( + "build/debug/deps/test-exe", + Some("target"), + &format!("target/debug/{}", binary_name()), + "profile fallback should resolve", + ) } #[test] fn falls_back_to_target_triple_directory() -> Result<()> { - let temp = tempfile::tempdir().context("create temp dir")?; - let root = utf8_root(&temp)?; - let exe = root.join("build/x86_64-unknown-linux-gnu/debug/deps/test-exe"); - touch(&exe)?; - let target_dir = root.join("target"); - let binary = target_dir - .join("x86_64-unknown-linux-gnu/debug") - .join(binary_name()); - touch(&binary)?; - - let located = netsuke_executable_from(&env_with_target_dir(Some(&target_dir)), &exe)?; - ensure!( - located == binary, - "triple fallback should resolve; got {located}" - ); - Ok(()) + assert_locates( + "build/x86_64-unknown-linux-gnu/debug/deps/test-exe", + Some("target"), + &format!("target/x86_64-unknown-linux-gnu/debug/{}", binary_name()), + "triple fallback should resolve", + ) } #[test] From ca2b00f62057fd27fb2278bff0b360eda324eca7 Mon Sep 17 00:00:00 2001 From: Payton McIntosh Date: Thu, 6 Aug 2026 01:21:09 +0100 Subject: [PATCH 5/5] Harden locator test coverage and split the docs contract tests Address the third review round: - Assert every candidate the locator generates in the missing-binary test, including the `CARGO_TARGET_DIR///` path. - Add a both-present priority test so inverting the candidate order can no longer pass unnoticed. - Cover `test_support::fs::try_is_file` directly: a directory reports `Ok(false)`, and traversal through a regular file propagates the error rather than being folded into absence. - Pass the located `Utf8PathBuf` straight to `assert_cmd::Command::new`, completing the camino migration. - Split the installation and release-contract tests (149 lines with no entanglement) into `tests/documentation_installation_tests.rs`, bringing `documentation_examples_tests.rs` from 505 lines to 355, under the repository's 400-line file cap. The new binary reuses the shared `documentation_examples` directory module via the same `pub mod` pattern as the loader tests; behaviour is unchanged. - Document `try_is_file` in the developers' guide under `test_support::fs`, with a cross-reference from the binary-locator section. --- docs/developers-guide.md | 13 +- test_support/src/fs.rs | 32 ++++- test_support/src/netsuke.rs | 25 +++- tests/documentation_examples_tests.rs | 150 -------------------- tests/documentation_installation_tests.rs | 163 ++++++++++++++++++++++ 5 files changed, 227 insertions(+), 156 deletions(-) create mode 100644 tests/documentation_installation_tests.rs diff --git a/docs/developers-guide.md b/docs/developers-guide.md index 84f1a2cb4..9ed87131b 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -1472,6 +1472,14 @@ is not obvious from the name: `ensure_manifest_exists` uses it both to reject a directory where a manifest file is expected, and to accept a destination directory that is already present. +- `try_is_file(path) -> io::Result` is the fallible counterpart to the + boolean predicates: `Ok(true)` when the path is a regular file, `Ok(false)` + when it is absent (`NotFound` is folded into the boolean result), and + `Err` for any other metadata failure, so callers can distinguish absence + from inaccessibility. The binary locator in `test_support/src/netsuke.rs` + (`netsuke_executable_from`, see + [Locating the netsuke binary](#locating-the-netsuke-binary)) relies on it to + surface unexpected filesystem errors while probing candidate paths. - `is_executable_file(path) -> bool` (Unix only) is `true` when the path is a regular file with any execute bit set, and `false` for an absent or unreadable path. It is the inverse of `set_mode`, and exists for probing a @@ -2220,8 +2228,9 @@ The locator checks candidate paths in order: profile directory nests under the target triple. Filesystem errors other than "not found" are surfaced rather than treated as -a missing candidate. When every candidate misses, the resulting error lists -all attempted paths. +a missing candidate, via the [`test_support::fs`](#test_supportfs) wrapper +`try_is_file`. When every candidate misses, the resulting error lists all +attempted paths. The locator's unit tests live in `test_support/src/netsuke.rs` and cover the primary lookup, both fallback paths, and the missing-binary case. diff --git a/test_support/src/fs.rs b/test_support/src/fs.rs index 77fc9e2cf..c0886cbc4 100644 --- a/test_support/src/fs.rs +++ b/test_support/src/fs.rs @@ -311,11 +311,39 @@ pub fn symlink(target: impl AsRef, link: impl AsRef) -> io::Result<( #[cfg(test)] mod tests { - //! Coverage for directory creation at existing-path boundaries. + //! Coverage for directory creation at existing-path boundaries and the + //! `try_is_file` error contract. - use super::{create_dir_all, write}; + use super::{create_dir_all, try_is_file, write}; use std::io; + #[test] + fn try_is_file_reports_a_directory_as_not_a_file() -> anyhow::Result<()> { + let temp = tempfile::tempdir()?; + + anyhow::ensure!( + !try_is_file(temp.path())?, + "a directory should not be reported as a regular file" + ); + Ok(()) + } + + #[test] + fn try_is_file_propagates_errors_other_than_not_found() -> anyhow::Result<()> { + let temp = tempfile::tempdir()?; + let file = temp.path().join("regular-file"); + write(&file, b"fixture")?; + + let Err(error) = try_is_file(file.join("child")) else { + anyhow::bail!("traversing through a regular file should fail"); + }; + anyhow::ensure!( + error.kind() != io::ErrorKind::NotFound, + "traversal through a file should not be reported as absence, got {error:?}" + ); + Ok(()) + } + #[test] fn create_dir_all_accepts_an_existing_directory() -> io::Result<()> { let temp = tempfile::tempdir()?; diff --git a/test_support/src/netsuke.rs b/test_support/src/netsuke.rs index 7620a3fe5..8d062969a 100644 --- a/test_support/src/netsuke.rs +++ b/test_support/src/netsuke.rs @@ -103,7 +103,7 @@ pub struct NetsukeRun { pub fn run_netsuke_in(current_dir: &Path, args: &[&str]) -> Result { let isolated_config_home = current_dir.join(".config"); let executable = netsuke_executable()?; - let mut cmd = assert_cmd::Command::new(executable.as_std_path()); + let mut cmd = assert_cmd::Command::new(executable); let output = cmd .current_dir(current_dir) .env("PATH", "") @@ -139,7 +139,7 @@ pub fn run_netsuke_in_with_env( extra_env: &[(&str, &str)], ) -> Result { let executable = netsuke_executable()?; - let mut cmd = assert_cmd::Command::new(executable.as_std_path()); + let mut cmd = assert_cmd::Command::new(executable); let isolated_config_home = current_dir.join(".config"); let isolated_path = tempfile::tempdir().context("create isolated executable directory")?; cmd.current_dir(current_dir) @@ -248,6 +248,26 @@ mod tests { ) } + #[test] + fn prefers_the_primary_candidate_when_fallback_also_exists() -> Result<()> { + let temp = tempfile::tempdir().context("create temp dir")?; + let root = utf8_root(&temp)?; + let exe = root.join("build/debug/deps/test-exe"); + touch(&exe)?; + let primary = root.join("build/debug").join(binary_name()); + touch(&primary)?; + let target_dir = root.join("target"); + let fallback = target_dir.join("debug").join(binary_name()); + touch(&fallback)?; + + let located = netsuke_executable_from(&env_with_target_dir(Some(&target_dir)), &exe)?; + ensure!( + located == primary, + "the primary candidate should win over the fallback; got {located}" + ); + Ok(()) + } + #[test] fn reports_every_attempted_candidate_when_missing() -> Result<()> { let temp = tempfile::tempdir().context("create temp dir")?; @@ -262,6 +282,7 @@ mod tests { for expected in [ root.join("build/debug").join(binary_name()), target_dir.join("debug").join(binary_name()), + target_dir.join("build/debug").join(binary_name()), ] { ensure!( message.contains(expected.as_str()), diff --git a/tests/documentation_examples_tests.rs b/tests/documentation_examples_tests.rs index a78201428..64fc78446 100644 --- a/tests/documentation_examples_tests.rs +++ b/tests/documentation_examples_tests.rs @@ -174,156 +174,6 @@ fn documented_first_run_flow_builds( Ok(()) } -#[test] -fn installation_examples_match_source_and_release_contracts() -> Result<()> { - assert_release_installation_contract()?; - let readme = documented_example("readme-source-install")?; - let guide = documented_example("guide-source-install")?; - let expected = concat!( - "git clone https://github.com/leynos/netsuke.git\n", - "cd netsuke\n", - "cargo install --path .\n" - ); - ensure!(readme.body == expected, "README source install drifted"); - ensure!(guide.body == expected, "user guide source install drifted"); - assert_windows_setup_examples() -} - -#[test] -fn registry_install_examples_pin_toolchain_and_polonius() -> Result<()> { - // Registry installs build outside a checkout, where neither - // rust-toolchain.toml nor .cargo/config.toml applies, so every tagged - // example installing from crates.io must select the pinned nightly and - // pass the Polonius flag itself. `cargo binstall` fetches a prebuilt - // binary and `cargo install --path .` runs inside a checkout, so both - // are exempt. - let mut registry_install_ids = Vec::new(); - for example in load_documented_examples()? { - for line in example.body.lines() { - if !line.contains("install netsuke-build") || line.contains("binstall") { - continue; - } - ensure!( - line.contains("cargo +nightly-2026-06-25 install netsuke-build"), - "{id} must install with the pinned nightly toolchain: {line}", - id = example.id - ); - ensure!( - line.contains("RUSTFLAGS=-Zpolonius=next"), - "{id} must pass the Polonius borrow-checker flag: {line}", - id = example.id - ); - registry_install_ids.push(example.id.clone()); - } - } - ensure!( - registry_install_ids.len() >= 2, - "expected registry-install examples in the README and users' guide, found {registry_install_ids:?}" - ); - // The quickstart carries no tested-example fences, so guard its prose - // against reintroducing an unsupported bare registry install. - let quickstart = - test_fs::read_to_string("docs/quickstart.md").context("read docs/quickstart.md")?; - ensure!( - !quickstart.contains("cargo install netsuke-build"), - "docs/quickstart.md must defer to the users' guide install command" - ); - Ok(()) -} - -/// Check the documented crates.io install command and release details. -fn assert_release_installation_contract() -> Result<()> { - let readme_binstall = documented_example("readme-binstall-install")?; - let guide_binstall = documented_example("guide-binstall-install")?; - let expected_binstall = "cargo binstall netsuke-build\n"; - ensure!( - readme_binstall.body == expected_binstall, - "README binstall drifted" - ); - ensure!( - guide_binstall.body == expected_binstall, - "user guide binstall drifted" - ); - let readme_release = documented_example("readme-crates-io-install")?; - let guide_release = documented_example("guide-crates-io-install")?; - // Registry installs run outside a checkout, so the packaged source sees - // neither rust-toolchain.toml nor .cargo/config.toml; the documented - // command must select the pinned nightly and the Polonius flag itself. - let expected_release = concat!( - "rustup toolchain install nightly-2026-06-25\n", - "RUSTFLAGS=-Zpolonius=next cargo +nightly-2026-06-25 install netsuke-build\n" - ); - ensure!(readme_release.body == expected_release, "README drifted"); - ensure!(guide_release.body == expected_release, "user guide drifted"); - // Derive version literals from the crate version so docs contracts track - // release bumps instead of drifting behind them. - let expected_release_details = [ - concat!( - "https://github.com/leynos/netsuke/releases/tag/v", - env!("CARGO_PKG_VERSION") - ), - "Debian (`.deb`) and RPM (`.rpm`)", - "Installer package (`.pkg`)", - "Windows Installer (`.msi`)", - "x86-64 (`amd64`) and Arm64 (`arm64`)", - "Installer packages do not have checksum", - "The Windows MSI installs to `C:\\Program Files\\netsuke`", - ]; - for path in ["README.md", "docs/users-guide.md"] { - let document = test_fs::read_to_string(path).with_context(|| format!("read {path}"))?; - for expected in expected_release_details { - ensure!( - document.contains(expected), - "{path} should document v{} release detail: {expected}", - env!("CARGO_PKG_VERSION") - ); - } - } - Ok(()) -} - -/// Check the documented Windows help, PATH, and staging contracts. -fn assert_windows_setup_examples() -> Result<()> { - let windows = documented_example("guide-windows-help")?; - ensure!(windows.body == "Get-Help Netsuke -Full\n", "help drifted"); - let windows_path = documented_example("guide-windows-path")?; - let windows_path_fragments = [ - "SetEnvironmentVariable", - "$netsukeDirectory", - "SetEnvironmentVariable('Path', $newUserPath, 'User')", - ]; - ensure!( - windows_path_fragments - .into_iter() - .all(|fragment| windows_path.body.contains(fragment)), - "Windows PATH setup should persist the MSI installation directory" - ); - let windows_help_install = documented_example("guide-windows-help-install")?; - let windows_help_fragments = [ - "Import-Module", - concat!( - "$moduleDirectory = Join-Path $moduleRoot 'Netsuke\\", - env!("CARGO_PKG_VERSION"), - "'" - ), - "Import-Module (Join-Path $moduleDirectory 'Netsuke.psd1')", - "*windows-$architecture*", - ]; - ensure!( - windows_help_fragments - .into_iter() - .all(|fragment| windows_help_install.body.contains(fragment)), - "Windows help setup should import the downloaded sidecars" - ); - let staging = test_fs::read_to_string(".github/release-staging.toml") - .context("read release staging configuration")?; - ensure!( - staging.contains("Netsuke-help.xml") && staging.contains("about_Netsuke.help.txt"), - "Windows release should stage the help consumed by Get-Help" - ); - Ok(()) -} - #[test] fn documented_cli_shape_matches_live_help() -> Result<()> { let example = documented_example("guide-cli-usage")?; diff --git a/tests/documentation_installation_tests.rs b/tests/documentation_installation_tests.rs new file mode 100644 index 000000000..102e77747 --- /dev/null +++ b/tests/documentation_installation_tests.rs @@ -0,0 +1,163 @@ +//! Executable contracts for the installation and release examples in the +//! public user documentation. +//! +//! Split from `documentation_examples_tests.rs` to keep that file within the +//! repository's 400-line cap; the shared fixtures live in the +//! `documentation_examples` directory module, which every documentation test +//! binary declares independently. + +pub mod documentation_examples; + +use anyhow::{Context, Result, ensure}; +use documentation_examples::{documented_example, load_documented_examples}; +use test_support::fs as test_fs; + +#[test] +fn installation_examples_match_source_and_release_contracts() -> Result<()> { + assert_release_installation_contract()?; + let readme = documented_example("readme-source-install")?; + let guide = documented_example("guide-source-install")?; + let expected = concat!( + "git clone https://github.com/leynos/netsuke.git\n", + "cd netsuke\n", + "cargo install --path .\n" + ); + ensure!(readme.body == expected, "README source install drifted"); + ensure!(guide.body == expected, "user guide source install drifted"); + assert_windows_setup_examples() +} + +#[test] +fn registry_install_examples_pin_toolchain_and_polonius() -> Result<()> { + // Registry installs build outside a checkout, where neither + // rust-toolchain.toml nor .cargo/config.toml applies, so every tagged + // example installing from crates.io must select the pinned nightly and + // pass the Polonius flag itself. `cargo binstall` fetches a prebuilt + // binary and `cargo install --path .` runs inside a checkout, so both + // are exempt. + let mut registry_install_ids = Vec::new(); + for example in load_documented_examples()? { + for line in example.body.lines() { + if !line.contains("install netsuke-build") || line.contains("binstall") { + continue; + } + ensure!( + line.contains("cargo +nightly-2026-06-25 install netsuke-build"), + "{id} must install with the pinned nightly toolchain: {line}", + id = example.id + ); + ensure!( + line.contains("RUSTFLAGS=-Zpolonius=next"), + "{id} must pass the Polonius borrow-checker flag: {line}", + id = example.id + ); + registry_install_ids.push(example.id.clone()); + } + } + ensure!( + registry_install_ids.len() >= 2, + "expected registry-install examples in the README and users' guide, found {registry_install_ids:?}" + ); + // The quickstart carries no tested-example fences, so guard its prose + // against reintroducing an unsupported bare registry install. + let quickstart = + test_fs::read_to_string("docs/quickstart.md").context("read docs/quickstart.md")?; + ensure!( + !quickstart.contains("cargo install netsuke-build"), + "docs/quickstart.md must defer to the users' guide install command" + ); + Ok(()) +} + +/// Check the documented crates.io install command and release details. +fn assert_release_installation_contract() -> Result<()> { + let readme_binstall = documented_example("readme-binstall-install")?; + let guide_binstall = documented_example("guide-binstall-install")?; + let expected_binstall = "cargo binstall netsuke-build\n"; + ensure!( + readme_binstall.body == expected_binstall, + "README binstall drifted" + ); + ensure!( + guide_binstall.body == expected_binstall, + "user guide binstall drifted" + ); + let readme_release = documented_example("readme-crates-io-install")?; + let guide_release = documented_example("guide-crates-io-install")?; + // Registry installs run outside a checkout, so the packaged source sees + // neither rust-toolchain.toml nor .cargo/config.toml; the documented + // command must select the pinned nightly and the Polonius flag itself. + let expected_release = concat!( + "rustup toolchain install nightly-2026-06-25\n", + "RUSTFLAGS=-Zpolonius=next cargo +nightly-2026-06-25 install netsuke-build\n" + ); + ensure!(readme_release.body == expected_release, "README drifted"); + ensure!(guide_release.body == expected_release, "user guide drifted"); + // Derive version literals from the crate version so docs contracts track + // release bumps instead of drifting behind them. + let expected_release_details = [ + concat!( + "https://github.com/leynos/netsuke/releases/tag/v", + env!("CARGO_PKG_VERSION") + ), + "Debian (`.deb`) and RPM (`.rpm`)", + "Installer package (`.pkg`)", + "Windows Installer (`.msi`)", + "x86-64 (`amd64`) and Arm64 (`arm64`)", + "Installer packages do not have checksum", + "The Windows MSI installs to `C:\\Program Files\\netsuke`", + ]; + for path in ["README.md", "docs/users-guide.md"] { + let document = test_fs::read_to_string(path).with_context(|| format!("read {path}"))?; + for expected in expected_release_details { + ensure!( + document.contains(expected), + "{path} should document v{} release detail: {expected}", + env!("CARGO_PKG_VERSION") + ); + } + } + Ok(()) +} + +/// Check the documented Windows help, PATH, and staging contracts. +fn assert_windows_setup_examples() -> Result<()> { + let windows = documented_example("guide-windows-help")?; + ensure!(windows.body == "Get-Help Netsuke -Full\n", "help drifted"); + let windows_path = documented_example("guide-windows-path")?; + let windows_path_fragments = [ + "SetEnvironmentVariable", + "$netsukeDirectory", + "SetEnvironmentVariable('Path', $newUserPath, 'User')", + ]; + ensure!( + windows_path_fragments + .into_iter() + .all(|fragment| windows_path.body.contains(fragment)), + "Windows PATH setup should persist the MSI installation directory" + ); + let windows_help_install = documented_example("guide-windows-help-install")?; + let windows_help_fragments = [ + "Import-Module", + concat!( + "$moduleDirectory = Join-Path $moduleRoot 'Netsuke\\", + env!("CARGO_PKG_VERSION"), + "'" + ), + "Import-Module (Join-Path $moduleDirectory 'Netsuke.psd1')", + "*windows-$architecture*", + ]; + ensure!( + windows_help_fragments + .into_iter() + .all(|fragment| windows_help_install.body.contains(fragment)), + "Windows help setup should import the downloaded sidecars" + ); + let staging = test_fs::read_to_string(".github/release-staging.toml") + .context("read release staging configuration")?; + ensure!( + staging.contains("Netsuke-help.xml") && staging.contains("about_Netsuke.help.txt"), + "Windows release should stage the help consumed by Get-Help" + ); + Ok(()) +}