Repository navigation
Conversation
There was a problem hiding this comment.
Review summary
This is a small, well-scoped change to the dev/wasmstdlib test-orchestration harness: it raises the WebAssembly goroutine stack to 1MB for three stack-heavy js packages, excludes test/cgo/c11 from wasm source selection, and classifies test/std/simd as a separately-executed experimental SIMD suite. Tests are updated accordingly, and TestFullLargeStackCommandsAreTargeted is a well-designed addition that exercises the full six-profile matrix including a negative control (test/std/fmt).
I verified:
- The
-goroutine-stack-sizeflag is a real build flag accepting values like1MB(cmd/internal/flags/flags.go:231). - The guard
!p.Reference && p.GOOS == "js"correctly scopes the flag toJ32-GoJS,J32-Emscripten, andJ64-Emscriptenonly (notW32-WASIor reference profiles). - The
test/cgo/c11andtest/std/simdreason strings accurately match the packages' build constraints. - Package tests pass (
go test ./dev/wasmstdlib/).
No security, performance, or correctness issues were found. One minor comment-wording nit is noted inline.
Optional (non-blocking): the two adjacent blocks at full.go:93 and full.go:103 share the identical !p.Reference && p.GOOS == "js" guard and could be merged, though the current grouping-by-concern is defensible.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Fix the Wasm failures reported in llgo-compat/ci#120:
Apply that budget only to these three packages on JS profiles. Recognize native-only
test/cgo/c11as not applicable, and includetest/std/simdin the experimental suites already tested by native/W32-WASI jobs. Neither classification reports a pass.Validation: the original ML-DSA stack overflow reproduces locally; all three affected packages pass on GoJS, Emscripten32 and Emscripten64 with the budget. Full acceptance-runner unit tests pass, including profile isolation and source accounting.