test(fixtures): introduce zspec Factory pattern for ProjectConfig + ResourceDef#121
Conversation
…esourceDef 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) <noreply@anthropic.com>
PR SummaryLow Risk Overview Updates project file tests to build resources/configs via these factories (removing manual arena string duplication) and adds a new save/load round-trip test asserting factory overrides persist and non-overridden defaults remain. Bumps Reviewed by Cursor Bugbot for commit df4b631. Bugbot is set up for automated code reviews on this repo. Configure here. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the zspec dependency to version 0.9.2 and introduces a factory pattern for test fixtures to centralize default data and simplify test setup. It includes new fixture definitions and refactors existing tests to use these factories, ensuring better maintenance and reduced boilerplate. The reviewer suggested updating the version prefix in the zspec hash for consistency.
| .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", |
There was a problem hiding this comment.
The hash prefix for zspec still references version 0.9.1 while the URL has been updated to v0.9.2. While Zig's package manager primarily uses the multihash suffix for integrity, having a version mismatch in the hash string is inconsistent and can be confusing for maintenance. It is recommended to update the prefix to match the new version.
.hash = "zspec-0.9.2-jaKLbXgMBACFwbNjflhbMyP113leoYjboxn-1UOP-FGw",
There was a problem hiding this comment.
Reply: tried this — Zig 0.16's package manager rejects the edit with hash mismatch: manifest declares zspec-0.9.2-... but the fetched package has zspec-0.9.1-.... The version prefix is sourced from upstream's own build.zig.zon .version field (still "0.9.1" at the v0.9.2 tag); only the base64 suffix is integrity-relevant. Left the prefix as-is with a comment in build.zig.zon explaining. The real fix is an upstream version bump — filed mentally for next zspec patch.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit fe9f1f2. Configure here.
…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) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR introduces shared zspec-based test fixtures (Factory pattern) to reduce repetitive manual arena.dupe setup in tests, while bumping the zspec dependency to pick up upstream fixes.
Changes:
- Bump
zspecdependency to v0.9.2. - Add
src/test_fixtures.zigplus a.zonfixture to define reusableResourceDefandProjectConfigfactories. - Update project save/load tests to use the factories and add a new “ProjectConfig overrides round-trip” test.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/tests.zig |
Switch resource test setup to ResourceFactory and add a new config override round-trip test using ProjectConfigFactory. |
src/test_fixtures.zig |
New shared module defining zspec factories for common test data. |
src/test_fixtures/resource.zon |
New ZON fixture backing the ResourceDef factory defaults. |
build.zig.zon |
Update zspec dependency URL/hash for v0.9.2. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| .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", |
There was a problem hiding this comment.
Reply: Zig 0.16's package manager rejects this edit — hash mismatch: manifest declares zspec-0.9.2-... but the fetched package has zspec-0.9.1-.... The prefix is baked from upstream's build.zig.zon .version field, which still reads "0.9.1" at the v0.9.2 tag. Only the base64 portion is integrity-checked; the prefix is cosmetic metadata sourced from upstream. Documented in a comment on the field — see df4b631.
| 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 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. |
- 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) <noreply@anthropic.com>
Minor release rolling up the post-0.2.1 PIE viewport editor work: - #107 / #111 — Game View panel (SHM consumer + ImGui::Image). - #108 / #115 — macOS IOSurface zero-copy consumer module. - #112 / #113 — TCP transport restore (JSON control plane). - #119 — Binary plane decoder (entity/component/flow frames). - #109 / #122 — ImGui Test Engine coverage of the Game View panel. - #120 / #123 — Triage of 5 long-silent-passing TE tests. - #121 — zspec Factory pattern for ProjectConfig + ResourceDef fixtures. - #124 — Dispatch iosurface consumer on `frame_offer.format`, FBO blit for `GL_TEXTURE_RECTANGLE → GL_TEXTURE_2D` so imgui_impl_opengl3 can display the zero-copy texture. Plus several Zig 0.16 compat fixes (preview transport restore, gizmos empty-index test, TE pub-fn gating, io.Threaded deadlock workarounds) and ci: gui-tests N>0 assertion (#118). Paired downstream: labelle-engine v1.37.3 (IOSurface producer) and labelle-assembler 0.19.0 (raylib PBO publishFrame). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
.simple-runnerio_instanceinit fix).src/test_fixtures.zigwith two factory styles:Factory.defineFromfor leaf types (ResourceDefviatest_fixtures/resource.zon) andFactory.definefor outer types with slice fields (ProjectConfig).resources round-trip through save + loadto build itsResourceDefviaResourceFactory.build(.{}), dropping the manualarena.dupe-per-field ceremony.ProjectConfigFactory overrides round-trip through save + loadtest 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 TE tests: triage the 5 long-silent-passing failures uncovered by #105/#106 #120'sproject_settings_edit_saveshape.gui_tests_exe.root_module.addImport("nfd", ...)that feat(iosurface): macOS zero-copy consumer module (#108 — phase 2) #115's review-fix rebase accidentally dropped. main has been red on Linux CI since feat(iosurface): macOS zero-copy consumer module (#108 — phase 2) #115 merged because of this; lands in this PR to unblock both.Why factories
project_settings_edit_save(one of the 5 long-silent-passing tests #120 will triage) needs a populatedProjectConfigbuilt per-test. Today's tests either run on defaults or hand-build the struct witharena.dupeper string field. Factories collapse that toFactory.build(.{ ...overrides })with comptime field-name validation. This PR proves the pattern on existing tests before #120 starts.Findings worth carrying forward
Factory.defineFrom(T, @import("…zon"))is clean for leaf types — a typo in the.zonfixture fails the build, not the test.defineFromcan't coerce.zon's empty struct literal.{}to a typed empty slice ([]const T). For outer types with slice fields, useFactory.define(T, .{ … })in Zig source where@as([]const X, &.{})is expressible. Thetest_fixtures.zigmodule doc-comment captures this so anyone picking up PIE viewport: imgui_test_engine coverage (state-mirror tests) #109/TE tests: triage the 5 long-silent-passing failures uncovered by #105/#106 #120 sees the guidance.Test plan
zig build test— 175/175 pass (was 174 on v0.9.2 baseline; +1 for the new factory-override round-trip test).zig build gui-test-buildsucceeds locally after the nfd-import restore.