[DX-4492] Faster WASM#22992
Conversation
|
✅ No conflicts with other open PRs targeting |
|
I see you updated files related to
|
There was a problem hiding this comment.
Pull request overview
Risk Rating: MEDIUM
This PR aims to speed up the workflow/compute WASM test suite by switching from on-the-fly go build compilation during tests to pre-generated, committed WASM fixtures (via go generate), and by parallelizing a large portion of the tests. It also includes a handful of small refactors/cleanups across workflow engine, monitoring, and artifacts persistence.
Changes:
- Replace test-time WASM compilation (
CreateTestBinary) with pre-generated fixture loading (GetTestBinary) and add ago generategenerator to build fixtures intotestdata/output.wasm.br. - Add
t.Parallel()broadly across workflow/compute/artifacts tests and annotate non-parallel-safe subtests with//nolint:paralleltest. - Minor refactors: monitoring label conversion, workflow artifacts ORM cleanup queries, and small engine/concurrency tweaks.
Areas requiring scrupulous human review:
- Newly-parallelized tests: ensure no shared mutable state (configs, maps, mocks, DB handles) is used across parallel subtests.
- WASM fixture generator + fixture loading semantics: ensure
go generate ./...is part of the expected developer/CI workflow and that fixtures remain reproducible and up to date.
Reviewed changes
Copilot reviewed 52 out of 78 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| go.md | Update dependency flowchart links/edges. |
| core/services/workflows/v2/secrets_test.go | Parallelize secrets fetcher tests. |
| core/services/workflows/v2/local_secrets_fetcher_test.go | Parallelize local secrets fetcher tests/subtests. |
| core/services/workflows/v2/engine.go | Add nolint:staticcheck on legacy execution ID generation path. |
| core/services/workflows/v2/engine_test.go | Add WASM fixture go:generate, add nolint:paralleltest notes, use wasmtest.GetTestBinary, parallelize some subtests. |
| core/services/workflows/v2/engine_execution_concurrency_test.go | Add nolint:staticcheck on legacy execution ID generation in tests. |
| core/services/workflows/v2/config_test.go | Add nolint:paralleltest where subtests mutate shared config. |
| core/services/workflows/v2/confidential_module_test.go | Parallelize multiple tests/subtests. |
| core/services/workflows/v2/capability_executor.go | Import formatting cleanup. |
| core/services/workflows/types/errors_test.go | Parallelize error tests/subtests. |
| core/services/workflows/syncer/versioning/contracts_test.go | Reduce duplication; share backend harness; mark subtests non-parallel due to shared backend. |
| core/services/workflows/syncer/v2/workflow_syncer_v2_test.go | Speed up syncer tests by reducing sleeps/intervals and setting tick interval options. |
| core/services/workflows/syncer/v2/handler_test.go | Add WASM fixture go:generate; switch to wasmtest.GetTestBinary. |
| core/services/workflows/syncer/v2/fetcher_test.go | Update gateway response signing helper usage and tighten error assertions. |
| core/services/workflows/syncer/handler_test.go | Switch to wasmtest.GetTestBinary. |
| core/services/workflows/syncer/fetcher_test.go | Use signed gateway responses and ErrorContains. |
| core/services/workflows/store/store_memory_test.go | Parallelize in-memory store tests. |
| core/services/workflows/store/models_test.go | Parallelize model deep copy test. |
| core/services/workflows/shardownership/steady_signal_test.go | Parallelize steady signal test. |
| core/services/workflows/shardownership/check_test.go | Parallelize committed owner check tests/subtests. |
| core/services/workflows/monitoring/steady_signal_metrics_test.go | Parallelize constant test. |
| core/services/workflows/monitoring/monitoring.go | Replace local kv-map-to-otel conversion with beholder.OtelAttributes(...).AsStringAttributes(). |
| core/services/workflows/monitoring/monitoring_test.go | Parallelize monitoring tests. |
| core/services/workflows/models_test.go | Parallelize parsing tests and switch to t.Context() usage. |
| core/services/workflows/metering/metering.go | Minor refactor to avoid shadowing err in conversion. |
| core/services/workflows/metering/metering_test.go | Add nolint:paralleltest with explicit reason. |
| core/services/workflows/internal/retry_test.go | Use require.NoError for stronger failure behavior. |
| core/services/workflows/events/emit.go | Doc comment formatting for deprecated execution ID function. |
| core/services/workflows/events/emit_test.go | Add nolint:paralleltest annotations for shared beholder observer. |
| core/services/workflows/engine.go | Use wg.Go, add legacy ID nolint:staticcheck, and refactor timeout override branching. |
| core/services/workflows/engine_test.go | Add WASM fixture go:generate; replace CreateTestBinary with GetTestBinary; parallelize many tests/subtests; replace beholder helper with beholdertest.NewObserver. |
| core/services/workflows/delegate_test.go | Parallelize spec validation subtests and switch to t.Context(). |
| core/services/workflows/cmd/cre/utils/standalone_engine.go | Minor error variable naming cleanup. |
| core/services/workflows/cmd/cre/utils/runner_test.go | Switch to wasmtest.GetTestBinary. |
| core/services/workflows/cmd/cre/utils/billing.go | Use net.ListenConfig.Listen(ctx, ...) instead of net.Listen. |
| core/services/workflows/cmd/cre/examples/compile_test.go | Add WASM fixture go:generate, switch to GetTestBinary, parallelize. |
| core/services/workflows/artifacts/v2/store.go | Switch limiter builder to limits.MakeUpperBoundLimiter. |
| core/services/workflows/artifacts/v2/store_test.go | Parallelize tests; avoid DB collisions via randomized IDs/URLs. |
| core/services/workflows/artifacts/v2/file_module_store_test.go | Parallelize tests; use for i := range 10 loops. |
| core/services/workflows/artifacts/store_test.go | Parallelize tests; switch to t.Context(). |
| core/services/workflows/artifacts/orm.go | Fix cleanup query execution by using ExecContext instead of QueryRowxContext().Scan(nil). |
| core/services/workflows/artifacts/orm_test.go | Parallelize tests; avoid collisions with randomized IDs/URLs; mark shared-DB subtests non-parallel. |
| core/internal/testutils/wasmtest/wasm.go | Replace test-time WASM compilation with fixture loading + caching; add repo-root discovery. |
| core/internal/testutils/wasmtest/wasm_test.go | New unit tests for fixture reading/decompression behavior. |
| core/internal/testutils/wasmtest/generator/main.go | New go generate tool to build + brotli-compress WASM fixtures into testdata/output.wasm.br. |
| core/capabilities/compute/transformer_test.go | Parallelize many tests; mark shared-map subtests non-parallel. |
| core/capabilities/compute/monitoring.go | Replace local kv-map-to-otel conversion with beholder.OtelAttributes(...).AsStringAttributes(). |
| core/capabilities/compute/compute.go | Update interface assertion to capabilities.ExecutableCapability. |
| core/capabilities/compute/compute_test.go | Add WASM fixture go:generate, switch to GetTestBinary, adjust timing cases, parallelize some tests. |
| core/capabilities/compute/cache.go | Use wg.Go helper for reap loop goroutine. |
| core/capabilities/compute/cache_test.go | Switch to GetTestBinary; make cache add assertions strict (require.NoError). |
| .gitignore | Ensure WASM fixture output can be committed (exception rule for testdata/output.wasm.br). |
| .dockerignore | Exclude testdata/ from docker build context. |
|




We were recompiling WASM binaries at runtime, often compiling/waiting on the same binary to finish before running tests. This PR optimizes our WASM binary flow to reduce test execution time and CPU usage for packages
core/capabilities/computeandcore/services/workflows.Results
Major Changes
CreateTestBinary->GetTestBinary: No longer compile WASM binaries during test runtime. Instead, compile as part ofgo generateand put intestdata/folders. We check in the compressed WASM binaries, and uncompress them at runtime if necessary (prevent bloating repo size too much). Saves 30+ seconds of runtime per test, plus tons of CPU usage savings.ExecContext(...)forDELETEquery: More proper SQL usage for DELETEmetering.go. Recreated with regression test.Also includes lots of linting and flake fixes for affected packages.