fix(persistence): delete bank-info.improv, and pin why it could not be read
improvise-odbc named two readings of the parse failure at
`- JORDAN MARTINEZ` and said to decide by asking whether improvise can
WRITE that line today. It cannot: `item_list`'s `name` is
`pipe_quoted | bare_name`, `bare_name` is `[A-Za-z_][A-Za-z0-9_-]*`, and
`quote_name` mirrors it character for character, so a spaced item name is
emitted `|JORDAN MARTINEZ|`. So the reader and the writer agree and the
round-trip property is not incomplete — the file was stale.
It was stale by hand, not by drift. c48a5cd migrated it to v2025-04-09 by
editing it in place: the `Date` items were pipe-quoted and the `Entity`
ones were not, and every line of its `## Data` section names a category
`Measure` where the file declares `_Measure`. It has not been loadable
since April, and nothing loaded it, so nothing said so. Deleted rather
than regenerated: there is no generator for it — it is hand-written fake
bank data — so "regenerating" would mean inventing a canonical form and
checking in 1200 lines whose only property is that they parse, which the
generator-driven proptests already establish over a far wider input space.
What is kept is the evidence and the gap the incident actually exposed:
- `an_item_name_the_item_line_cannot_carry_round_trips` puts the file's own
item names through `format_md`/`parse_md` and asserts the spaced ones come
back quoted and the bare-legal one stays bare.
- `a_bare_item_name_with_a_space_is_rejected` asserts the reader really does
refuse the unquoted line, with the quoted form parsing as the precondition.
Without it the first test would be consistent with the reader accepting
both spellings, which is not what happened.
- `tests/shipped_documents_load.rs` closes "a sample document exercised by
nothing": it walks the two directories `.gitignore` un-ignores and loads
every `.improv` in them. A directory walk, not a list of names, because a
list is a second place to remember. Proved discriminating by dropping the
offending line into `examples/` and watching it fail.
Unrelated, reported not fixed: `foo.improv` in the repo root has no version
line at all and is equally unloadable.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,53 @@
|
||||
//! Every `.improv` document this repository ships must load (improvise-odbc).
|
||||
//!
|
||||
//! The finding behind this test: `bank-info.improv` sat in the repo root for
|
||||
//! five months unable to parse — hand-edited through the v2025-04-09 format
|
||||
//! change rather than round-tripped, so its item lines carried bare names the
|
||||
//! grammar cannot spell (`- JORDAN MARTINEZ`) and its data lines named a
|
||||
//! category `Measure` the file declares as `_Measure`. Nothing loaded it, so
|
||||
//! nothing said so.
|
||||
//!
|
||||
//! The grammar itself is covered far better than a fixture can manage, by the
|
||||
//! 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, and it is deliberately a
|
||||
//! directory walk rather than 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 two directories `.gitignore` un-ignores — `examples/` and
|
||||
//! `web/`. Everything else matching `*.improv` is somebody's working file.
|
||||
|
||||
use std::path::{Path, PathBuf};
|
||||
|
||||
fn shipped_documents() -> Vec<PathBuf> {
|
||||
let root = Path::new(env!("CARGO_MANIFEST_DIR"));
|
||||
let mut found: Vec<PathBuf> = ["examples", "web"]
|
||||
.iter()
|
||||
.flat_map(|dir| std::fs::read_dir(root.join(dir)).expect("a shipped-document directory"))
|
||||
.map(|entry| entry.expect("a readable directory entry").path())
|
||||
.filter(|p| p.extension().is_some_and(|e| e == "improv"))
|
||||
.collect();
|
||||
found.sort();
|
||||
found
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn every_shipped_document_loads() {
|
||||
let documents = shipped_documents();
|
||||
assert!(
|
||||
documents.len() >= 2,
|
||||
"precondition: the walk must find the documents that exist, got {documents:?}"
|
||||
);
|
||||
|
||||
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<_>>()
|
||||
);
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user