diff --git a/.gitignore b/.gitignore index a96b662..594ae6a 100644 --- a/.gitignore +++ b/.gitignore @@ -1,6 +1,7 @@ target/ **/*.rs.bk __pycache__/ +.coverage # Local spelling-policy caches and helper tool state .uv-cache/ diff --git a/Cargo.lock b/Cargo.lock index 21f894d..2ae7aeb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -151,6 +151,7 @@ dependencies = [ "serde_json", "sys-locale", "thiserror 2.0.19", + "trybuild", ] [[package]] @@ -294,7 +295,7 @@ version = "0.6.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "59a98bbaacea1c0eb6a0876280051b892eb73594fd90cf3b20e9c817029c57d2" dependencies = [ - "toml", + "toml 0.5.11", ] [[package]] @@ -1185,6 +1186,15 @@ dependencies = [ "zmij", ] +[[package]] +name = "serde_spanned" +version = "1.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6662b5879511e06e8999a8a235d848113e942c9124f211511b16466ee2995f26" +dependencies = [ + "serde_core", +] + [[package]] name = "sha2" version = "0.10.9" @@ -1261,6 +1271,12 @@ dependencies = [ "libc", ] +[[package]] +name = "target-triple" +version = "1.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c3a6bfce3d99adfa72d24750a61f782f3036a81e7f86d8841ee1326deaebd171" + [[package]] name = "tempfile" version = "3.27.0" @@ -1274,6 +1290,15 @@ dependencies = [ "windows-sys 0.61.2", ] +[[package]] +name = "termcolor" +version = "1.4.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "06794f8f6c5c898b3275aebefa6b8a1cb24cd2c6c79397ab15774837a0bc5755" +dependencies = [ + "winapi-util", +] + [[package]] name = "termtree" version = "0.5.1" @@ -1351,6 +1376,21 @@ dependencies = [ "serde", ] +[[package]] +name = "toml" +version = "1.1.4+spec-1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3aace63f4bbcdfc2c965b059de67119c89c4017a70d633be6c104910f67056f5" +dependencies = [ + "indexmap", + "serde_core", + "serde_spanned", + "toml_datetime", + "toml_parser", + "toml_writer", + "winnow", +] + [[package]] name = "toml_datetime" version = "1.1.1+spec-1.1.0" @@ -1374,13 +1414,34 @@ dependencies = [ [[package]] name = "toml_parser" -version = "1.1.2+spec-1.1.0" +version = "1.1.3+spec-1.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a2abe9b86193656635d2411dc43050282ca48aa31c2451210f4202550afb7526" +checksum = "1d38ac1cf9b95face32296c0a3ede1fdc270627c9d9c02a7274dd6d960dc4d56" dependencies = [ "winnow", ] +[[package]] +name = "toml_writer" +version = "1.1.2+spec-1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7d56353a2a665ad0f41a421187180aab746c8c325620617ad883a99a1cbe66d2" + +[[package]] +name = "trybuild" +version = "1.0.120" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1e605bf6b39357663d8ba4e984f8be8da8df6bb32e81031d6889024ea8fd68e4" +dependencies = [ + "glob", + "serde", + "serde_derive", + "serde_json", + "target-triple", + "termcolor", + "toml 1.1.4+spec-1.1.0", +] + [[package]] name = "type-map" version = "0.5.1" diff --git a/Cargo.toml b/Cargo.toml index 86054ef..acb47c8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -99,3 +99,4 @@ rstest = "0.26.1" rstest-bdd = "0.5.0" rstest-bdd-macros = "0.5.0" serde_json = "1.0.150" +trybuild = "1.0.120" diff --git a/docs/developers-guide.md b/docs/developers-guide.md index 7fe1b42..66b6128 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -50,6 +50,60 @@ The test suite covers the same behaviour from several angles: output. - End-to-end tests in `tests/e2e.rs` build and run the compiled binary with accelerated logical seconds. +- UI tests in `tests/ui/` compile against the public crate boundary, pin the + user-facing `Display` output of public error types, and pin the compiler + diagnostics that keep those error enums non-exhaustive. + +### Public error UI tests + +The `tests/ui.rs` harness uses `trybuild` in two complementary modes, each +covering what the other cannot. + +Pass fixtures, `tests/ui/*_display.rs`, are compiled and executed as external +crates. Pass mode is required for message text: Rust evaluates `Display` +implementations at runtime, so a compile-fail fixture can snapshot compiler +diagnostics but never observes an error value's formatted output. + +Compile-fail fixtures, `tests/ui/*_non_exhaustive.rs`, match every public +variant of an error enum without a wildcard arm. Each is expected to fail with +`E0004`, which pins `#[non_exhaustive]` on `CliError`, `DurationParseError`, +and `ClockConfigError`. That contract is what keeps adding an error variant a +non-breaking change for downstream crates. + +Run the focused harness with: + +```sh +cargo test --test ui +``` + +`make test` also discovers the harness and is the required pre-commit and CI +entrypoint. + +#### Updating display fixtures + +Treat each expected string literal in a display fixture as a UI snapshot. When +adding a public error type or variant, add an assertion with representative +field values to the corresponding fixture, or add a new `*_display.rs` file. If +an intentional wording change alters a message, update the expected literal in +the same commit and review the string diff deliberately. Display fixtures have +no adjacent `.stderr` file, so `TRYBUILD=overwrite` does not maintain them. + +#### Updating compile-fail snapshots + +Each `*_non_exhaustive.rs` fixture has an adjacent `.stderr` file holding the +expected diagnostic. Add every new variant to the fixture's `match`, then +regenerate the snapshot with: + +```sh +TRYBUILD=overwrite cargo test --test ui +``` + +Review the regenerated diagnostic before committing. Because the snapshots +capture compiler output, they are tied to the toolchain pinned in +`rust-toolchain.toml`; a toolchain bump that rewords `E0004` requires the same +regeneration step. A fixture that starts *passing* means the enum has lost +`#[non_exhaustive]`, which is a breaking change rather than a snapshot to +refresh. ## Spelling gate diff --git a/docs/execplans/initial-app.md b/docs/execplans/initial-app.md index 1951452..13a539c 100644 --- a/docs/execplans/initial-app.md +++ b/docs/execplans/initial-app.md @@ -120,6 +120,17 @@ test bodies rather than shared production paths. `make test`, `make markdownlint`, and `make nixie`. - [x] 2026-06-01: Committed the validated implementation as `10273a6`. - [x] 2026-06-01: Completion-audited the objective against the current tree. +- [x] 2026-08-08: Added the `trybuild` UI harness for issue + [#8](https://github.com/leynos/catnap/issues/8), pinning the public error + surface at the crate boundary. +- [x] 2026-08-08: Added pass-mode display fixtures for `CliError`, + `DurationParseError`, and `ClockConfigError`. +- [x] 2026-08-08: Added compile-fail fixtures with `.stderr` snapshots pinning + `#[non_exhaustive]` on all three public error enums. +- [x] 2026-08-08: Documented the UI test workflow in the developers' guide and + fixture ownership in the repository layout. +- [x] 2026-08-08: `make test` passed with 27 tests across six binaries, and + `cargo test --doc` passed with 14 doctests. Completed implementation checklist: @@ -131,6 +142,8 @@ Completed implementation checklist: - [x] Add snapshot tests using `insta`. - [x] Add end-to-end tests that use the hidden logical-second duration argument. +- [x] Add `trybuild` UI tests pinning the public error types' `Display` output + and non-exhaustive matching contract. ## Surprises & Discoveries @@ -152,8 +165,27 @@ build the binary in this configured split target/build directory. The e2e test therefore builds `vsleep` explicitly and reads `target_directory` from `cargo metadata` before running the compiled binary. +`trybuild` cannot express issue +[#8](https://github.com/leynos/catnap/issues/8) as written. The issue asks for +compile-fail tests covering `Display` output, but Rust evaluates `Display` at +runtime, so a compile-fail fixture only ever captures compiler diagnostics and +never sees a formatted message. Splitting the harness across both modes was the +only way to cover the whole request. + ## Decision Log +2026-08-08: Pin public error messages with `trybuild` pass fixtures rather than +compile-fail fixtures. Pass fixtures compile *and execute* as external crates, +so they exercise the real public boundary and assert the actual message text. +This is the only mode that can observe `Display` output. + +2026-08-08: Additionally pin `#[non_exhaustive]` on each public error enum with +compile-fail fixtures. Message text and API stability are separate guarantees: +the compile-fail snapshots catch the removal of `#[non_exhaustive]`, which would +silently make every later variant addition a breaking change for downstream +crates. The `.stderr` snapshots are tied to the toolchain pinned in +`rust-toolchain.toml` and are regenerated with `TRYBUILD=overwrite`. + 2026-06-01: Use `src/lib.rs` for reusable command, parsing, formatting, and clock logic, and keep `src/main.rs` as a thin process boundary. This makes unit, behavioural, snapshot, and e2e tests easier without putting business @@ -269,6 +301,22 @@ Completion audit on 2026-06-01: - Gates: satisfied by final cleaned-tree runs of `make check-fmt`, `make lint`, `make test`, `make markdownlint`, and `make nixie`. +Public error UI tests landed on 2026-08-08 for issue +[#8](https://github.com/leynos/catnap/issues/8): + +- `trybuild` integration: satisfied by the test-only dependency in `Cargo.toml` + and the `tests/ui.rs` harness, which standard Cargo test discovery runs. +- Display coverage: satisfied by `tests/ui/cli_error_display.rs`, + `tests/ui/duration_parse_error_display.rs`, and + `tests/ui/clock_config_error_display.rs`, which compile as external crates and + assert every public variant's message. +- Compile-fail coverage: satisfied by the `tests/ui/*_non_exhaustive.rs` + fixtures and their `.stderr` snapshots, which pin `E0004` for each public + error enum. +- Documentation: satisfied by the UI test sections of + [developers' guide](../developers-guide.md) and + [repository layout](../repository-layout.md). + Planning validation passed on 2026-06-01: ```plaintext @@ -330,3 +378,28 @@ Summary: 0 error(s) make nixie 2>&1 | tee /tmp/nixie-vsleep-initial-app-final2.out All diagrams validated successfully ``` + +Public error UI test validation passed on 2026-08-08: + +```plaintext +make check-fmt +cargo fmt --all -- --check + +make lint +Finished `dev` profile + +make typecheck +cargo check --all-targets --all-features + +make test +Summary [ ...] 27 tests run: 27 passed, 0 skipped + +make markdownlint +Summary: 0 error(s) + +make nixie +All diagrams validated successfully + +cargo test --doc +test result: ok. 14 passed; 0 failed; 0 ignored +``` diff --git a/docs/repository-layout.md b/docs/repository-layout.md index 159a509..071ef20 100644 --- a/docs/repository-layout.md +++ b/docs/repository-layout.md @@ -39,7 +39,10 @@ omits build output such as `target/`. │ ├── e2e.rs │ ├── features/ │ │ └── sleep_cli.feature -│ └── snapshots.rs +│ ├── snapshots.rs +│ ├── ui.rs +│ └── ui/ +│ └── ... ├── AGENTS.md ├── Cargo.toml ├── LICENSE @@ -96,6 +99,10 @@ omits build output such as `target/`. scenarios. - `tests/snapshots.rs`: Pins representative locale-aware remaining-time output. +- `tests/ui.rs`: Runs the `trybuild` UI fixtures in pass and compile-fail modes. +- `tests/ui/`: Holds external-crate fixtures that pin user-facing error text + (`*_display.rs`) and the non-exhaustive matching contract of the public error + enums (`*_non_exhaustive.rs`, with adjacent `.stderr` snapshots). - `AGENTS.md`: Provides repository-specific working instructions for agents and contributors. - `Cargo.toml`: Defines package metadata, dependencies, lint policy, and Cargo diff --git a/tests/ui.rs b/tests/ui.rs new file mode 100644 index 0000000..dfd2d10 --- /dev/null +++ b/tests/ui.rs @@ -0,0 +1,20 @@ +//! UI checks for the public error types. +//! +//! Two `trybuild` modes cover complementary guarantees. Pass fixtures compile +//! and run as external crates so they can observe formatted `Display` output, +//! which Rust only evaluates at runtime. Compile-fail fixtures snapshot the +//! compiler diagnostics that keep the public error enums non-exhaustive. + +/// Compiles and runs every display fixture, pinning public error message text. +#[test] +fn public_error_display_output() { + let cases = trybuild::TestCases::new(); + cases.pass("tests/ui/*_display.rs"); +} + +/// Compiles every non-exhaustive fixture, pinning the public matching contract. +#[test] +fn public_error_non_exhaustive_matching() { + let cases = trybuild::TestCases::new(); + cases.compile_fail("tests/ui/*_non_exhaustive.rs"); +} diff --git a/tests/ui/cli_error_display.rs b/tests/ui/cli_error_display.rs new file mode 100644 index 0000000..3a223ce --- /dev/null +++ b/tests/ui/cli_error_display.rs @@ -0,0 +1,33 @@ +//! Pins the user-facing display output of `CliError`. + +use catnap::{CliError, DurationParseError}; + +/// Asserts that an error renders exactly the pinned user-facing message. +fn assert_display(error: CliError, expected: &str) { + assert_eq!(error.to_string(), expected); +} + +/// Checks every public `CliError` variant against its pinned message. +fn main() { + assert_display(CliError::NonUnicodeArgument, "invalid non-Unicode argument"); + assert_display( + CliError::MissingOptionValue { option: "--colour" }, + "option '--colour' requires an argument", + ); + assert_display( + CliError::InvalidLogicalSecond { + value: "0".to_owned(), + }, + "invalid logical second duration '0'", + ); + assert_display( + CliError::UnknownOption { + option: "--unknown".to_owned(), + }, + "unrecognized option '--unknown'", + ); + assert_display( + CliError::Duration(DurationParseError::MissingOperand), + "missing operand", + ); +} diff --git a/tests/ui/cli_error_non_exhaustive.rs b/tests/ui/cli_error_non_exhaustive.rs new file mode 100644 index 0000000..e502b77 --- /dev/null +++ b/tests/ui/cli_error_non_exhaustive.rs @@ -0,0 +1,22 @@ +//! Pins `CliError` as non-exhaustive for downstream crates. + +use catnap::CliError; + +/// Matches every currently public variant without a wildcard arm. +/// +/// This must not compile. `CliError` is `#[non_exhaustive]`, so downstream +/// crates are required to keep a wildcard arm and adding a variant stays a +/// non-breaking change. +fn classify(error: &CliError) -> &'static str { + match error { + CliError::NonUnicodeArgument => "non-unicode-argument", + CliError::MissingOptionValue { .. } => "missing-option-value", + CliError::InvalidLogicalSecond { .. } => "invalid-logical-second", + CliError::UnknownOption { .. } => "unknown-option", + CliError::Duration(_) => "duration", + } +} + +fn main() { + println!("{}", classify(&CliError::NonUnicodeArgument)); +} diff --git a/tests/ui/cli_error_non_exhaustive.stderr b/tests/ui/cli_error_non_exhaustive.stderr new file mode 100644 index 0000000..4bb3f98 --- /dev/null +++ b/tests/ui/cli_error_non_exhaustive.stderr @@ -0,0 +1,18 @@ +error[E0004]: non-exhaustive patterns: `&_` not covered + --> tests/ui/cli_error_non_exhaustive.rs:11:11 + | +11 | match error { + | ^^^^^ pattern `&_` not covered + | +note: `CliError` defined here + --> src/cli.rs + | + | pub enum CliError { + | ^^^^^^^^^^^^^^^^^ + = note: the matched value is of type `&CliError` + = note: `CliError` is marked as non-exhaustive, so a wildcard `_` is necessary to match exhaustively +help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern or an explicit pattern as shown + | +16 ~ CliError::Duration(_) => "duration", +17 ~ &_ => todo!(), + | diff --git a/tests/ui/clock_config_error_display.rs b/tests/ui/clock_config_error_display.rs new file mode 100644 index 0000000..7660402 --- /dev/null +++ b/tests/ui/clock_config_error_display.rs @@ -0,0 +1,11 @@ +//! Pins the user-facing display output of `ClockConfigError`. + +use catnap::ClockConfigError; + +/// Checks every public `ClockConfigError` variant against its pinned message. +fn main() { + assert_eq!( + ClockConfigError::ZeroLogicalSecond.to_string(), + "logical second duration must be greater than zero", + ); +} diff --git a/tests/ui/clock_config_error_non_exhaustive.rs b/tests/ui/clock_config_error_non_exhaustive.rs new file mode 100644 index 0000000..906b343 --- /dev/null +++ b/tests/ui/clock_config_error_non_exhaustive.rs @@ -0,0 +1,18 @@ +//! Pins `ClockConfigError` as non-exhaustive for downstream crates. + +use catnap::ClockConfigError; + +/// Matches every currently public variant without a wildcard arm. +/// +/// This must not compile. `ClockConfigError` is `#[non_exhaustive]`, so +/// downstream crates are required to keep a wildcard arm and adding a variant +/// stays a non-breaking change. +fn classify(error: &ClockConfigError) -> &'static str { + match error { + ClockConfigError::ZeroLogicalSecond => "zero-logical-second", + } +} + +fn main() { + println!("{}", classify(&ClockConfigError::ZeroLogicalSecond)); +} diff --git a/tests/ui/clock_config_error_non_exhaustive.stderr b/tests/ui/clock_config_error_non_exhaustive.stderr new file mode 100644 index 0000000..311f593 --- /dev/null +++ b/tests/ui/clock_config_error_non_exhaustive.stderr @@ -0,0 +1,18 @@ +error[E0004]: non-exhaustive patterns: `&_` not covered + --> tests/ui/clock_config_error_non_exhaustive.rs:11:11 + | +11 | match error { + | ^^^^^ pattern `&_` not covered + | +note: `ClockConfigError` defined here + --> src/clock.rs + | + | pub enum ClockConfigError { + | ^^^^^^^^^^^^^^^^^^^^^^^^^ + = note: the matched value is of type `&ClockConfigError` + = note: `ClockConfigError` is marked as non-exhaustive, so a wildcard `_` is necessary to match exhaustively +help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern or an explicit pattern as shown + | +12 ~ ClockConfigError::ZeroLogicalSecond => "zero-logical-second", +13 ~ &_ => todo!(), + | diff --git a/tests/ui/duration_parse_error_display.rs b/tests/ui/duration_parse_error_display.rs new file mode 100644 index 0000000..69eb2e6 --- /dev/null +++ b/tests/ui/duration_parse_error_display.rs @@ -0,0 +1,43 @@ +//! Pins the user-facing display output of `DurationParseError`. + +use catnap::DurationParseError; + +/// Asserts that an error renders exactly the pinned user-facing message. +fn assert_display(error: DurationParseError, expected: &str) { + assert_eq!(error.to_string(), expected); +} + +/// Checks every public `DurationParseError` variant against its pinned message. +fn main() { + assert_display(DurationParseError::MissingOperand, "missing operand"); + assert_display( + DurationParseError::EmptyOperand { + operand: String::new(), + }, + "invalid time interval ''", + ); + assert_display( + DurationParseError::InvalidSuffix { + operand: "2fortnights".to_owned(), + }, + "invalid time suffix in '2fortnights'", + ); + assert_display( + DurationParseError::InvalidNumber { + operand: "soon".to_owned(), + }, + "invalid time interval 'soon'", + ); + assert_display( + DurationParseError::TooPrecise { + operand: "0.0000000001s".to_owned(), + }, + "time interval '0.0000000001s' has more than nanosecond precision", + ); + assert_display( + DurationParseError::Overflow { + operand: "340282366920938463464s".to_owned(), + }, + "time interval '340282366920938463464s' is too large", + ); +} diff --git a/tests/ui/duration_parse_error_non_exhaustive.rs b/tests/ui/duration_parse_error_non_exhaustive.rs new file mode 100644 index 0000000..96a3e10 --- /dev/null +++ b/tests/ui/duration_parse_error_non_exhaustive.rs @@ -0,0 +1,23 @@ +//! Pins `DurationParseError` as non-exhaustive for downstream crates. + +use catnap::DurationParseError; + +/// Matches every currently public variant without a wildcard arm. +/// +/// This must not compile. `DurationParseError` is `#[non_exhaustive]`, so +/// downstream crates are required to keep a wildcard arm and adding a variant +/// stays a non-breaking change. +fn classify(error: &DurationParseError) -> &'static str { + match error { + DurationParseError::MissingOperand => "missing-operand", + DurationParseError::EmptyOperand { .. } => "empty-operand", + DurationParseError::InvalidSuffix { .. } => "invalid-suffix", + DurationParseError::InvalidNumber { .. } => "invalid-number", + DurationParseError::TooPrecise { .. } => "too-precise", + DurationParseError::Overflow { .. } => "overflow", + } +} + +fn main() { + println!("{}", classify(&DurationParseError::MissingOperand)); +} diff --git a/tests/ui/duration_parse_error_non_exhaustive.stderr b/tests/ui/duration_parse_error_non_exhaustive.stderr new file mode 100644 index 0000000..c1dc5f1 --- /dev/null +++ b/tests/ui/duration_parse_error_non_exhaustive.stderr @@ -0,0 +1,18 @@ +error[E0004]: non-exhaustive patterns: `&_` not covered + --> tests/ui/duration_parse_error_non_exhaustive.rs:11:11 + | +11 | match error { + | ^^^^^ pattern `&_` not covered + | +note: `DurationParseError` defined here + --> src/duration.rs + | + | pub enum DurationParseError { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = note: the matched value is of type `&DurationParseError` + = note: `DurationParseError` is marked as non-exhaustive, so a wildcard `_` is necessary to match exhaustively +help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern or an explicit pattern as shown + | +17 ~ DurationParseError::Overflow { .. } => "overflow", +18 ~ &_ => todo!(), + |