From 406176bc9fa78a9118a7d4a0e186696ed662c582 Mon Sep 17 00:00:00 2001 From: ThinkChaos Date: Thu, 21 Dec 2023 10:12:39 -0500 Subject: [PATCH 1/5] chore: add artifacts/ to gitignore --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index d787b70..58881e2 100644 --- a/.gitignore +++ b/.gitignore @@ -1,2 +1,3 @@ +/artifacts /target /result From fa48a3ba90295cbdd3c6572680d77b7fd9a1aef4 Mon Sep 17 00:00:00 2001 From: ThinkChaos Date: Thu, 21 Dec 2023 10:13:36 -0500 Subject: [PATCH 2/5] fix: interpret path printed by nix as relative to repo root This was causing `check` to always see the tests as new/failing, but `review` to show a diff with no changes. --- src/cmd/check.rs | 5 +++-- src/cmd/clean.rs | 5 +++-- src/cmd/review.rs | 4 ++-- src/main.rs | 19 ++++++++++++++++--- 4 files changed, 24 insertions(+), 9 deletions(-) diff --git a/src/cmd/check.rs b/src/cmd/check.rs index d2cd8d9..320c55e 100644 --- a/src/cmd/check.rs +++ b/src/cmd/check.rs @@ -1,6 +1,7 @@ use std::{ fs::{self, create_dir_all, remove_dir_all, File}, io::{stderr, BufRead, Write}, + path::Path, process::exit, }; @@ -14,7 +15,7 @@ use crate::{ proto::{TestOutput, TestResult}, }; -pub fn check(opts: Opts, cfg: Option) -> Result<()> { +pub fn check(root: &Path, opts: Opts, cfg: Option) -> Result<()> { let output = nix_check(opts, cfg)?; let success = output.status.success(); @@ -26,7 +27,7 @@ pub fn check(opts: Opts, cfg: Option) -> Result<()> { let output = serde_json::from_str::(line)?; - let pending = output.dir.join("_snapshots").join(".pending"); + let pending = root.join(output.dir).join("_snapshots/.pending"); let _ = remove_dir_all(&pending); create_dir_all(&pending)?; fs::write(pending.join(".gitignore"), "*")?; diff --git a/src/cmd/clean.rs b/src/cmd/clean.rs index 99b45fe..8b378cf 100644 --- a/src/cmd/clean.rs +++ b/src/cmd/clean.rs @@ -1,5 +1,6 @@ use std::{ fs::{read_dir, remove_dir_all, remove_file}, + path::Path, io::{stderr, BufRead, Write}, }; @@ -8,7 +9,7 @@ use owo_colors::OwoColorize; use crate::{cfg::Config, cli::Opts, cmd::run::nix_eval, proto::TestOutput}; -pub fn clean(opts: Opts, cfg: Option) -> Result<()> { +pub fn clean(root: &Path, opts: Opts, cfg: Option) -> Result<()> { let output = nix_eval(opts, cfg)?; for line in output.stderr.lines() { @@ -19,7 +20,7 @@ pub fn clean(opts: Opts, cfg: Option) -> Result<()> { let mut out = stderr().lock(); let output = serde_json::from_str::(line)?; - let snapshots = output.dir.join("_snapshots"); + let snapshots = root.join(output.dir).join("_snapshots"); for entry in read_dir(snapshots)? { let entry = entry?; diff --git a/src/cmd/review.rs b/src/cmd/review.rs index c6d2192..58867b3 100644 --- a/src/cmd/review.rs +++ b/src/cmd/review.rs @@ -20,7 +20,7 @@ use crate::{ proto::{Snapshot, TestOutput}, }; -pub fn review(opts: Opts, cfg: Option) -> Result<()> { +pub fn review(root: &Path, opts: Opts, cfg: Option) -> Result<()> { let output = nix_eval(opts, cfg)?; let _ = ctrlc::set_handler(|| { let mut term = Term::stderr(); @@ -36,7 +36,7 @@ pub fn review(opts: Opts, cfg: Option) -> Result<()> { }; let output = serde_json::from_str::(line)?; - let snapshots = output.dir.join("_snapshots"); + let snapshots = root.join(output.dir).join("_snapshots"); for entry in read_dir(snapshots.join(".pending"))? { use bstr::ByteSlice; diff --git a/src/main.rs b/src/main.rs index 3fb8847..3c0c45d 100644 --- a/src/main.rs +++ b/src/main.rs @@ -23,9 +23,22 @@ fn main() -> Result<()> { let cfg = cfg::load()?; + let root = repo_root()?; + match opts.subcmd { - Subcommand::Check => check(opts, cfg), - Subcommand::Clean => clean(opts, cfg), - Subcommand::Review => review(opts, cfg), + Subcommand::Check => check(&root, opts, cfg), + Subcommand::Clean => clean(&root, opts, cfg), + Subcommand::Review => review(&root, opts, cfg), } } + +fn repo_root() -> Result { + let mut cmd = std::process::Command::new("git"); + cmd.args(["rev-parse", "--show-toplevel"]); + + let out = cmd.output()?; + let str = std::str::from_utf8(&out.stdout)?; + let str = str.trim_end(); + + Ok(str.to_owned().into()) +} From c6e72ba5640f82403aaf9a57d126411d9dfc5ffd Mon Sep 17 00:00:00 2001 From: ThinkChaos Date: Thu, 21 Dec 2023 10:15:50 -0500 Subject: [PATCH 3/5] feat(check): track test additions separately from failures Tweak the output so those cases are more easily distinguishable, especially the color used by the `+` symbol next to the test name. Exit code is now 2 if all existing tests succeeded, but some were added. --- src/cmd/check.rs | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/src/cmd/check.rs b/src/cmd/check.rs index 320c55e..da6c01b 100644 --- a/src/cmd/check.rs +++ b/src/cmd/check.rs @@ -34,6 +34,7 @@ pub fn check(root: &Path, opts: Opts, cfg: Option) -> Result<()> { let total = output.results.len(); let mut failures = 0; + let mut additions = 0; for (name, res) in output.results { let new = pending.join(&name); match res { @@ -42,14 +43,19 @@ pub fn check(root: &Path, opts: Opts, cfg: Option) -> Result<()> { } TestResult::Failure { snapshot, old } => { - failures += 1; - println!("{} {name}", if old { "✘" } else { "🞥" }.red()); + if old { + failures += 1; + println!("{} {name}", "✘".red()); + } else { + additions += 1; + println!("{} {name}", "🞥".blue()); + } snapshot.to_writer(File::create(new)?)?; } } } - if failures == 0 { + if failures == 0 && additions == 0 { if success { eprintln!("All {total} tests succeeded"); return Ok(()); @@ -57,9 +63,15 @@ pub fn check(root: &Path, opts: Opts, cfg: Option) -> Result<()> { break; } } else { - eprintln!("{failures} out of {total} tests failed"); + if failures != 0 { + let existing = total - additions; + eprintln!("{failures} out of {existing} tests failed"); + } + if additions != 0 { + eprintln!("{additions} new tests found"); + } eprintln!("run `namaka review` to review the pending snapshots"); - exit(1); + exit(if failures != 0 { 1 } else { 2 }); } } From e37d0d8839137e46ff2b7c377d69061de47a4f19 Mon Sep 17 00:00:00 2001 From: ThinkChaos Date: Thu, 21 Dec 2023 10:37:55 -0500 Subject: [PATCH 4/5] fix: only show `flake` and `dir` warning when relevant Forgetting to set `src` was triggering it. Ideally we'd just change the function to use `{ src, ... }@args:` so Nix can provide an explicit error the users are accustomed to. --- nix/load.nix | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/nix/load.nix b/nix/load.nix index 3ad4944..76c8d55 100644 --- a/nix/load.nix +++ b/nix/load.nix @@ -30,9 +30,14 @@ args: let src = toString ( - args.src or (warn - "namaka.load: `flake` and `dir` have been deprecated, use `src` directly instead" - (args.flake + "/${args.dir or "tests"}")) + if args ? src then + args.src + else if args ? flake then + warn + "namaka.load: `flake` and `dir` have been deprecated, use `src` directly instead" + (args.flake + "/${args.dir or "tests"}") + else + throw "namaka.load: missing mandatory `src' argument" ); tests = haumea.load (removeAttrs args [ "flake" "dir" ] // { From c1eebb02ec22fd7e307056381b4080d1ca42b1eb Mon Sep 17 00:00:00 2001 From: ThinkChaos Date: Wed, 25 Feb 2026 20:24:16 -0500 Subject: [PATCH 5/5] Apply suggestions from code review Co-authored-by: figsoda --- nix/load.nix | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/nix/load.nix b/nix/load.nix index 76c8d55..39da029 100644 --- a/nix/load.nix +++ b/nix/load.nix @@ -32,12 +32,12 @@ let src = toString ( if args ? src then args.src - else if args ? flake then + else if args ? flake || args ? dir then warn "namaka.load: `flake` and `dir` have been deprecated, use `src` directly instead" (args.flake + "/${args.dir or "tests"}") else - throw "namaka.load: missing mandatory `src' argument" + throw "namaka.load: missing mandatory `src` argument" ); tests = haumea.load (removeAttrs args [ "flake" "dir" ] // {