git-ents.gitmain
⌘K
foforge
commit 67c5c00
test: pin the on-disk meta-ref formats against fixed fixtures

A document’s Facet shape is its on-disk format, so an incompatible change silently breaks reading data already on a ref. Each document type now has a load test against a tree built with raw git plumbing in the real layout, and git-store documents the "never change a meta-ref type incompatibly" policy.

test: add fixed-format load tests for signers, checks, and runs refactor: share unique_repo and a meta-ref fixture builder in testutil docs: record the meta-ref format-stability policy in git-store Assisted-by: Claude:claude-opus-4-8

Joseph D. Carpinelli · 1 month ago

Reviews

No reviews of this commit yet — record a verdict below.

Start a review

verdict

crates/git-ents/src/checks.rs @@ -211,25 +211,11 @@ reason = "unit test" )] - use std::path::PathBuf; - use std::process::Command; - use std::sync::atomic::{AtomicUsize, Ordering}; - use super::*; + use crate::testutil::{unique_repo as new_repo, write_meta_doc}; - fn unique_repo() -> PathBuf { - static COUNTER: AtomicUsize = AtomicUsize::new(0); - let n = COUNTER.fetch_add(1, Ordering::SeqCst); - let dir = std::env::temp_dir().join(format!("git-ents-checks-{}-{n}", std::process::id())); - std::fs::create_dir_all(&dir).unwrap(); - let status = Command::new("git") - .arg("-C") - .arg(&dir) - .args(["init", "-q"]) - .status() - .unwrap(); - assert!(status.success()); - dir + fn unique_repo() -> std::path::PathBuf { + new_repo("checks") } fn check(name: &str, command: &str) -> Check { @@ -273,6 +259,53 @@ let _ = std::fs::remove_dir_all(&repo); } + #[test] + fn loads_the_on_disk_checks_format() { + // A fixture written as the real `checks/<name>` blob layout must keep + // loading, guarding the Checks document's shape against an incompatible + // change to data already on a ref. + let repo = unique_repo(); + write_meta_doc( + &repo, + CHECKS_REF, + "checks", + &[("fmt", "cargo fmt --check"), ("test", "cargo nextest run")], + ); + let mut loaded = load(&repo).unwrap(); + loaded.sort_by(|a, b| a.name.cmp(&b.name)); + assert_eq!( + loaded, + vec![ + check("fmt", "cargo fmt --check"), + check("test", "cargo nextest run") + ] + ); + let _ = std::fs::remove_dir_all(&repo); + } + + #[test] + fn loads_the_on_disk_runs_format() { + // A fixture written as the real `results/<name>` blob layout on a run ref + // must keep loading, guarding the RunDoc document's shape. + let repo = unique_repo(); + let commit = "0123456789012345678901234567890123456789"; + write_meta_doc( + &repo, + &format!("{RUNS_NS}/{commit}"), + "results", + &[("fmt", "pass"), ("test", "fail")], + ); + let commits = runs(&repo).unwrap(); + assert_eq!(commits.len(), 1); + assert_eq!(commits[0].commit, commit); + assert_eq!(commits[0].runs.len(), 1); + assert_eq!( + commits[0].runs[0].results, + vec![outcome("fmt", "pass"), outcome("test", "fail")] + ); + let _ = std::fs::remove_dir_all(&repo); + } + fn outcome(name: &str, outcome: &str) -> RunOutcome { RunOutcome { name: name.to_owned(),
crates/git-ents/src/lib.rs @@ -2,6 +2,8 @@ pub mod checks; pub mod signers; +#[cfg(test)] +mod testutil; /// The all-zero object id git uses for a created or deleted ref in a push /// (`<old> <new> <ref>` lines): a zero `<old>` is a create, a zero `<new>` a
crates/git-ents/src/signers.rs @@ -101,30 +101,16 @@ reason = "unit test" )] - use std::path::PathBuf; - use std::process::Command; - use std::sync::atomic::{AtomicUsize, Ordering}; - use super::*; + use crate::testutil::{unique_repo as new_repo, write_meta_doc}; const KEY_A: &str = "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaA alice"; const KEY_B: &str = "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbB bob"; - fn unique_repo() -> PathBuf { - static COUNTER: AtomicUsize = AtomicUsize::new(0); - let n = COUNTER.fetch_add(1, Ordering::SeqCst); - let dir = std::env::temp_dir().join(format!("git-ents-signers-{}-{n}", std::process::id())); - std::fs::create_dir_all(&dir).unwrap(); - let status = Command::new("git") - .arg("-C") - .arg(&dir) - .args(["init", "-q"]) - .status() - .unwrap(); - assert!(status.success()); - dir + fn unique_repo() -> std::path::PathBuf { + new_repo("signers") } fn signer(fingerprint: &str, key: &str) -> Signer { @@ -162,6 +148,27 @@ let _ = std::fs::remove_dir_all(&repo); } + #[test] + fn loads_the_on_disk_signers_format() { + // A fixture written as the real `signers/<fingerprint>` blob layout must + // keep loading; this fails if the Auth document's shape changes + // incompatibly with data already on a ref. + let repo = unique_repo(); + write_meta_doc( + &repo, + AUTH_REF, + "signers", + &[("aa:bb:cc", KEY_A), ("dd:ee:ff", KEY_B)], + ); + let mut loaded = load(&repo).unwrap(); + loaded.sort_by(|a, b| a.fingerprint.cmp(&b.fingerprint)); + assert_eq!( + loaded, + vec![signer("aa:bb:cc", KEY_A), signer("dd:ee:ff", KEY_B)] + ); + let _ = std::fs::remove_dir_all(&repo); + } + #[test] fn renders_a_wildcard_allowed_signers_file() { assert_eq!(
crates/git-store/src/lib.rs @@ -7,6 +7,16 @@ //! timestamp, so nothing about versioning has to be modeled in the tree //! itself. This is the single home for the plumbing that the signer set, the //! check set, and the run log all share. +//! +//! # Format stability +//! +//! A document's [`Facet`] shape *is* its on-disk format: the tree git holds is +//! derived from it, so an incompatible change (a renamed field, a changed +//! field type) silently stops reading data already on a ref and only surfaces +//! at load time. The policy is therefore to never change a meta-ref document +//! type incompatibly; each type carries a load test against a hand-built +//! fixture in the real layout to catch a regression at compile-and-test time +//! rather than in production. use std::cmp::Reverse; use std::collections::BTreeMap;
crates/git-ents/src/testutil.rs @@ -1,0 +1,83 @@ +//! Shared test helpers for the meta-ref modules: a throwaway git repository and +//! a builder that lays an on-disk `refs/meta/*` document out with raw git +//! plumbing. +//! +//! Building the tree directly — rather than through [`git_store::Store`] — pins +//! the *on-disk* layout each document type promises: a `<subtree>/<key>` blob +//! per entry. A load test against a fixture written this way fails the moment an +//! incompatible change to a document's [`facet::Facet`] shape stops reading data +//! already in the wild, the failure mode that broke every push once before. + +#![allow( + clippy::unwrap_used, + clippy::let_underscore_must_use, + reason = "test support" +)] + +use std::io::Write as _; +use std::path::{Path, PathBuf}; +use std::process::{Command, Stdio}; +use std::sync::atomic::{AtomicUsize, Ordering}; + +/// A freshly initialized, uniquely named git repository under the temp dir. +#[must_use] +pub(crate) fn unique_repo(label: &str) -> PathBuf { + static COUNTER: AtomicUsize = AtomicUsize::new(0); + let n = COUNTER.fetch_add(1, Ordering::SeqCst); + let dir = std::env::temp_dir().join(format!("git-ents-{label}-{}-{n}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + let status = Command::new("git") + .arg("-C") + .arg(&dir) + .args(["init", "-q"]) + .status() + .unwrap(); + assert!(status.success()); + dir +} + +/// Lay a document out at `refname` as the real on-disk format: one +/// `<subtree>/<key>` blob per pair, committed and pointed to by the ref. Used to +/// assert that loaders still read the format independent of the writer. +pub(crate) fn write_meta_doc(repo: &Path, refname: &str, subtree: &str, pairs: &[(&str, &str)]) { + let mut entries = String::new(); + for (key, value) in pairs { + let blob = git_with_stdin(repo, &["hash-object", "-w", "--stdin"], value); + entries.push_str(&format!("100644 blob {blob}\t{key}\n")); + } + let sub = git_with_stdin(repo, &["mktree"], &entries); + let root = git_with_stdin( + repo, + &["mktree"], + &format!("040000 tree {sub}\t{subtree}\n"), + ); + let commit = git_with_stdin(repo, &["commit-tree", &root, "-m", "fixture"], ""); + let status = Command::new("git") + .arg("-C") + .arg(repo) + .args(["update-ref", refname, &commit]) + .status() + .unwrap(); + assert!(status.success()); +} + +/// Run git in `repo` with `input` on stdin, returning its trimmed stdout. +fn git_with_stdin(repo: &Path, args: &[&str], input: &str) -> String { + let mut child = Command::new("git") + .arg("-C") + .arg(repo) + .args(args) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .spawn() + .unwrap(); + child + .stdin + .take() + .unwrap() + .write_all(input.as_bytes()) + .unwrap(); + let output = child.wait_with_output().unwrap(); + assert!(output.status.success(), "git {args:?} failed"); + String::from_utf8(output.stdout).unwrap().trim().to_owned() +}