Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
37 changes: 37 additions & 0 deletions docs/developers-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -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<bool>` 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
Expand Down Expand Up @@ -2198,6 +2206,35 @@ 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/<profile>/`, 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/<triple>/<profile>/`, 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, 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.

## Digest rendering

`src/hex.rs` (`netsuke::hex`) is the single owner of lowercase hexadecimal
Expand Down
21 changes: 21 additions & 0 deletions docs/snapshot-testing-in-netsuke-using-insta.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 9 additions & 0 deletions src/diagnostic_json_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,21 @@ fn parse_json_value(document: &str) -> Result<Value> {
}

/// Builds insta [`Settings`] pointing at the `src/snapshots/diagnostic_json` directory.
///
/// 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#"("name": "netsuke",\s*\n\s*"version": ")[^"]+(")"#,
r"${1}[version]${2}",
);
settings
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ expression: rendered
"schema_version": 1,
"generator": {
"name": "netsuke",
"version": "0.1.0"
"version": "[version]"
},
"diagnostics": [
{
Expand Down
Original file line number Diff line number Diff line change
@@ -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": [
{
Expand Down
60 changes: 58 additions & 2 deletions test_support/src/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,34 @@ pub fn is_dir(path: impl AsRef<Path>) -> 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<Path>) -> io::Result<bool> {
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
Expand Down Expand Up @@ -283,11 +311,39 @@ pub fn symlink(target: impl AsRef<Path>, link: impl AsRef<Path>) -> 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()?;
Expand Down
Loading
Loading