Skip to content

test: capture resolved statements - #2947

Open
huytdps13400 wants to merge 2 commits into
software-mansion:mainfrom
huytdps13400:fix/capture-following-statement
Open

test: capture resolved statements#2947
huytdps13400 wants to merge 2 commits into
software-mansion:mainfrom
huytdps13400:fix/capture-following-statement

Conversation

@huytdps13400

Copy link
Copy Markdown

Summary

  • add a CAPTURE_FOLLOWING() no-op marker and captureStatements() helper for inspecting ResolvedStatement output
  • keep pending captures scoped to the current block so nested statements and separately resolved helper functions cannot consume them
  • preserve the existing captureSnippets(): Snippet[] contract instead of widening it to a mixed union
  • reject markers that are not followed by a statement

Closes #2715.

Verification

  • focused capture suite: 11/11 passed
  • full source suite with bounded workers: 275 files, 2,872 passed, 2 skipped
  • built package suite with attest enabled: 243 files, 2,443 passed, 2 skipped
  • full monorepo typecheck: 0 errors
  • full Oxlint/Prettier/Oxfmt style gate: passed
  • circular dependency check: passed
  • git diff --check: passed

The default highly parallel source run twice hit the existing 5-second timeout in typegpu-docs probability example; that test passed alone in 662 ms, and the complete suite passed with --maxWorkers=4.

Copilot AI lite review requested due to automatic review settings August 30, 2026 07:53

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a new testing-only capture mechanism to inspect resolved statement output from the WGSL generator, complementing the existing snippet capture utilities.

Changes:

  • Introduces CAPTURE_FOLLOWING() marker and captureStatements() helper to collect ResolvedStatement results during resolution.
  • Scopes “pending capture” state to the current block to avoid nested blocks or other resolves consuming it.
  • Expands internal test coverage to validate marker no-op behavior, capturing semantics, and error cases.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
packages/typegpu/tests/internal/capturedSnippets.test.ts Adds a focused test suite for CAPTURE_FOLLOWING + captureStatements behavior and error handling.
packages/typegpu-testing-utility/src/index.ts Re-exports CAPTURE_FOLLOWING and captureStatements from the testing utility package entrypoint.
packages/typegpu-testing-utility/src/capture.ts Implements statement capture plumbing in a custom WgslGenerator and adds the new marker/helper API.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread packages/typegpu-testing-utility/src/capture.ts Outdated

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

captureStatements returns .code that can still contain unresolved #VAR_n# placeholder tokens, so the captured text diverges from what actually lands in the shader for non-scalar let/const declarations and loops. See the inline comment on capture.ts.

Reviewed changes — one commit (dc530ca, "test: capture resolved statements") that adds a statement-capture facility to typegpu-testing-utility for inspecting ResolvedStatement output:

  • CAPTURE_FOLLOWING(): a dualImpl no-op whose call CapturingGenerator._expression intercepts to set a per-block capture flag (packages/typegpu-testing-utility/src/capture.ts).
  • _statement/_block overrides: consume the flag on the next statement, scope capture state per block with a stack so nested statements and separately resolved helpers can't consume another block's flag, and throw when a marker isn't followed by a statement.
  • captureStatements() helper, index.ts re-exports, and five new tests in packages/typegpu/tests/internal/capturedSnippets.test.ts.

Verified the focused suite passes locally (11/11) and probed edge cases (loop bodies, comptime-pruned statements, inner-block markers) with scratch tests that were deleted afterward.

ℹ️ Nitpicks

  • A marker followed by a statement that comptime-folds to nothing (e.g. CAPTURE_FOLLOWING(); if (false) { ... }) silently captures an empty { code: '' } statement and consumes the marker. Harmless, but a test author may be surprised; a one-line doc note on captureStatements (or skipping empty-code captures) would cover it.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

Comment thread packages/typegpu-testing-utility/src/capture.ts

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes — one new commit (acd21101, "fix: normalize captured statements") that resolves the prior review's finding that captured statement .code leaked unresolved #VAR_n# placeholders for non-scalar declarations and loops:

  • Factored the deferred-var substitution out of WgslGenerator.functionDefinition into a shared protected _replaceVariablePlaceholders(code) (packages/typegpu/src/tgsl/wgslGenerator.ts), preserving exact behavior in the existing call site.
  • Added a CapturingGenerator.functionDefinition override that applies the finalized function-scope placeholderForVariable/modifiedVariables mapping to statements captured during that function's generation, so captured text matches the emitted shader.
  • CAPTURE_FOLLOWING() markers now resolve to an empty statement instead of falling through, and consecutive markers in the same block throw 'CAPTURE_FOLLOWING must be followed by a statement'.
  • Added tests for consecutive-marker rejection and for deferred-placeholder resolution across a non-scalar const plus a for loop. The latter is genuine regression coverage — it fails when the new override is removed (verified by mutation) — and cross-checks each captured statement against the emitted shader.
  • Documented on captureStatements that a comptime-pruned following statement captures an empty { code: '' } (covers the prior nitpick).

The prior review's placeholder-divergence thread was resolved. The wgslGenerator.ts change is behavior-preserving; the internal suite (431 tests) and typechecks for both packages pass, and the nested-helper capture path (marker inside a separately-resolved tgpuFn) was verified at runtime.

Pullfrog  | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

testing: Expect statement snippet

2 participants