diff --git a/crates/helm-bv/tests/conformance.rs b/crates/helm-bv/tests/conformance.rs --- a/crates/helm-bv/tests/conformance.rs +++ b/crates/helm-bv/tests/conformance.rs @@ -489,6 +489,14 @@ }; for entry in entries.flatten() { let path = entry.path(); + // `logs/` is MegaMek's own scratch, not data it ships: anything that + // starts MegaMek rewrites it. A corpus that includes it changes under + // the test, and a case it once covered can disappear without anyone + // touching the repository - which is how the only ejected crew in the + // corpus went missing. + if path.file_name().is_some_and(|n| n == "logs") { + continue; + } if path.is_dir() { collect_muls(&path, out); } else if path diff --git a/crates/helm-unitfile/src/mul.rs b/crates/helm-unitfile/src/mul.rs --- a/crates/helm-unitfile/src/mul.rs +++ b/crates/helm-unitfile/src/mul.rs @@ -335,12 +335,17 @@ /// Attach a finished element to whatever contains it - or, if it is an entity, /// read it into the model. fn close(open: &mut [Kept], done: Kept, mul: &mut Mul) { + // Only an entity the document holds directly is a unit, which is what + // MULParser reads. One nested inside another is not a `.mul` shape at all, + // and treating it as a unit hoists it out of whatever held it - so it is + // kept where it was found and left alone. + let is_unit = done.name == "entity" && open.len() <= 1; // An entity is read into the model, and the document keeps an empty one in // its place. The position is what the document needs - a `.mul` can carry // `` and a crew that got out beside its units, and writing them // back in the wrong order is still a change to somebody's file - while the // entity itself is held once, by the unit that speaks for it. - let done = if done.name == "entity" { + let done = if is_unit { let name = done.name.clone(); mul.units.push(read_entity(done)); Kept { @@ -1320,6 +1325,166 @@ let changed: Vec<_> = a.lines().zip(b.lines()).filter(|(x, y)| x != y).collect(); assert_eq!(changed.len(), 1, "more than the camo changed: {changed:?}"); assert!(changed[0].1.contains("camoCategory=Clans/Ghost Bear")); + } + + /// A tiny deterministic generator, so a failure can be rerun. + /// + /// Hand-rolled rather than a property-testing crate: what is wanted here + /// is a few thousand awkward documents, not a shrinking framework, and + /// this crate's parsers carry no dependencies for the same reason. + struct Rng(u64); + + impl Rng { + fn next(&mut self) -> u64 { + // xorshift64*, which is enough randomness to build XML with. + self.0 ^= self.0 >> 12; + self.0 ^= self.0 << 25; + self.0 ^= self.0 >> 27; + self.0.wrapping_mul(0x2545_f491_4f6c_dd1d) + } + + fn pick<'a, T>(&mut self, from: &'a [T]) -> &'a T { + &from[(self.next() % from.len() as u64) as usize] + } + + fn upto(&mut self, n: u64) -> u64 { + self.next() % n + } + } + + /// Build a document this crate has never seen, out of the pieces a `.mul` + /// is made of and the ones it is not. + /// + /// The names and values are deliberately a mixture: the ones this crate + /// acts on, so the edit paths are exercised, and ones it has never heard + /// of, so the keeping paths are. The values include the ones that have + /// caught it out before - a word where a number was expected, an + /// ampersand, an empty string. + fn awkward_document(seed: u64) -> String { + const ELEMENTS: [&str; 10] = [ + "entity", + "pilot", + "location", + "armor", + "slot", + "force", + "quirks", + "bombs", + "escapedCrew", + "somethingNew", + ]; + const NAMES: [&str; 10] = [ + "chassis", + "model", + "type", + "index", + "points", + "shots", + "isDestroyed", + "gunnery", + "camoCategory", + "aFieldFromTheFuture", + ]; + const VALUES: [&str; 10] = [ + "Destroyed", + "N/A", + "0", + "-1", + "99999", + "", + "Loki (Hellbringer)", + "Bob & Sons", + "a \"quoted\" thing", + "", + ]; + let mut rng = Rng(seed | 1); + let mut out = String::from("\n"); + if rng.upto(2) == 0 { + out.push_str("\n"); + } + out.push_str("\n"); + + for _ in 0..rng.upto(4) + 1 { + let mut depth = 0; + for _ in 0..rng.upto(8) + 1 { + match rng.upto(6) { + 0 if depth > 0 => { + depth -= 1; + out.push_str("\n"); + // Only entities are opened as containers below, so the + // close is always theirs. + } + 1 => out.push_str("\n"), + 2 => out.push_str("some text\n"), + 3 => { + out.push_str("\n"); + depth += 1; + } + _ => { + out.push('<'); + out.push_str(rng.pick(&ELEMENTS)); + attributes(&mut rng, &NAMES, &VALUES, &mut out); + out.push_str("/>\n"); + } + } + } + for _ in 0..depth { + out.push_str("\n"); + } + } + out.push_str("\n"); + out + } + + /// A few attributes, each named once: a repeated name is not XML, and a + /// parser is right to refuse it rather than pick one. + fn attributes(rng: &mut Rng, names: &[&str], values: &[&str], out: &mut String) { + let mut used: Vec<&str> = Vec::new(); + for _ in 0..rng.upto(4) { + let name = *rng.pick(names); + if used.contains(&name) { + continue; + } + used.push(name); + out.push_str(&format!(" {name}=\"{}\"", escaped(rng.pick(values)))); + } + } + + /// Attribute values have to be legal XML before they can be parsed. + fn escaped(value: &str) -> String { + let mut out = String::new(); + for c in value.chars() { + match c { + '&' => out.push_str("&"), + '<' => out.push_str("<"), + '>' => out.push_str(">"), + '"' => out.push_str("""), + _ => out.push(c), + } + } + out + } + + /// Two thousand documents this crate has never seen, each read, written + /// and read again. + /// + /// Every loss found by hand so far was found by looking somewhere new - + /// attributes, then elements beside the units, then the version, then the + /// order of locations, then comments, then the notice above the document. + /// Six times the answer was that nobody had looked there yet. This looks + /// in places nobody chose. + #[test] + fn documents_nobody_wrote_survive_a_round_trip() { + for seed in 1..=2000 { + let xml = awkward_document(seed); + if let Err(why) = survives_structurally(&xml) { + panic!("seed {seed} lost something:{why}\n\n{xml}"); + } + } } // `type` must not match `crewType`, and `armor` must not match