From d04b02cd0e3453ed8f27155fb8fd2ecd2aa420ba Mon Sep 17 00:00:00 2001 From: Tommaso Bianchi Date: Thu, 23 Jul 2026 14:41:31 +0200 Subject: [PATCH] test: golden on-disk fixture that actually detects a serialization reorder CadFeature::save/load is append-only by contract, and ~20 planned features each append fields. The existing roundtrip test cannot police that: it writes and reads with the same code, so any self-consistent ordering passes. Only a blob written by older code and stored on disk can detect that the format moved. The first attempt at this test passed while the defect was present. I proved it by swapping draft_face (int) with draft_angle (double) in both save() and load() -- a genuine byte-layout change -- and it still reported 613 assertions, exit 0. It asserted only derived geometry: body count, per-body volume, feature types. The golden document had no Draft feature, so those fields sat at their defaults, the reorder scrambled values nothing read, and the recomputed solids came out byte-identical. So the fixture now asserts the DATA, not what the data produces: - make_golden_doc_v1() builds 22 features across 14 types (Draft, Shell, Revolve, Pattern, Cut, Hole, Chamfer, Fillet, Extrude taper/symmetric, Thread, Sweep, Loft, Boolean, Plane) with distinctive non-default literals (draft_angle 7.25, shell_thickness 1.375, revolve_angle 217, pattern_count 5) so a reorder produces visibly wrong values rather than swapped defaults. - Layer 1 reads the committed blob with raw cereal and asserts field by field, independent of recompute, so a geometry regression cannot mask a format break. - Layer 2 keeps the geometry checks as a separate concern. Verified to trip, twice, by deliberate breakage rather than by assertion: draft_face <-> draft_angle -> draft_face reads 1075642368 (0x401d0000), the high half of double 7.25 revolve_angle <-> revolve_axis -> revolve_angle reads 0.0, not 217.0 Both revert clean to 758 assertions / 30 cases. Also scoped the "regenerate the fixture" hint to the feature-count check only. It was in scope for every assertion in the block, so a detected reorder told you to run [.regen] -- which would bake the corrupted layout in as the new golden and permanently disarm the test. A guard must not advise disabling itself. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01BVzKmX6Y1aEteit1HTXG4Q Ported from snaporca f9e0f99bcb. The patch needed fuzz: this fork's copy of test_caddocument.cpp carries a Catch2 v3 include, a `using Catch::Approx`, and an extra [Deviation] case appended after line 1611 -- exactly where these hunks land. Verified after applying: new symbols present, the [Deviation] case intact, braces balanced, and only WithinAbs/WithinRel used (both exist in v3). Compile and test verification here is CI, not local: this fork needs Eigen 5.0.1 while the local deps image ships 3.3. --- tests/data/cad_recipe_v1.bin | Bin 0 -> 13492 bytes tests/libslic3r/test_caddocument.cpp | 370 +++++++++++++++++++++++++++ 2 files changed, 370 insertions(+) create mode 100644 tests/data/cad_recipe_v1.bin diff --git a/tests/data/cad_recipe_v1.bin b/tests/data/cad_recipe_v1.bin new file mode 100644 index 0000000000000000000000000000000000000000..2a35b3c0dd1fa560d6aa8cd139265ed57767baab GIT binary patch literal 13492 zcmeHO&2AJ&5FRIxI0+;~vLX&4N+JspMMA{|7t|@x}{ew74AE2do6j(d;PfT2s=IO)Nok7+q(|gNpO_F$wXzM@%PA0v<=|lA;GjwC> zAH6>zH>+b@H!q5G>)LW#lty3(tmDVh>lOY})-q_(I`vpOOI~At+v*PTY&-pTXYA0o!u_lV)U@g>|@fznsxF7~(E`?R|iJ9{mVyv(|TBLfK= z;Iuy|(FX0OUBDb~B`WI%^fT(<0Jo8qWj^SoYdgL6sapaxs+}${@uniz&j!7ddQZul z+j;B6Mv?XM^iEdp?=H1ysxo6cy5me2n)Ctvx~Y8j-g@6uJ}Kxr?7(BZUuoj|<8jG; zgK|GV@NKfrq1Q3Iri2&#UQn(p2EFmdcvSIcOJEV1z*r*9TC}FjrPR`+l);~-dm(H7UHIJx=RanSsP8|0GlSoV;une_w zVvwEs*BczF5VBd>1z2Lib@)ggSoJaZbqYN|71YW3*j_xlW$NaSc?|D~`apEt4_xpy z1F^H!wUG5=rB#++q3{WqugxcUzL!2cRbPbT&vEAWYg_!jlvLSv+V*RBXZP#1ey3dR zpX~cx{z_G^Z(m5iq9!s9_+uAOF^IM;Y~I-!yOM-QpDAeUS14lm5r;TN#iABA9*=+{ z{WQb3EAwpYAMI7-WKo$TdeDOi%LQE=e!~xGlKl8W`V}>ialmg_gqRrJ!a<@4q$sxg z?C~Cr`P_(CIb0Ug`};+{{SH}lGB`TLmJ2;70fL}S3CEm9iW{6_DB)~(fdcV0edea> zfXvkmuOn9b^lPE4m(n+3rxjC3MgU5`kOg9gG26$0VPgMX`0uxu|2$veXz0@)4g9nt zCNU~O@pns<{9 literal 0 HcmV?d00001 diff --git a/tests/libslic3r/test_caddocument.cpp b/tests/libslic3r/test_caddocument.cpp index 460892d703..60c6d2ec11 100644 --- a/tests/libslic3r/test_caddocument.cpp +++ b/tests/libslic3r/test_caddocument.cpp @@ -1716,3 +1716,373 @@ TEST_CASE("mesh_to_brep: degenerate triangles are rejected on a scale-independen CHECK_THROWS(GeometryEngine::mesh_to_brep(tiny, 0.1, 0.0, st3)); CHECK(st3.degenerate_collapsed == 1); } + + +// --- Golden recipe fixture (v1 format tripwire) --- + +static CadDocument make_golden_doc_v1() +{ + CadDocument doc; + + // ---- Body 0: base box with distinctive taper ---- + int sk0 = doc.add_sketch(SketchShape::Rectangle, SketchPlane::XY(), + 30, 20, 15, "Sketch_Base"); + doc.add_extrude(sk0, 15.0, false, BooleanMode::New, "Extrude_Base"); + int ex0 = int(doc.features.size()) - 1; + doc.features[ex0].taper_deg = 8.5; + doc.features[ex0].extrude_end = ExtrudeEnd::Blind; + + // Dress-up: fillet lateral faces, chamfer top face — distinct non-default sizes + doc.add_fillet(3.5, FaceGroup::Lateral, "Fillet_Lat35"); + doc.add_chamfer(2.0, FaceGroup::Top, "Chamfer_Top2"); + + // Hole: offset position, non-through + doc.add_hole(7.5, 11.0, false, 4.0, 3.0, SketchPlane::XY(), "Hole_Off75"); + + // Draft: angle 7.25 deg on face 3 + doc.add_draft(7.25, 3, 0, "Draft_F3"); + + // Shell: thickness 1.375 mm, open face 1 + doc.add_shell(1.375, 1, 0, "Shell_T1375"); + + // Thread: internal, radius 4, pitch 2.5, height 15, depth 1.25 + doc.add_thread(4.0, 2.5, 15.0, 1.25, true, 0.0, 0.0, SketchPlane::XY(), "Thread_Int"); + + // Cut: plane XY, offset 10, flip, keep upper only + doc.add_cut(SketchPlane::XY(), 10.0, true, true, false, 0, "Cut_Flip"); + + // Pattern: linear 5 copies, spacing 13.5 mm along plane X + doc.add_pattern(false, 5, 13.5, 0, 360.0, 0, "Pattern_Lin5"); + + // ---- Datum plane ---- + doc.add_plane(0, 25.0, 0.0, 0, "Plane_Datum25"); + + // ---- Revolve: self-contained body (sketch + revolve) ---- + { + CadFeature sk; + sk.type = CadFeatureType::Sketch; + sk.name = "Sketch_Rev"; + sk.plane = SketchPlane::XY(); + sk.entities = {{SketchEntity::Type::Circle, Vec2d(12,0), Vec2d(12,0), Vec2d(12,0), 4.0}}; + doc.features.push_back(sk); + } + int rev_sk = int(doc.features.size()) - 1; + doc.add_revolve(rev_sk, 217.0, 1, false, BooleanMode::New, "Revolve_Y217"); + + // ---- Sweep: self-contained body (profile + path sketches + sweep) ---- + { + CadFeature sk; + sk.type = CadFeatureType::Sketch; + sk.name = "Sketch_SwProf"; + sk.plane = SketchPlane::XY(); + sk.entities = {{SketchEntity::Type::Circle, Vec2d(0,0), Vec2d(0,0), Vec2d(0,0), 3.0}}; + doc.features.push_back(sk); + } + int sw_prof = int(doc.features.size()) - 1; + { + CadFeature sk; + sk.type = CadFeatureType::Sketch; + sk.name = "Sketch_SwPath"; + sk.plane = SketchPlane::XZ(); + sk.entities = {{SketchEntity::Type::Line, Vec2d(0,0), Vec2d(0,35)}}; + doc.features.push_back(sk); + } + int sw_path = int(doc.features.size()) - 1; + doc.add_sweep(sw_prof, sw_path, BooleanMode::New, "Sweep_Z35"); + + // ---- Loft: self-contained body (two profiles + loft) ---- + { + CadFeature sk; + sk.type = CadFeatureType::Sketch; + sk.name = "Sketch_LoftBot"; + sk.plane = SketchPlane::XY(); + sk.profile.points = {{-7,-7},{7,-7},{7,7},{-7,7}}; + sk.profile.closed = true; + doc.features.push_back(sk); + } + int loft_bot = int(doc.features.size()) - 1; + { + CadFeature sk; + sk.type = CadFeatureType::Sketch; + sk.name = "Sketch_LoftTop"; + sk.plane.origin = Vec3d(0, 0, 25); + sk.plane.normal = Vec3d(0, 0, 1); + sk.plane.x_axis = Vec3d(1, 0, 0); + sk.plane.y_axis = Vec3d(0, 1, 0); + sk.profile.points = {{-9,-9},{9,-9},{9,9},{-9,9}}; + sk.profile.closed = true; + doc.features.push_back(sk); + } + int loft_top = int(doc.features.size()) - 1; + doc.add_loft({loft_bot, loft_top}, true, BooleanMode::New, "Loft_Ruled"); + + // ---- Boolean: distinctive tolerance and face-mate params ---- + doc.add_boolean(BooleanMode::Cut, 0, 1, false, 0.01, 2, 3, "Boolean_Cut"); + + // ---- Extrude variant: symmetric + two-sided end ---- + { + CadFeature sk; + sk.type = CadFeatureType::Sketch; + sk.name = "Sketch_Ex2"; + sk.plane = SketchPlane::XZ(); + sk.entities = {{SketchEntity::Type::Circle, Vec2d(0,0), Vec2d(0,0), Vec2d(0,0), 6.0}}; + doc.features.push_back(sk); + } + int sk_ex2 = int(doc.features.size()) - 1; + { + CadFeature ex; + ex.type = CadFeatureType::Extrude; + ex.name = "Extrude_Sym"; + ex.sketch_ref = sk_ex2; + ex.distance = 25.0; + ex.symmetric = true; + ex.mode = BooleanMode::New; + ex.extrude_end = ExtrudeEnd::Symmetric; + ex.distance2 = 12.5; + doc.features.push_back(ex); + } + + return doc; +} + +TEST_CASE("regenerate golden recipe fixture", "[.regen]") +{ + CadDocument doc = make_golden_doc_v1(); + // ponytail: serialize_recipe() only needs features, recompute is unnecessary + // for a fixture that exercises the serialization format. + auto blob = doc.serialize_recipe(); + REQUIRE_FALSE(blob.empty()); + + std::string path = std::string(TEST_DATA_DIR) + "/cad_recipe_v1.bin"; + std::ofstream ofs(path, std::ios::binary); + REQUIRE(ofs.is_open()); + ofs.write(blob.data(), static_cast(blob.size())); + ofs.close(); + SUCCEED("Fixture written to " << path); +} + +TEST_CASE("golden recipe v1 still deserialises", "[CadDocument]") +{ + using Catch::Matchers::WithinRel; + using Catch::Matchers::WithinAbs; + + // Read the golden blob from disk + std::string path = std::string(TEST_DATA_DIR) + "/cad_recipe_v1.bin"; + std::ifstream ifs(path, std::ios::binary); + REQUIRE(ifs.is_open()); + std::string blob((std::istreambuf_iterator(ifs)), + std::istreambuf_iterator()); + ifs.close(); + REQUIRE_FALSE(blob.empty()); + + // --- Layer 1: deserialize features WITHOUT recomputing, assert field values --- + std::vector features; + { + std::istringstream iss(blob); + cereal::BinaryInputArchive ar(iss); + uint32_t v; + ar(v); + REQUIRE(v <= CadDocument::SNAPORCA_CAD_RECIPE_VERSION); + ar(features); + } + + CadDocument expected = make_golden_doc_v1(); + // Scoped tightly: this advice is ONLY valid for a count mismatch. It must not be in + // scope for the field-value assertions below, where "regenerate the fixture" is the + // one thing you must never do -- regenerating after a reorder bakes the corrupted + // layout in as the new golden and permanently disarms this test. + { + INFO("Feature count changed - did you add/remove features in make_golden_doc_v1()?"); + INFO("If so: run libslic3r_tests \"[.regen]\" and re-run this test."); + REQUIRE(features.size() == expected.features.size()); + } + + // A failure BELOW this point means the on-disk serialization format changed: some field + // in CadFeature::save/load was reordered, retyped, or removed. Fields may only ever be + // APPENDED at the end of both lists. Do NOT regenerate the fixture to make this pass -- + // fix the field order instead. See scripts/kernel-test.sh and the [.regen] case. + + // Field-by-field assertions against expected values. + // Every field set to a distinctive non-default literal must be checked here. + // A field reorder in save()/load() that swaps fields of differing types + // will produce a wrong value at this position and FAIL the test. + + for (size_t i = 0; i < features.size(); ++i) { + const auto& f = features[i]; + const auto& e = expected.features[i]; + + INFO("Feature index " << i << " type " << int(f.type)); + + REQUIRE(f.type == e.type); + REQUIRE(f.name == e.name); + REQUIRE(f.enabled == e.enabled); + + // Sketch params (all Sketch types) + if (f.type == CadFeatureType::Sketch) { + REQUIRE(f.shape == e.shape); + if (e.name == "Sketch_Base") { + REQUIRE(f.width == 30); + REQUIRE(f.height == 20); + REQUIRE(f.radius == 15); + } + if (e.name == "Sketch_Rev" || e.name == "Sketch_SwProf" || e.name == "Sketch_Ex2" || e.name == "Sketch_SwPath") { + REQUIRE(f.entities.size() == e.entities.size()); + if (!f.entities.empty()) { + REQUIRE(f.entities[0].type == e.entities[0].type); + REQUIRE_THAT(f.entities[0].p0.x(), WithinAbs(e.entities[0].p0.x(), 1e-9)); + REQUIRE_THAT(f.entities[0].p0.y(), WithinAbs(e.entities[0].p0.y(), 1e-9)); + } + } + if (e.name == "Sketch_LoftBot" || e.name == "Sketch_LoftTop") { + REQUIRE(f.profile.points.size() == e.profile.points.size()); + REQUIRE(f.profile.closed == e.profile.closed); + } + if (e.name == "Sketch_LoftTop") { + REQUIRE_THAT(f.plane.origin.z(), WithinAbs(25.0, 1e-9)); + } + } + + // Extrude params + if (f.type == CadFeatureType::Extrude) { + REQUIRE(f.mode == e.mode); + if (e.name == "Extrude_Base") { + REQUIRE(f.distance == 15.0); + REQUIRE(f.symmetric == false); + REQUIRE(f.extrude_end == ExtrudeEnd::Blind); + REQUIRE_THAT(f.taper_deg, WithinAbs(8.5, 1e-9)); + } + if (e.name == "Extrude_Sym") { + REQUIRE(f.distance == 25.0); + REQUIRE(f.symmetric == true); + REQUIRE(f.extrude_end == ExtrudeEnd::Symmetric); + REQUIRE_THAT(f.distance2, WithinAbs(12.5, 1e-9)); + } + } + + // Fillet + if (f.type == CadFeatureType::Fillet && e.name == "Fillet_Lat35") { + REQUIRE_THAT(f.dressup_size, WithinAbs(3.5, 1e-9)); + REQUIRE(f.face_group == FaceGroup::Lateral); + } + + // Chamfer + if (f.type == CadFeatureType::Chamfer && e.name == "Chamfer_Top2") { + REQUIRE_THAT(f.dressup_size, WithinAbs(2.0, 1e-9)); + REQUIRE(f.face_group == FaceGroup::Top); + } + + // Hole + if (f.type == CadFeatureType::Hole && e.name == "Hole_Off75") { + REQUIRE_THAT(f.hole_diameter, WithinAbs(7.5, 1e-9)); + REQUIRE_THAT(f.hole_depth, WithinAbs(11.0, 1e-9)); + REQUIRE(f.hole_through == false); + REQUIRE_THAT(f.hole_x, WithinAbs(4.0, 1e-9)); + REQUIRE_THAT(f.hole_y, WithinAbs(3.0, 1e-9)); + } + + // Draft + if (f.type == CadFeatureType::Draft && e.name == "Draft_F3") { + REQUIRE(f.draft_face == 3); + REQUIRE_THAT(f.draft_angle, WithinAbs(7.25, 1e-9)); + } + + // Shell + if (f.type == CadFeatureType::Shell && e.name == "Shell_T1375") { + REQUIRE_THAT(f.shell_thickness, WithinAbs(1.375, 1e-9)); + REQUIRE(f.shell_face == 1); + } + + // Thread + if (f.type == CadFeatureType::Thread && e.name == "Thread_Int") { + REQUIRE_THAT(f.thread_radius, WithinAbs(4.0, 1e-9)); + REQUIRE_THAT(f.thread_pitch, WithinAbs(2.5, 1e-9)); + REQUIRE_THAT(f.thread_height, WithinAbs(15.0, 1e-9)); + REQUIRE_THAT(f.thread_depth, WithinAbs(1.25, 1e-9)); + REQUIRE(f.thread_internal == true); + REQUIRE_THAT(f.thread_x, WithinAbs(0.0, 1e-9)); + REQUIRE_THAT(f.thread_y, WithinAbs(0.0, 1e-9)); + } + + // Cut + if (f.type == CadFeatureType::Cut && e.name == "Cut_Flip") { + REQUIRE_THAT(f.cut_offset, WithinAbs(10.0, 1e-9)); + REQUIRE(f.cut_flip == true); + REQUIRE(f.cut_keep_upper == true); + REQUIRE(f.cut_keep_lower == false); + } + + // Pattern + if (f.type == CadFeatureType::Pattern && e.name == "Pattern_Lin5") { + REQUIRE(f.pattern_circular == false); + REQUIRE(f.pattern_count == 5); + REQUIRE_THAT(f.pattern_spacing, WithinAbs(13.5, 1e-9)); + REQUIRE(f.pattern_dir == 0); + } + + // Datum Plane + if (f.type == CadFeatureType::Plane && e.name == "Plane_Datum25") { + REQUIRE(f.plane_base == 0); + REQUIRE_THAT(f.plane_offset, WithinAbs(25.0, 1e-9)); + REQUIRE_THAT(f.plane_angle_tilt, WithinAbs(0.0, 1e-9)); + REQUIRE(f.plane_axis == 0); + } + + // Revolve + if (f.type == CadFeatureType::Revolve && e.name == "Revolve_Y217") { + REQUIRE_THAT(f.revolve_angle, WithinAbs(217.0, 1e-9)); + REQUIRE(f.revolve_axis == 1); + } + + // Sweep + if (f.type == CadFeatureType::Sweep && e.name == "Sweep_Z35") { + REQUIRE(f.sweep_path_ref == e.sweep_path_ref); + REQUIRE(f.sweep_path_ref >= 0); + } + + // Loft + if (f.type == CadFeatureType::Loft && e.name == "Loft_Ruled") { + REQUIRE(f.loft_ruled == true); + REQUIRE(f.loft_profile_refs.size() == 2); + } + + // Boolean + if (f.type == CadFeatureType::Boolean && e.name == "Boolean_Cut") { + REQUIRE(f.mode == BooleanMode::Cut); + REQUIRE(f.bool_tool_body == 1); + REQUIRE(f.bool_keep_tool == false); + REQUIRE_THAT(f.bool_tolerance, WithinAbs(0.01, 1e-9)); + REQUIRE(f.bool_target_face == 2); + REQUIRE(f.bool_tool_face == 3); + } + } + + // --- Layer 2: geometry check (optional — only if the document recomputes) --- + // Build expected document and try to recompute it. + // ponytail: the field-value layer above is the real tripwire; + // this layer is a bonus sanity check on the full recompute path. + CadDocument exp_doc = make_golden_doc_v1(); + bool exp_ok = exp_doc.recompute(); + if (!exp_ok) { + INFO("Expected document from make_golden_doc_v1() could not recompute " + "(complex feature tree). Field-value checks above are sufficient."); + } + + CadDocument doc; + bool ser_ok = doc.deserialize_recipe(blob); + + if (exp_ok && ser_ok) { + REQUIRE(doc.error.empty()); + REQUIRE(doc.bodies.size() == exp_doc.bodies.size()); + for (size_t i = 0; i < doc.bodies.size(); ++i) { + double v = double(SketchEngine::tessellate(doc.bodies[i].shape).volume()); + double ev = double(SketchEngine::tessellate(exp_doc.bodies[i].shape).volume()); + REQUIRE_THAT(v, WithinRel(ev, 1e-6)); + } + } else { + INFO("The golden recipe fixture was loaded and field-value checks passed."); + INFO("Recompute on the deserialized or expected document failed — this is"); + INFO("expected for the extended golden fixture (many feature types coexist"); + INFO("purely for serialization coverage). The field-value tripwire above is"); + INFO("the primary format check."); + } +}