From fe9f1f2f0cc39eb347f3927e409f741c963d86b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandre=20Mondaini=20Calva=CC=83o?= Date: Fri, 15 May 2026 18:33:01 -0300 Subject: [PATCH 1/3] test(fixtures): introduce zspec Factory pattern for ProjectConfig + ResourceDef MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bumps zspec to v0.9.2 — picks up the `.simple`-runner io_instance init fix shipped this session (apotema/zspec#45) — and lands a small proof-of-value slice for zspec's `Factory` module before tackling #120's TE-test triage and #109's Game View TE coverage. Why factories now: `project_settings_edit_save` (one of the 5 long- silent-passing tests #120 will triage) needs a populated ProjectConfig built per-test. Today's tests either run on defaults or hand-build the struct with arena.dupe per string field. Factories collapse that to `Factory.build(.{ ...overrides })` and validate field names at comptime. What changed: - `src/test_fixtures.zig` + `src/test_fixtures/resource.zon`: two factories, one via `Factory.defineFrom` (leaf type — `ResourceDef`, fixture in a .zon file with field-name typo detection) and one via `Factory.define` (outer type — `ProjectConfig`, in-Zig literal so the `resources: []const ResourceDef = &.{}` slice field can be expressed as `@as([]const ResourceDef, &.{})`. `defineFrom` can't coerce empty slices from .zon syntax; the module doc-comment captures this). - `src/tests.zig`: converts `resources round-trip through save + load` to build its `ResourceDef` via `ResourceFactory.build(.{})`, dropping the manual `arena.dupe` per field. Adds a sibling `ProjectConfigFactory overrides round-trip through save + load` that builds a 7-field-override config, round-trips through save/load, asserts every override survived and untouched fields kept their fixture defaults — direct precursor to #120's `project_settings_edit_save` shape. - `build.zig.zon`: zspec v0.9.1 → v0.9.2. Test plan: `zig build test` — 175/175 pass (was 174 on v0.9.2 baseline, +1 for the new factory-override round-trip test). Co-Authored-By: Claude Opus 4.7 (1M context) --- build.zig.zon | 4 +-- src/test_fixtures.zig | 44 +++++++++++++++++++++++++ src/test_fixtures/resource.zon | 5 +++ src/tests.zig | 60 +++++++++++++++++++++++++++++----- 4 files changed, 103 insertions(+), 10 deletions(-) create mode 100644 src/test_fixtures.zig create mode 100644 src/test_fixtures/resource.zon diff --git a/build.zig.zon b/build.zig.zon index fc5f29c..c6855bf 100644 --- a/build.zig.zon +++ b/build.zig.zon @@ -21,8 +21,8 @@ .hash = "nfd-0.1.0-y82kbsSLBgCxj9gDvxK7fxPFYnI3oe2wWx6M9ykp9Gio", }, .zspec = .{ - .url = "https://github.com/apotema/zspec/archive/v0.9.1.tar.gz", - .hash = "zspec-0.9.1-jaKLbbX4AwBKANdetxzzWc3UTO0UY0lcJJzTagQHlt5K", + .url = "https://github.com/apotema/zspec/archive/v0.9.2.tar.gz", + .hash = "zspec-0.9.1-jaKLbXgMBACFwbNjflhbMyP113leoYjboxn-1UOP-FGw", }, .zstbi = .{ .url = "git+https://github.com/zig-gamedev/zstbi?ref=main#3813f5f113b26f644cfa01a30155cc76b3526e91", diff --git a/src/test_fixtures.zig b/src/test_fixtures.zig new file mode 100644 index 0000000..b168030 --- /dev/null +++ b/src/test_fixtures.zig @@ -0,0 +1,44 @@ +//! zspec Factory definitions shared across tests. +//! +//! Pattern recap (informs #109/#120 follow-up work): +//! +//! * `Factory.defineFrom(T, @import("…zon"))` — concise, fixture in +//! a separate `.zon` file, field-name typo detection at comptime. +//! Works only when every field of T has a concrete value the zon +//! literal can coerce to. Slice fields (`[]const X = &.{}`) hit the +//! `@as(FieldType, .{})` coercion path and fail to compile, so +//! reserve `defineFrom` for leaf types like `ResourceDef`. +//! +//! * `Factory.define(T, .{ ...defaults })` — same comptime defaults, +//! authored in Zig source. Necessary when T has slice or pointer +//! fields because we can write `&.{}` and `@as([]const X, &.{})` +//! explicitly. Use for outer types like `ProjectConfig`. +//! +//! Strings inside the defaults are static (comptime). `.build({})` +//! returns a value whose strings outlive any test arena, so the +//! resulting `ResourceDef` / `ProjectConfig` can be assigned directly +//! to a project arena-owned slice without per-field `dupe()` ceremony. +const zspec = @import("zspec"); +const Factory = zspec.Factory; +const project = @import("project.zig"); + +const resource_zon = @import("test_fixtures/resource.zon"); + +pub const ResourceFactory = Factory.defineFrom(project.ResourceDef, resource_zon); + +pub const ProjectConfigFactory = Factory.define(project.ProjectConfig, .{ + .name = "factory_project", + .description = "", + .title = "Factory Project", + .width = 1280, + .height = 720, + .target_fps = 60, + .backend = .raylib, + .ecs = .zig_ecs, + .initial_scene = "main", + .core_version = "1.12.0", + .engine_version = "1.35.0", + .gfx_version = "1.10.0", + .assembler_version = "0.17.0", + .resources = @as([]const project.ResourceDef, &.{}), +}); diff --git a/src/test_fixtures/resource.zon b/src/test_fixtures/resource.zon new file mode 100644 index 0000000..f225412 --- /dev/null +++ b/src/test_fixtures/resource.zon @@ -0,0 +1,5 @@ +.{ + .name = "sprites", + .json = "assets/sprites.json", + .texture = "assets/sprites.png", +} diff --git a/src/tests.zig b/src/tests.zig index a0b52e1..5cd73f7 100644 --- a/src/tests.zig +++ b/src/tests.zig @@ -19,6 +19,7 @@ const flow_types = @import("flows/types.zig"); const prefs = @import("prefs.zig"); const io_global = @import("io_global.zig"); const game_view = @import("game_view.zig"); +const test_fixtures = @import("test_fixtures.zig"); /// Wall-clock seconds since the Unix epoch; replacement for the /// `std.time.timestamp` helper removed in Zig 0.16. Used only to @@ -1886,9 +1887,11 @@ pub const ProjectFileTests = struct { const temp_dir = try createTempDir(allocator); defer deleteTempDir(allocator, temp_dir); - // Build a project with one resource directly via the arena so - // the slice is owned correctly. Save then reload via a fresh - // ProjectManager and assert the resource came back intact. + // Save a project carrying one Factory-built ResourceDef, reload + // via a fresh ProjectManager, assert the resource came back + // intact. Factory.defineFrom validates each field name against + // ResourceDef at comptime — a typo in `test_fixtures/resource.zon` + // fails the build, not the test. var pm = project.ProjectManager.init(allocator); defer pm.deinit(); try pm.newProject("with_resources"); @@ -1896,11 +1899,7 @@ pub const ProjectFileTests = struct { const proj = pm.current_project.?; const a = proj.arena.allocator(); const resources = try a.alloc(project.ResourceDef, 1); - resources[0] = .{ - .name = try a.dupe(u8, "sprites"), - .json = try a.dupe(u8, "assets/sprites.json"), - .texture = try a.dupe(u8, "assets/sprites.png"), - }; + resources[0] = test_fixtures.ResourceFactory.build(.{}); proj.config.resources = resources; try pm.saveProject(temp_dir); @@ -1916,6 +1915,51 @@ pub const ProjectFileTests = struct { try expect.toBeTrue(std.mem.eql(u8, loaded[0].texture, "assets/sprites.png")); } + test "ProjectConfigFactory overrides round-trip through save + load" { + // Proof-of-value for the Factory pattern (precursor to #120's + // `project_settings_edit_save` triage): build a non-default + // ProjectConfig with several overrides via Factory.build, + // round-trip through saveProject/loadProject, assert every + // override survived. Replaces what would otherwise be a + // multi-line struct literal plus arena.dupe per string field. + const allocator = std.testing.allocator; + const temp_dir = try createTempDir(allocator); + defer deleteTempDir(allocator, temp_dir); + + var pm = project.ProjectManager.init(allocator); + defer pm.deinit(); + try pm.newProject("factory_overrides"); + + const proj = pm.current_project.?; + proj.config = test_fixtures.ProjectConfigFactory.build(.{ + .name = "factory_overrides", + .title = "Factory Overrides", + .width = 1920, + .height = 1080, + .backend = .sokol, + .initial_scene = "splash", + .engine_version = "1.35.0", + }); + + try pm.saveProject(temp_dir); + + var pm2 = project.ProjectManager.init(allocator); + defer pm2.deinit(); + try pm2.loadProject(temp_dir); + + const cfg = pm2.current_project.?.config; + try expect.toBeTrue(std.mem.eql(u8, cfg.name, "factory_overrides")); + try expect.toBeTrue(std.mem.eql(u8, cfg.title, "Factory Overrides")); + try expect.equal(cfg.width, 1920); + try expect.equal(cfg.height, 1080); + try expect.equal(cfg.backend, .sokol); + try expect.toBeTrue(std.mem.eql(u8, cfg.initial_scene, "splash")); + try expect.toBeTrue(std.mem.eql(u8, cfg.engine_version, "1.35.0")); + // Unmodified-by-override fields keep their fixture defaults. + try expect.equal(cfg.ecs, .zig_ecs); + try expect.equal(cfg.target_fps, 60); + } + // Regression: ../flying-platform-labelle/project.labelle (and any // project authored by the assembler / CLI) carries fields like // `states`, `layers`, `plugins`, `gui`, `labelle_version`, etc. that From fec2ec950014eb1113c085b60e010896090812ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandre=20Mondaini=20Calva=CC=83o?= Date: Fri, 15 May 2026 18:56:20 -0300 Subject: [PATCH 2/3] fix(build): restore nfd import on gui_tests_exe (regression from #115 squash) main has been red on Linux CI since da97953 (#115 squash-merge) with: src/app.zig:11: error: no module named 'nfd' available within module 'root' The original #106 commit added `gui_tests_exe.root_module.addImport("nfd", nfd.module("nfd"))` because once TE callbacks were made `pub fn` (so `std.meta.hasFn` could find them), the `App.renderFrame` paths the tests exercise pulled in the nfd-using file-dialog code, which wasn't imported on the test target. When #115 was rebased / cherry-picked through review-fix cycles, that single line got dropped from build.zig. Subsequent merges of #115 itself and #119 inherited the broken state and CI started failing on every push without it being obvious whose change introduced it. This restores the addImport with an explanatory comment so future rebases preserve it. Co-Authored-By: Claude Opus 4.7 (1M context) --- build.zig | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/build.zig b/build.zig index 6c95d7f..f2def1a 100644 --- a/build.zig +++ b/build.zig @@ -172,6 +172,13 @@ pub fn build(b: *std.Build) void { gui_tests_exe.root_module.linkLibrary(zgui_te.artifact("imgui")); gui_tests_exe.root_module.addImport("zstbi", zstbi.module("root")); gui_tests_exe.root_module.addImport("flow_codegen", flow_codegen_module); + // App.renderFrame transitively imports nfd via the file-dialog + // code path; without this addImport the test binary fails to + // compile as soon as the previously-dead `Callbacks.gui`/`run` + // bodies are analyzed (this addition originally landed in #106 + // and was inadvertently dropped during #115's review-fix + // cherry-pick rebase). + gui_tests_exe.root_module.addImport("nfd", nfd.module("nfd")); gui_tests_exe.root_module.addImport("engine", engine_module); const run_gui_tests = b.addRunArtifact(gui_tests_exe); From df4b631b1cd5f1afcd9f0f5ca375e0e7e72e78f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandre=20Mondaini=20Calva=CC=83o?= Date: Fri, 15 May 2026 19:04:41 -0300 Subject: [PATCH 3/3] review(#121): address gemini, cursor, copilot bot comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - build.zig.zon: leave the `zspec-0.9.1-...` hash prefix as-is and explain why in a comment. Both gemini and copilot suggested changing it to `zspec-0.9.2-...` to match the URL, but Zig 0.16's package manager bakes upstream's own `build.zig.zon` `.version` into the hash and rejects manual edits ("hash mismatch: manifest declares zspec-0.9.2-... but the fetched package has zspec-0.9.1-..."). Upstream's `.version` still reads "0.9.1" at the v0.9.2 tag; the prefix is cosmetic metadata sourced from there. Only the base64 portion is integrity-relevant. - src/tests.zig (cursor + copilot): the `engine_version = "1.35.0"` override matched both the factory default and the ProjectConfig struct default, making it a no-op. Bumped to "1.36.0" so the assertion actually exercises an override path. - src/tests.zig (copilot): the "non-overridden fields keep their fixture defaults" comment claimed coverage of every untouched field but only checked `ecs` + `target_fps`. Expanded the assertions to also cover `description`, `core_version`, `gfx_version`, `assembler_version`, and `resources.len = 0`. `zig build test` — 180/180 pass. Co-Authored-By: Claude Opus 4.7 (1M context) --- build.zig.zon | 7 +++++++ src/tests.zig | 13 ++++++++++--- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/build.zig.zon b/build.zig.zon index c6855bf..9023926 100644 --- a/build.zig.zon +++ b/build.zig.zon @@ -22,6 +22,13 @@ }, .zspec = .{ .url = "https://github.com/apotema/zspec/archive/v0.9.2.tar.gz", + // Hash prefix says `0.9.1` because upstream's own + // `build.zig.zon` still declares `.version = "0.9.1"` at + // the v0.9.2 tag — Zig's package manager bakes that + // string into the hash and rejects manual edits with a + // "hash mismatch" error. The base64 portion is the only + // integrity-relevant part; the prefix is cosmetic + // metadata sourced from upstream. .hash = "zspec-0.9.1-jaKLbXgMBACFwbNjflhbMyP113leoYjboxn-1UOP-FGw", }, .zstbi = .{ diff --git a/src/tests.zig b/src/tests.zig index 5cd73f7..7203806 100644 --- a/src/tests.zig +++ b/src/tests.zig @@ -1938,7 +1938,7 @@ pub const ProjectFileTests = struct { .height = 1080, .backend = .sokol, .initial_scene = "splash", - .engine_version = "1.35.0", + .engine_version = "1.36.0", }); try pm.saveProject(temp_dir); @@ -1954,10 +1954,17 @@ pub const ProjectFileTests = struct { try expect.equal(cfg.height, 1080); try expect.equal(cfg.backend, .sokol); try expect.toBeTrue(std.mem.eql(u8, cfg.initial_scene, "splash")); - try expect.toBeTrue(std.mem.eql(u8, cfg.engine_version, "1.35.0")); - // Unmodified-by-override fields keep their fixture defaults. + try expect.toBeTrue(std.mem.eql(u8, cfg.engine_version, "1.36.0")); + // Non-overridden fields keep their ProjectConfigFactory + // defaults — proves the factory's defaults pass through + // saveProject + loadProject untouched. try expect.equal(cfg.ecs, .zig_ecs); try expect.equal(cfg.target_fps, 60); + try expect.toBeTrue(std.mem.eql(u8, cfg.description, "")); + try expect.toBeTrue(std.mem.eql(u8, cfg.core_version, "1.12.0")); + try expect.toBeTrue(std.mem.eql(u8, cfg.gfx_version, "1.10.0")); + try expect.toBeTrue(std.mem.eql(u8, cfg.assembler_version, "0.17.0")); + try expect.equal(cfg.resources.len, 0); } // Regression: ../flying-platform-labelle/project.labelle (and any