dx: More robust snippet testing#2716
Conversation
|
@pullfrog review |
|
pkg.pr.new packages benchmark commit |
Bundle size comparison (
|
| 🟢 Decreased | ➖ Unchanged | 🔴 Increased | ❔ Unknown |
|---|---|---|---|
| 0 | 321 | 0 | 0 |
import { ... } in PR vs import * as ... in PR (is the library tree-Shakeable?):
| Test | tsdown |
|---|---|
| tgpu_init.ts | 256.97 kB ( |
| tgpu_initFromDevice.ts | 256.46 kB ( |
| tgpu_resolve.ts | 164.34 kB ( |
| tgpu_resolveWithContext.ts | 164.27 kB ( |
| tgpu_bindGroupLayout.ts | 68.96 kB ( |
| tgpu_mutableAccessor.ts | 65.97 kB ( |
| tgpu_accessor.ts | 65.96 kB ( |
| tgpu_privateVar.ts | 65.31 kB ( |
| tgpu_workgroupVar.ts | 65.30 kB ( |
| tgpu_const.ts | 64.55 kB ( |
| tgpu_fn.ts | 38.14 kB ( |
| tgpu_fragmentFn.ts | 38.13 kB ( |
| tgpu_vertexFn.ts | 37.95 kB ( |
| tgpu_computeFn.ts | 37.66 kB ( |
| tgpu_vertexLayout.ts | 26.79 kB ( |
| tgpu_comptime.ts | 14.48 kB ( |
| tgpu_unroll.ts | 1.66 kB ( |
| tgpu_slot.ts | 1.54 kB ( |
| tgpu_lazy.ts | 1.19 kB ( |
If you wish to run a comparison for other, slower bundlers, run the 'Tree-shake test' from the GitHub Actions menu.
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 [0.80, 1.52, 3.21, 5.71, 6.95, 9.52, 19.96, 20.89]
line [0.76, 1.67, 3.62, 5.54, 6.44, 9.17, 18.35, 21.66]
line [0.85, 1.71, 3.65, 5.86, 6.62, 10.93, 20.03, 22.30]
---
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.28, 0.54, 0.66, 0.79, 1.06, 1.02, 1.28, 1.36]
line [0.30, 0.50, 0.67, 0.81, 1.02, 1.14, 1.29, 1.48]
line [0.31, 0.54, 0.70, 0.80, 1.09, 1.11, 1.37, 1.50]
---
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 [0.68, 1.78, 3.20, 5.51, 10.94, 23.43, 48.67, 96.94]
line [0.65, 1.88, 3.76, 5.82, 10.69, 22.45, 48.25, 98.86]
line [0.79, 1.80, 3.81, 5.65, 10.80, 22.90, 51.62, 104.55]
|
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
The new capture.ts file imports tinyest at runtime, but packages/typegpu-testing-utility/package.json does not declare it as a dependency. Running pnpm vitest run --project=!browser packages/typegpu/tests/internal/capturedSnippets.test.ts fails with ERR_MODULE_NOT_FOUND in a clean workspace install (verified locally). Adding tinyest to the testing utility's dependencies resolves the failure.
Reviewed changes — adds a CAPTURE-based snippet extraction utility for more robust TGSL tests, plus the internal exports it needs.
- Add
CapturingGenerator,CAPTURE,captureSnippets, andsimplifyTypeintypegpu-testing-utilityto intercept and collect WGSL snippets during resolution. - Export
dualImplfromtypegpu/~internalso the utility can build a dual-implementation marker function. - Add unit tests in
packages/typegpu/tests/internal/capturedSnippets.test.tscovering regular-resolve no-op behavior, snippet extraction, repeated captures, and inner-to-outer capture ordering.
Kimi K2 (free via Pullfrog for OSS) ｜ 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes since last review — the follow-up commit adds the missing tinyest dependency to packages/typegpu-testing-utility/package.json (and the lockfile) so the new capture.ts module can resolve its runtime import in a clean pnpm workspace.
- Add
tinyestas a runtime dependency oftypegpu-testing-utilityusing theworkspace:*spec.
I verified pnpm vitest run --project=!browser packages/typegpu/tests/internal/capturedSnippets.test.ts now passes and both typegpu-testing-utility and typegpu typecheck cleanly.
Kimi K2 (free via Pullfrog for OSS) ｜ 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes — added a CAPTURE dual-impl helper and captureSnippets utility to the testing package, plus internal exports needed to support them.
- Add
CAPTURE/captureSnippets/simplifyTypetotypegpu-testing-utility—CAPTUREis transparent in normal resolves and records its argument's snippet when resolved throughCapturingGenerator. - Expose
dualImplfromtypegpu/~internal— required to declareCAPTUREas aDualFn. - Add
capturedSnippets.test.ts— covers normal resolution transparency, snippet extraction, re-capture, and nested capture ordering.
Kimi K2 (free via Pullfrog for OSS) ｜ 𝕏
There was a problem hiding this comment.
it('works with typedExpression - struct', () => {
const Boid = d.struct({
pos: d.vec3f,
});
const fn = tgpu.fn(
[],
Boid,
)(() => {
'use gpu';
return CAPTURE({ pos: d.vec3f() });
});
const captured = captureSnippets(fn);
expect(captured[0]?.value).toMatchInlineSnapshot(`"Boid(vec3f())"`);
});
it('works with typedExpression - numeric', () => {
const fn = tgpu.fn(
[],
d.u32,
)(() => {
'use gpu';
return CAPTURE(d.f32(1.67));
});
const captured = captureSnippets(fn);
expect(captured[0]?.value).toMatchInlineSnapshot(`1.6699999570846558`);
});Could you add these tests?
The second test needs to be fixed.
There was a problem hiding this comment.
So I think we decided that this is intended behavior, and that we want to remove expected type stack, right? I created an issue for that: #2726
There was a problem hiding this comment.
✅ No new issues found.
Kimi K2 (free via Pullfrog for OSS) ｜ 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes since last review — the new commit cleans up the capture.ts imports and adds a regression test verifying struct-literal capture before return-type casting.
- Consolidate
typegpu/~internalimports incapture.ts— pullsdualImplinto the existingtypegpu/~internalimport rather than importing it separately. - Add
captures structs after castingtest — verifies thatCAPTURErecords the originalBoidstruct data type of{ pos: d.vec3f() }before the function return type casts it.
Kimi K2 (free via Pullfrog for OSS) ｜ 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes since last review — merged latest main into the feature branch and updated the possibleSideEffects snapshot that became stale due to main's new side-effect tracking.
- Synced
maininto the branch — incorporates recent changes including the more comprehensive side-effect tracking that now marks captured constant abstract expressions aspossibleSideEffects: false. - Updated
allows snippet extractionsnapshot inpackages/typegpu/tests/internal/capturedSnippets.test.tsso the test matches the new behavior frommain.
Kimi K2 (free via Pullfrog for OSS) ｜ 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Snapshot update aligns the allows snippet extraction test with main's new side-effect tracking for constant abstract expressions.
Kimi K2 (free via Pullfrog for OSS) ｜ 𝕏
No description provided.