fiddlerwoaroof/improvise
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

every_shipped_document_loads walks the index, not a .gitignore reading (improvise-c0aj)

Browse Source
This commit is contained in:
Edward Langley
2026-09-23 08:56:26 -07:00
parent 4e52d06838
commit 6dceaad1c7
2 changed files with 97 additions and 78 deletions
Download Patch File Download Diff File Expand all files Collapse all files
context/test-evidence/tui.md
+47 -26
View File
@@ -426,38 +426,59 @@ generator-driven proptests in `improvise-io`. What *those* cannot see is a
checked-in document drifting out of the format, because they generate their
inputs. This test is aimed at exactly that gap.
**It is deliberately a directory walk rather than a list of names**: a list is a
**It walks the files the repository tracks, not a list of names**: a list is a
second place to remember, and the file that gets forgotten is the one nobody
added to it. The scope is the directories `.gitignore` un-ignores `*.improv`
in. Everything else matching `*.improv` is somebody's working file.
added to it. "Shipped" means "tracked", and `shipped_documents` reads it off
the index: every `*.improv` and `*.improv.gz` path that `ls-files` lists. It does
not include untracked files, so a working `.improv.gz` in someone's tree cannot
fail their run. A document not yet added is walked once it is, which is the
state CI tests.
**The directory list is read from `.gitignore`, not written in the test
(improvise-qlrh).** The test used to hard-code `["examples", "web"]`, a list of
exactly the kind the paragraph above warns against. When improvise-plcx
un-ignored `harness/fixtures/*.improv`, nothing added that directory to the
walk. Its two fixtures were then loaded only by the Playwright job, where a
missing seed degrades to an empty model and surfaces as a missing-label
failure, not a load error. `shipped_directories` now takes every
`!<dir>/*.improv` line, so un-ignoring a directory is what enlists it.
**How the walk got here: two steps, each closing the previous one's holes.**
The precondition is per directory: each derived directory must yield at least
one document. The old whole-walk `>= 2` could not see a directory silently lost.
- **improvise-qlrh.** The test hard-coded `["examples", "web"]`. When
improvise-plcx un-ignored `harness/fixtures/*.improv`, nothing added that
directory. Its two fixtures were then loaded only by the Playwright job,
where a missing seed degrades to an empty model and fails as a missing label,
not a load error. The fix read `!<dir>/*.improv` lines from the root
`.gitignore`, with a per-directory precondition. Red at `705c92d`:
`harness/fixtures/tall.improv` replaced with `{"legacy": true}` passed the old
test.
- **improvise-c0aj.** That `.gitignore` reading was a parser for one shape of
one file, so four other ways of shipping a document were missed silently:
- a single-file un-ignore (`!web/demo.improv`);
- a `.improv.gz` anywhere, since `*.improv` never ignores it and so no
un-ignore line names it;
- a `!` line in a nested `.gitignore`;
- a force-added (`add -f`) document.
**Red, measured at `705c92d`:** `harness/fixtures/tall.improv` was replaced with
`{"legacy": true}`, and the old test still passed. **Rivals, each run against
the new test:**
Each now has its own test, which builds the shape in a scratch repository
and asserts the walk lists the document. **All four were red** against the
`.gitignore` walk, each failing at its `walked` assertion with its setup
commands succeeding. Walking the index closes all four by construction and
deletes the parser, because git has already answered what the parser
approximated.
- **That same broken fixture.** Fails with "…/harness/fixtures/tall.improv does
not load".
- **A walk that drops every `harness` path.** Fails the per-directory
precondition, naming `harness/fixtures`.
- **A derivation that matches no `.gitignore` line.** Fails "…must un-ignore at
least one *.improv directory".
**The one new failure mode, answered: no index.** `ls-files` needs a checkout.
In CI (`actions/checkout`) and every worktree it has one. No nix build or
packaging step runs this test; the flake is a dev shell only. Where there is no
index, the walk must not answer "nothing", because an empty walk passes every
per-document assertion vacuously. So it asserts `ls-files` succeeded, and
`a_directory_that_is_not_a_checkout_fails_loudly` pins that. The whole-walk
precondition "at least one document" stays as the second line of defence.
**It matches on the full name, not on `Path::extension()`**: `.improv.gz` is the
same document gzipped and `extension()` calls it `gz`, so matching on the
extension alone would skip it and put back the very hole this test exists to
close.
**Rivals, each run against the new walk:**
- **The `.improv.gz` pathspec removed.** Fails `a_gzipped_document_is_walked`,
and nothing else.
- **A failed `ls-files` answered with an empty list instead of a panic.** Fails
`a_directory_that_is_not_a_checkout_fails_loudly`.
- **`harness/fixtures/tall.improv` replaced with `{"legacy": true}`.** Fails
`every_shipped_document_loads`, naming the file.
**It matches on the full name, not on `Path::extension()`.** `.improv.gz` is the
same document gzipped, and `extension()` calls it `gz`. The pathspec lists both
suffixes for the same reason.
---
tests/shipped_documents_load.rs
+50 -52
View File
@@ -1,71 +1,52 @@
//! Every `.improv` document this repository ships must load (improvise-odbc).
//! Why a DIRECTORY WALK and not a list: `context/test-evidence/tui.md`.
//! "Shipped" is "tracked", read off git's index: `context/test-evidence/tui.md`.
use std::path::{Path, PathBuf};
/// The directories `.gitignore` un-ignores `*.improv` in: the definition of
/// "shipped", read from the one place that states it (improvise-qlrh).
fn shipped_directories(root: &Path) -> Vec<PathBuf> {
let gitignore = std::fs::read_to_string(root.join(".gitignore")).expect("a .gitignore");
gitignore
.lines()
.filter_map(|line| line.trim().strip_prefix('!'))
.filter_map(|pattern| pattern.strip_suffix("/*.improv"))
.map(|dir| root.join(dir))
.collect()
}
fn documents_in(dir: &Path) -> Vec<PathBuf> {
let mut found: Vec<PathBuf> = std::fs::read_dir(dir)
.expect("a shipped-document directory")
.map(|entry| entry.expect("a readable directory entry").path())
// `extension()` of `x.improv.gz` is `gz`, so matching on the extension
// alone would skip it and put back the hole this test closes.
.filter(|p| {
p.to_str()
.is_some_and(|s| s.ends_with(".improv") || s.ends_with(".improv.gz"))
})
/// Every tracked `.improv` and `.improv.gz` under `root`. Outside a checkout this
/// panics rather than answering empty (improvise-c0aj).
fn shipped_documents(root: &Path) -> Vec<PathBuf> {
let out = std::process::Command::new("git")
.arg("-C")
.arg(root)
.args(["ls-files", "-z", "--", "*.improv", "*.improv.gz"])
.output()
.expect("git runs: this test reads the index, so it needs a checkout");
assert!(
out.status.success(),
"git ls-files failed in {}: {}",
root.display(),
String::from_utf8_lossy(&out.stderr)
);
let mut found: Vec<PathBuf> = String::from_utf8(out.stdout)
.expect("UTF-8 paths")
.split('\0')
.filter(|p| !p.is_empty())
.map(|p| root.join(p))
.collect();
found.sort();
found
}
/// Every shipped document under `root`: the walk the test below loads.
fn shipped_documents(root: &Path) -> Vec<PathBuf> {
shipped_directories(root)
.iter()
.flat_map(|dir| documents_in(dir))
.collect()
}
#[test]
fn every_shipped_document_loads() {
let root = Path::new(env!("CARGO_MANIFEST_DIR"));
let dirs = shipped_directories(root);
let documents = shipped_documents(root);
assert!(
!dirs.is_empty(),
"precondition: .gitignore must un-ignore at least one *.improv directory"
!documents.is_empty(),
"precondition: the index must list at least one shipped document"
);
for dir in dirs {
let documents = documents_in(&dir);
// Per directory, so a walk that silently loses one cannot pass (qlrh).
for path in documents {
let workbook = improvise::persistence::load(&path)
.unwrap_or_else(|e| panic!("{} does not load: {e}", path.display()));
assert!(
!documents.is_empty(),
"precondition: {} is un-ignored for *.improv but the walk found none there",
dir.display()
workbook.views.contains_key(&workbook.active_view),
"{} loads but its initial view {:?} is not one of {:?}",
path.display(),
workbook.active_view,
workbook.views.keys().collect::<Vec<_>>()
);
for path in documents {
let workbook = improvise::persistence::load(&path)
.unwrap_or_else(|e| panic!("{} does not load: {e}", path.display()));
assert!(
workbook.views.contains_key(&workbook.active_view),
"{} loads but its initial view {:?} is not one of {:?}",
path.display(),
workbook.active_view,
workbook.views.keys().collect::<Vec<_>>()
);
}
}
}
@@ -125,10 +106,27 @@ fn a_document_un_ignored_by_a_nested_gitignore_is_walked() {
write(root, ".gitignore", "*.improv\n");
write(root, "harness/.gitignore", "!keep.improv\n");
write(root, "harness/keep.improv", "x");
run_git(root, &["add", ".gitignore", "harness/.gitignore", "harness/keep.improv"]);
run_git(
root,
&[
"add",
".gitignore",
"harness/.gitignore",
"harness/keep.improv",
],
);
assert!(walked(root, "harness/keep.improv"));
}
/// An empty walk would pass every per-document assertion vacuously, so a
/// directory git cannot read must stop the test instead.
#[test]
#[should_panic(expected = "git ls-files failed")]
fn a_directory_that_is_not_a_checkout_fails_loudly() {
let not_a_repo = tempfile::tempdir().expect("a temp dir");
shipped_documents(not_a_repo.path());
}
#[test]
fn a_force_added_document_is_walked() {
let repo = scratch_repo();