Repository navigation
test: Exercise both compiled and fallback writers in schema binary tests - #3228
mishablablabla wants to merge 4 commits into
Conversation
Add `writeWithBothWriters` to typegpu-testing-utility. It encodes a value with `writeToArrayBuffer` once with the compiled writer and once with the fallback writer (eval blocked while a fresh TypeGPU instance loads), checks that each path was really used, and fails if the encoded bytes differ. Migrate the write/roundtrip tests in arrayBufferIO, struct, array and vector. The `*.noEval.test.ts` files are left as they are.
|
pkg.pr.new packages benchmark commit |
Resolution Time Benchmark---
config:
themeVariables:
xyChart:
plotColorPalette: "#E63946, #3B82F6, #059669"
---
xychart
title "Random Branching (🔴 PR | 🔵 main | 🟢 release)"
x-axis "max depth" [1, 2, 3, 4, 5, 6, 7, 8]
y-axis "time (ms)"
line [1.09, 2.15, 4.49, 7.14, 7.93, 12.97, 26.34, 25.85]
line [1.08, 2.19, 4.50, 7.23, 8.05, 13.34, 25.79, 26.94]
line [1.01, 2.12, 4.83, 7.38, 8.28, 12.17, 27.08, 28.07]
---
config:
themeVariables:
xyChart:
plotColorPalette: "#E63946, #3B82F6, #059669"
---
xychart
title "Linear Recursion (🔴 PR | 🔵 main | 🟢 release)"
x-axis "max depth" [1, 2, 3, 4, 5, 6, 7, 8]
y-axis "time (ms)"
line [0.30, 0.53, 0.72, 0.85, 1.14, 1.25, 1.49, 1.69]
line [0.32, 0.57, 0.78, 0.90, 1.27, 1.27, 1.56, 1.70]
line [0.31, 0.52, 0.74, 0.95, 1.24, 1.27, 1.54, 1.67]
---
config:
themeVariables:
xyChart:
plotColorPalette: "#E63946, #3B82F6, #059669"
---
xychart
title "Full Tree (🔴 PR | 🔵 main | 🟢 release)"
x-axis "max depth" [1, 2, 3, 4, 5, 6, 7, 8]
y-axis "time (ms)"
line [1.00, 2.59, 4.28, 7.55, 14.26, 27.88, 58.31, 119.43]
line [0.94, 2.17, 3.70, 7.55, 13.84, 27.20, 60.49, 119.71]
line [1.04, 2.39, 4.53, 7.61, 13.61, 28.32, 60.38, 120.95]
|
Bundle size comparison (
|
| 🟢 Decreased | ➖ Unchanged | 🔴 Increased | ❔ Unknown |
|---|---|---|---|
| 0 | 355 | 0 | 0 |
No notable changes.
If you wish to run a comparison for other, slower bundlers, run the 'Tree-shake test' from the GitHub Actions menu.
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
This run reviewed the full PR (#3228, single commit 153c04aa) against main.
- New test helper
writeWithBothWriters— loads a freshtypegpuinstance per path viavi.resetModules(), blocksglobalThis.Functionat load to force the eval-disallowed fallback writer, countsFunctionconstructions during the write, captures[fallback]warnings, compares both byte outputs, and restores__TYPEGPU_*globals andconsole.warn. - Export wiring — the helper and its
BothWritersCase/BothWritersResulttypes are re-exported fromtypegpu-testing-utility. - Serialization test migrations —
array,arrayBufferIO,struct, andvectorwrite/roundtrip tests now go through the helper while keeping their existing byte and layout assertions; the f16 struct roundtrips were split out of the layout tests.
I verified the helper exercises what it claims: the compiled run asserts a writer was generated and no fallback warning fired, the fallback run asserts eval was probed while blocked, no code was generated during the write, and exactly one fallback warning was emitted. The fresh-instance requirement and typed-array fast-path limitation are documented clearly. I ran the four changed test files (227 passed) and oxlint / oxfmt --check on the changed files (clean).
( ◦°^°◦) Arr, I read the whole diff end to end, ran the four migrated test scullies (227 passin'), checked lint and format, and found no leaks in the hull. Approvin' this one.
deepseek-v4.1-flash (free via Pullfrog for OSS) ｜ 𝕏
There was a problem hiding this comment.
🟡 Changes recommended
Constructor restoration can overwrite caller-installed Function spies or stubs, breaking test isolation.
1 open issue
What changed in this PR
Adds shared test coverage comparing TypeGPU’s compiled and fallback binary writers.
Changes:
- Adds a helper that verifies writer selection and byte equality.
- Migrates serialization tests while preserving layout and roundtrip assertions.
- Adds global interception, cleanup, and serialized helper calls.
| File | Description |
|---|---|
| packages/typegpu/tests/vector.test.ts | Exercises both writers for vector serialization. |
| packages/typegpu/tests/unstruct.test.ts | Migrates loose-structure roundtrip tests. |
| packages/typegpu/tests/struct.test.ts | Migrates structure serialization and f16 coverage. |
| packages/typegpu/tests/matrix.test.ts | Migrates matrix roundtrip tests. |
| packages/typegpu/tests/disarray.test.ts | Checks both writers for packed arrays. |
| packages/typegpu/tests/arrayBufferIO.test.ts | Expands dual-writer input and layout coverage. |
| packages/typegpu/tests/array.test.ts | Migrates array layout and roundtrip tests. |
| packages/typegpu-testing-utility/src/index.ts | Exports the helper and its types. |
| packages/typegpu-testing-utility/src/bothWriters.ts | Implements writer verification and byte comparison. |
Review effort: Balanced
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
This run reviewed the delta since the prior Pullfrog review (153c04aa → 796d221), which extends the writeWithBothWriters migration to more schema serialization tests on top of a merge of main.
- Migrated
disarray,matrix, andunstructbinary serialization tests towriteWithBothWriters, keeping their size/alignment assertions in place. - Added
arrayBufferIOcases: "handles d.vec input", "handles plain array input", "handles structs with attributes" (alignedu32member), and "handles mat3x3f with matrix input"; migrated the read "handles vectors" / "handles structs" cases. - Split the f16
structtests into dedicated roundtrip tests while preserving the layoutits. - Migrated the remaining
vectorencode tests (vec2f/vec2i/vec3f/vec3i/vec4f/vec2h) and switched the f16 rounding expectation totypegpu.d. - Dropped now-unused imports (
Infer,readFromArrayBuffer/writeToArrayBuffer) from the migrated files.
Verified locally: the 7 changed test files pass (305 tests) and pnpm --filter typegpu test:types (tsc --noEmit) is clean. Each migration follows the helper's contract — schema and value are built inside build from the fresh instance, the compiled/fallback byte comparison is unchanged, and existing byte/layout assertions are retained.
( ◦°^°◦) Arr, I charted the new commits, read the whole diff fore-to-aft, ran the seven changed test scullies (305 passin'), and typechecked the cargo — nary a leak below the waterline. Ship it!
deepseek-v4.1-flash (free via Pullfrog for OSS) ｜ 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
This run re-reviewed the delta since the prior Pullfrog review (796d221 → 609e8dc), a single follow-up commit to the writeWithBothWriters test helper.
Functionrestore preserves a caller's spy —interceptFunctionConstructornow capturesglobalThis.Functionat install time and restores that exact value instead of the module-levelRealFunction, so aFunctionproxy or stub installed by a calling test survives a helper call.- Helper regression tests —
packages/typegpu/tests/internal/bothWriters.test.tsasserts the caller'sFunctionspy is still installed after both a successful and a throwing call, and that the original is restored (and the helper keeps working) after a queued call fails.
I verified the fix end to end: read the incremental range and the full diff, then ran the new helper tests (3 passed) and the seven migrated schema test files (305 passed) in source mode. The prior Copilot thread about constructor restoration overwriting caller spies is addressed by this commit.
( ◦°^°◦) 🏴☠️ Arr, the new commit patches the leak where the helper clobbered a caller's Function spy — I read the delta and the whole diff, ran the three new regression scullies (all passin') alongside the seven migrated ones, and found nary a loose cannon aboard. Approvin' this one.
deepseek-v4.1-flash (free via Pullfrog for OSS) ｜ 𝕏
Summary
Add a shared test helper that verifies binary serialization produces identical results with both compiled and fallback writers.
Changes
writeWithBothWriters()totypegpu-testing-utility.arrayBufferIO,struct,array, andvector.Validation
typegpuproject — 2,917 tests passed.TEST_BUILT=1):typegpuproject — 2,448 tests passed.git diff --checkpassed.Scope
This change focuses on schema-based binary serialization tests.
patch,buffer.write, other schema test files, and existing*.noEval.test.tstests are not included.