From aeea097d386e5fdfacf3da8bb69c1581c6929b62 Mon Sep 17 00:00:00 2001 From: "@permadeath.com" Date: Wed, 19 Aug 2026 15:07:09 -0400 Subject: [PATCH] test(lexicon): pin the generated record types against the captured fixtures Seven cases run each generated `sh.tangled.*` struct over every captured record, asserting both what parses and reserializes unchanged and what a lexicon-faithful type refuses. The read paths' permissiveness rested on four doc comments and no test, so a regeneration against a tightened schema would have surfaced as a record that silently stopped listing. It now fails here, naming the collection. Co-Authored-By: Claude Opus 5 (1M context) Change-Id: Idff9047b3002522cd8c0fbbf210ab9e0f796ba43 --- src/lexicon/tangled.rs | 200 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 200 insertions(+) diff --git a/src/lexicon/tangled.rs b/src/lexicon/tangled.rs index c778fa1..b6ba7c4 100644 --- a/src/lexicon/tangled.rs +++ b/src/lexicon/tangled.rs @@ -409,4 +409,204 @@ mod tests { assert_eq!(IssueState::from_token("closed"), None); assert_eq!(IssueState::from_token(""), None); } + + // ----------------------------------------------------------------------- + // What the generated types will and will not accept + // ----------------------------------------------------------------------- + + /// Every captured `sh.tangled.*` record, run through the generated struct + /// for its collection. + /// + /// This module's header asks the next person to keep deserialization + /// permissive, and until now that was a request with nothing behind it. + /// The reason it is a request at all is that the generated structs are + /// *lexicon-faithful*: a property the schema marks required is a + /// non-`Option` field, so a record missing it fails to deserialize whole + /// rather than arriving with that one field empty. Every one of these + /// records was written by software this project does not control, and the + /// read paths (`cmd::pr::read`, `cmd::issue::read`, `cmd::repo::read`, + /// `clients::tangled::comments`) therefore read fields off + /// `serde_json::Value` instead — deliberately, and with the reasoning + /// spread across four doc comments and no test. + /// + /// This is the test. It pins which real shapes each generated type + /// accepts, so that regenerating the bindings against a tightened schema + /// — or a fixture recaptured off a service that changed — turns up here, + /// in one failure that names the collection, instead of in somebody's + /// `pr list` as a record that silently stopped listing. + /// + /// Where a type *refuses* a real record, that refusal is asserted too and + /// named below. Those are not bugs in the fixtures; they are the exact + /// reason the corresponding read path is not typed. + mod generated_types_against_live_records { + use serde_json::Value; + use tangled_lexicon::sh_tangled::feed::comment::Comment as FeedComment; + use tangled_lexicon::sh_tangled::public_key::PublicKey; + use tangled_lexicon::sh_tangled::repo::Repo; + use tangled_lexicon::sh_tangled::repo::pull::Pull; + use tangled_lexicon::sh_tangled::repo::pull::status::Status as PullStatus; + + macro_rules! fixture { + ($name:literal) => { + include_str!(concat!( + env!("CARGO_MANIFEST_DIR"), + "/tests/fixtures/", + $name + )) + }; + } + + /// The `records` array of a captured `com.atproto.repo.listRecords` + /// page, or the `items` array of a captured Bobbin listing — both nest + /// the record under `value`, which is the one thing the two envelopes + /// agree on. + fn values(fixture: &str, key: &str) -> Vec { + let page: Value = serde_json::from_str(fixture).expect("fixture is JSON"); + let rows = page[key].as_array().expect("fixture has that array"); + assert!(!rows.is_empty(), "a fixture with no rows proves nothing"); + rows.iter().map(|r| r["value"].clone()).collect() + } + + /// One captured `getRecord`-shaped fixture: `{uri, cid, value}`. + fn single(fixture: &str) -> Value { + serde_json::from_str::(fixture).expect("fixture is JSON")["value"].clone() + } + + fn parses(value: &Value) -> Result<(), String> { + serde_json::from_value::(value.clone()) + .map(|_| ()) + .map_err(|e| e.to_string()) + } + + /// A record atgc reads back and writes again — `pr resubmit` appending + /// a round, `repo edit` rewriting a description — has to survive the + /// round trip byte for byte, or the write silently drops whatever the + /// struct did not model. The generated `extra_data` catch-all is what + /// carries those through, and this is the assertion that it does. + fn round_trips(value: &Value) { + let parsed: T = serde_json::from_value(value.clone()).expect("live record parses"); + assert_eq!( + serde_json::to_value(&parsed).expect("reserializes"), + *value, + "a field was lost between deserialize and serialize" + ); + } + + /// Eleven live pull records off this account's PDS, and the two + /// captured singly. All modern, all accepted, all lossless. + #[test] + fn every_modern_pull_record_parses_and_round_trips() { + for value in values(fixture!("pds_pulls_page.json"), "records") { + round_trips::(&value); + } + for fixture in [ + fixture!("pull_new_record.json"), + fixture!("pull_new_with_source.json"), + ] { + round_trips::(&single(fixture)); + } + } + + /// The one that matters, and the reason `cmd::pr::read` reads pull + /// records off `Value`. + /// + /// A pre-rounds record carries a single inline `patch` string and has + /// neither `rounds` nor `target`; the current schema marks both + /// required, so `Pull` refuses the whole record. `pr list`, `pr view`, + /// `pr diff` and `pr merge` all still work on one — see + /// `cmd::pr::review`'s `reads_a_pre_rounds_record_that_pull_cannot_parse`, + /// which asserts the same refusal from the other side. Typing those + /// read paths would drop this record from every listing it belongs in, + /// which is why they are not typed. + #[test] + fn a_pre_rounds_pull_record_is_refused_by_the_generated_type() { + let value = single(fixture!("pull_old_record.json")); + assert!(value.get("rounds").is_none(), "fixture predates rounds"); + assert!(value.get("target").is_none(), "fixture predates target"); + assert!(value["patch"].is_string(), "fixture inlines its patch"); + let Err(refusal) = parses::(&value) else { + panic!( + "`Pull` now accepts a pre-rounds record. If the schema relaxed \ + `rounds`/`target`, this test and `cmd::pr::read`'s permissive \ + field reads can both be revisited." + ); + }; + assert!(!refusal.is_empty()); + } + + /// The index re-emits the record, it does not restate it. + /// + /// `cmd::pr::read::sources` merges a Bobbin listing with a PDS + /// listing on the at-uri and then reads fields off whichever row + /// survived, which is only correct if the two carry the same record. + /// Every item on this captured page parses as `Pull` and reserializes + /// to the bytes it arrived as, which is that assumption checked + /// against the index rather than assumed of it. + #[test] + fn the_indexed_copy_of_a_pull_is_the_same_record() { + let rows = values(fixture!("bobbin_list_pulls_by.json"), "items"); + for value in &rows { + round_trips::(value); + } + } + + /// Status records are the one collection a read path could type: every + /// captured one parses, and an unknown token deserializes into + /// `StatusStatus::Other` rather than failing. `cmd::pr::read` still + /// reads them off `Value`, because `createdAt` and `pull` are required + /// here and a record missing either would drop out of the state join + /// entirely rather than leaving one pull's state unknown. + #[test] + fn every_status_record_parses_and_round_trips() { + for fixture in [ + fixture!("pds_pull_statuses_page.json"), + fixture!("pull_statuses.json"), + ] { + for value in values(fixture, "records") { + round_trips::(&value); + } + } + } + + /// Both captured repo records, and the whole captured page. `name` is + /// absent from every one of them — the repo is named by its record key + /// — which is what `cmd::repo::read` falls back to. + #[test] + fn every_repo_record_parses_and_round_trips() { + for fixture in [ + fixture!("repo_record.json"), + fixture!("repo_record_full.json"), + ] { + round_trips::(&single(fixture)); + } + for value in values(fixture!("pds_repos_page.json"), "records") { + round_trips::(&value); + } + } + + /// Current `sh.tangled.feed.comment` records parse. The legacy + /// `sh.tangled.repo.issue.comment` shape a thread older than the + /// unification is made of does not, and cannot: it names its subject + /// as a bare at-uri where the current lexicon takes a strongRef whose + /// CID it has nowhere to put. `clients::tangled::comments` reads both + /// off `Value` for exactly that reason — see its module header. + #[test] + fn every_current_comment_record_parses_and_round_trips() { + for value in values(fixture!("feed_comment_records.json"), "records") { + round_trips::(&value); + } + for value in values(fixture!("feed_comments_on_pull.json"), "items") { + round_trips::(&value); + } + } + + /// The collection atgc both writes and reads whole, and the one where + /// a refusal would be correct: a key record with no key is not a key. + #[test] + fn every_public_key_record_parses_and_round_trips() { + for value in values(fixture!("public_keys.json"), "records") { + round_trips::(&value); + } + } + } } -- 2.51.2