From ae3740a0503b8ff59e69ddde1a5002bc341d2a48 Mon Sep 17 00:00:00 2001 From: Meta Vens Date: Mon, 12 Oct 2026 06:38:09 +0200 Subject: [PATCH] Fix s2 findings: product validate/inspect/key-collision/price-1 tests; ship_cc.read drops the zero-byte compensator ship_cc.read lets cultcache-py refuse a zero-byte file (CultLib pins that); the test expects the refusal. New behavioural tests: ValidateJudgesProductsOnADraftAndInspectListsThem (through ShipAuthoringCommands.Run), AProductKeyAlreadyInTheCatalogExcludesThePackage, and price 1 in ASoldPackageHullNeedsAPrice. Tests only plus the add-on deletion; all commits build. Co-Authored-By: Claude Opus 5.5 --- .../Aetheria.Shared.Tests/ShipProductTests.cs | 50 +++++++++++++++++++ tools/blender/aetheria_ships/ship_cc.py | 3 +- tools/blender/tests/test_ship_cc.py | 5 +- 3 files changed, 54 insertions(+), 4 deletions(-) diff --git a/tests/Aetheria.Shared.Tests/ShipProductTests.cs b/tests/Aetheria.Shared.Tests/ShipProductTests.cs index 76e51bb43..21b8c8bbc 100644 --- a/tests/Aetheria.Shared.Tests/ShipProductTests.cs +++ b/tests/Aetheria.Shared.Tests/ShipProductTests.cs @@ -113,6 +113,27 @@ public sealed class ShipProductTests : IDisposable Assert.Contains("manufacturer", excluded.Reason); } + [Fact] + public void AProductKeyAlreadyInTheCatalogExcludesThePackage() + { + // A catalog record already at this ship's product key for this maker, under another name. + using (var shipped = AetheriaStores.Open(Shipped, catalogWritable: true)) + { + var held = Product("Held", _acme, "mod.keyed"); + shipped.UpsertAsync(typeof(FactionProductData), held, ShipModCatalog.ProductKey("mod.keyed", _acme)).GetAwaiter().GetResult(); + shipped.FlushAsync().Wait(); + } + Package("mod.keyed", "Keyedhull", 100, Product("Fresh Name", _acme, "mod.keyed")); + Package("mod.fine", "Finehull", 100, Product("Fine", _acme, "mod.fine")); + + var composition = ShipModCatalog.Compose(Shipped, Derived, Mods); + + Assert.Equal(new[] { "mod.fine" }, composition.Included); + var excluded = Assert.Single(composition.Excluded); + Assert.Equal("mod.keyed", excluded.Package); + Assert.Contains("product key", excluded.Reason); + } + [Fact] public void AProductNameCollisionExcludesThePackage() { @@ -173,6 +194,9 @@ public sealed class ShipProductTests : IDisposable Assert.Contains("price above 0", Assert.Throws(() => ShipAuthoringStore.Read(sold)).Message); Assert.Single(ShipAuthoringStore.Load(sold).Products); + // The smallest sellable price: the rule is "above 0", so 1 passes. + Assert.Single(ShipAuthoringStore.Read(Package("mod.cheap", "Cheap", 1, Product("Cheap Skiff", _acme, "mod.cheap"))).Products); + Assert.Empty(ShipAuthoringStore.Read(Package("mod.console", "Console", 0)).Products); Assert.Single(ShipAuthoringStore.Read(Package("mod.priced", "Priced", 5, Product("Priced Skiff", _acme, "mod.priced"))).Products); } @@ -256,4 +280,30 @@ public sealed class ShipProductTests : IDisposable Assert.Empty(ShipAuthoringStore.Load(path).Products); RefusedAndUntouched("product", path, "ACM", "--remove"); } + + [Fact] + public void ValidateJudgesProductsOnADraftAndInspectListsThem() + { + Environment.CurrentDirectory = _root.Path; + string Refusal(string path) + { + var (code, _, error) = Run("validate", path); + Assert.Equal(1, code); + return error; + } + + Assert.Contains("price above 0", Refusal(Package("mod.free", "Free", 0, Product("Free Skiff", _acme, "mod.free")))); + Assert.Contains("needs a name", Refusal(Package("mod.blank", "Blank", 100, Product(" ", _acme, "mod.blank")))); + Assert.Contains("unique", Refusal(Package("mod.twice", "Twice", 100, Product("Same", _acme, "mod.twice"), Product("Same", _rival, "mod.twice")))); + + var sound = Package("mod.sound", "Sound", 100, Product("Sound Skiff", _acme, "mod.sound"), Product("Sound Rebrand", _rival, "mod.sound")); + var (validated, output, _) = Run("validate", sound); + Assert.Equal(0, validated); + Assert.Contains("records only", output); + + var (inspected, listing, _) = Run("inspect", sound); + Assert.Equal(0, inspected); + Assert.Contains($"product {_acme.Value}: Sound Skiff", listing); + Assert.Contains($"product {_rival.Value}: Sound Rebrand", listing); + } } diff --git a/tools/blender/aetheria_ships/ship_cc.py b/tools/blender/aetheria_ships/ship_cc.py index 50745ea3b..84f1747ee 100644 --- a/tools/blender/aetheria_ships/ship_cc.py +++ b/tools/blender/aetheria_ships/ship_cc.py @@ -88,8 +88,7 @@ class ShipFile(NamedTuple): def read(path: str, cultlib_packages: str) -> ShipFile: cultcache, msgpack = _libraries(cultlib_packages) store = cultcache.SingleFileMessagePackBackingStore(path) - # cultcache-py raises an Unpack error on a zero-byte file; an empty file holds no records, so the shape refusal below names it. - envelopes = [] if Path(path).is_file() and Path(path).stat().st_size == 0 else store.pull_all() + envelopes = store.pull_all() ships = [envelope for envelope in envelopes if envelope.type == SCHEMA] hulls = [envelope for envelope in envelopes if envelope.type == HULL_SCHEMA] # Before S1 the hull lived inside the ship record. Saving such a file would preserve the stale hull under a ship that diff --git a/tools/blender/tests/test_ship_cc.py b/tools/blender/tests/test_ship_cc.py index b0806544c..0a07f8377 100644 --- a/tools/blender/tests/test_ship_cc.py +++ b/tools/blender/tests/test_ship_cc.py @@ -153,10 +153,11 @@ class ReadTests(ShipFileCase): with self.assertRaisesRegex(ValueError, "must hold exactly one"): ship_cc.read(self.path, PACKAGES) - def test_refuses_a_file_with_no_record(self): + def test_refuses_a_zero_byte_file_as_cultcache_does(self): + # An empty file is not a store (CultLib pins this): the owner's refusal passes through, the add-on adds none. empty = str(Path(self.path).with_name("empty.cc")) Path(empty).write_bytes(b"") - with self.assertRaisesRegex(ValueError, "must hold exactly one"): + with self.assertRaises(ValueError): ship_cc.read(empty, PACKAGES) def test_refuses_a_ship_record_that_still_carries_the_retired_embedded_hull(self):