feat(vitest): route workflow trigger through globalThis mock#1188
Closed
toiroakr wants to merge 21 commits into
Closed
feat(vitest): route workflow trigger through globalThis mock#1188toiroakr wants to merge 21 commits into
toiroakr wants to merge 21 commits into
Conversation
Mirror the Platform JSON serialization boundary in workflow mocks so tests catch invalid payloads (NaN/Infinity/BigInt/class instances) the same way the real Platform does. Both wjob.trigger() and wf.trigger() now delegate to globalThis.tailor.workflow.triggerJobFunction / triggerWorkflow, matching the wait/resolve pattern. - Add platformSerialize utility validating JSON-boundary safety - Introduce a globalThis-keyed registry so the mock can look up job and workflow bodies by name across module instances - Route wf.trigger() through mockTriggerJobFunction so main-job invocations are recorded in triggeredJobs and honor setJobHandler / enqueueResult uniformly
🦋 Changeset detectedLatest commit: 0b76ea0 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
⚡ pkg.pr.new@tailor-platform/sdk@tailor-platform/create-sdk
|
This comment has been minimized.
This comment has been minimized.
…ialize errors - Move getPlatformWorkflow() into registry.ts so job.ts / workflow.ts share one shim - Note body fallback behaviour in the Workflow Mock docs section - Throw specific TypeError messages for root-level function / symbol in platformSerialize
This comment has been minimized.
This comment has been minimized.
Resolve workflow-mock conflicts: - Keep HEAD's globalThis-mock trigger routing; integrate main's workflowMock.setEnv() into the mock's buildJobContext (readWorkflowTestEnv with deprecated env-var fallback). - Fix the nested integration Vitest config's @/ alias path math (broke when the dir moved from __tests__/integration to integration). - Make mock triggerJobFunction synchronous (Promise only for async bodies) to match both the platform contract and runtime/workflow's typed wrapper. - Drop the already-released wrap-trigger-job-function-in-promise changeset.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
…c callbacks The workflow mock wrapped the user resolve callback in an async function, so test handlers that call `callback(...)` synchronously (the workflow template's resolveApproval test and the docs/testing.md example) received a Promise instead of the serialized value. Return the serialized result directly for sync callbacks and only as a Promise for async ones, mirroring triggerJobFunction and the platform's synchronous contract.
This comment has been minimized.
This comment has been minimized.
createWorkflowJob now bundles a small globalThis-registry shim (~230-320B per job) so the vitest mock can look up job bodies by name. The recorded baselines had already drifted ~10KB below current sizes from accumulated growth on main, exhausting the 10KB buffer, so fetch-customer/process-payment tipped over. Update the workflow-job baselines to current sizes; the 10KB regression buffer is preserved.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
…RM_BUNDLE Replace the globalThis.__TAILOR_PLATFORM_BUNDLE__ sentinel with process.env.TAILOR_PLATFORM_BUNDLE, which the workflow bundler substitutes to true before minify/DCE. Drops the bespoke global declaration; in tests the env var is simply unset so the registry/trigger shim runs as before.
Option B shrank the workflow-job bundles back under the original baselines, so the bumped values are no longer needed; revert the file to main to keep the PR diff minimal.
…ndler The function test-run bundler did not fold `process.env.TAILOR_PLATFORM_BUNDLE`, so createWorkflowJob's guard reached the Platform Web runtime (no `process`) and crashed — failing the SDK E2E 'runs workflow job by name' test. Extract a shared platformBundleDefinePlugin and apply it in both the workflow deploy bundler and the test-run bundler, and add a unit guard asserting the flag is folded away.
This comment has been minimized.
This comment has been minimized.
Comment on lines
+19
to
+21
| export function platformSerialize<T>(value: T): T { | ||
| // Top-level undefined is allowed (jobs may take no input). | ||
| if (value === undefined) return undefined as T; |
Comment on lines
+3
to
+6
| // Fold `process.env.TAILOR_PLATFORM_BUNDLE` to `true` so the minifier DCEs the | ||
| // test-only workflow registry/trigger shim. Apply in every bundler that builds | ||
| // runnable functions; a missed path leaves the env read in place and fails | ||
| // loudly on the Platform's Web runtime (no `process`), which an e2e catches. |
…dler Move the shared plugin to cli/shared and wire it into the resolver, executor, auth, seed, migrate, query, and tailordb-hooks bundlers (in addition to workflow deploy and function test-run). Any bundler that could pull in createWorkflow(Job) now folds the platform-bundle flag, so the unsubstituted process.env read can never reach the Platform Web runtime. A surviving read fails loudly in e2e.
…mise type In a platform bundle .trigger() is rewritten away, but the dead stub was a synchronous throw while the type is Promise-returning. Make it async so any stray call rejects rather than throws synchronously.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
remiposo
reviewed
Jun 3, 2026
Contributor
remiposo
left a comment
remiposo
left a comment
There was a problem hiding this comment.
[imo]
Is it acceptable that workflow testing becomes impossible without using tailor-runtime?
Personally, I think it would be simpler and more desirable to have jobs called as plain function calls locally. The JSON serialization issue could be addressed simply by wrapping the function call, and that alone should not be a reason to require globalThis mocking.
If using globalThis eliminates the need for AST manipulation during bundling, that would be a compelling reason to do it — but otherwise I'm not convinced.
…njectMocks) Correct the overly strong 'requires the tailor-runtime environment' note: workflow .trigger() mocking also works in a plain test via injectMocks(globalThis), not only under the tailor-runtime environment.
Code Metrics Report (packages/sdk)
Details | | main (32a99be) | #1188 (2a3c8db) | +/- |
|--------------------|----------------|-----------------|-------|
+ | Coverage | 64.6% | 64.8% | +0.1% |
| Files | 381 | 384 | +3 |
| Lines | 13180 | 13257 | +77 |
+ | Covered | 8517 | 8591 | +74 |
+ | Code to Test Ratio | 1:0.4 | 1:0.4 | +0.0 |
| Code | 87952 | 88414 | +462 |
+ | Test | 37763 | 38037 | +274 |Code coverage of files in pull request scope (89.3% → 89.9%)SDK Configure Bundle Size
Runtime Performance
Type Performance (instantiations)
Reported by octocov |
Contributor
Author
|
The statement that tailor-runtime is required was an error in the docs. Based on the assumption of #1282, it seems possible to organize this more cleanly, so I will recreate this as a PR directed toward that. |
Contributor
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
wjob.trigger()/wf.trigger()so vitest catches NaN/Infinity/BigInt/class-instance payloads the same way the real Platform does.wjob.trigger()andwf.trigger()throughglobalThis.tailor.workflow.triggerJobFunction/triggerWorkflow, matching the existing wait/resolve mock pattern.registerJob/registerWorkflow) so the mock can look up bodies by name across module instances; bodies are registered as a side effect ofcreateWorkflowJob/createWorkflow. The registry module also exports a sharedgetPlatformWorkflow()shim used by bothjob.tsandworkflow.ts.platformSerializeutility (with tests) used at the mock boundary to validate payloads and strip top-levelundefined; root-levelfunction/symbolvalues throw a specific TypeError instead of a generic message.mockTriggerWorkflow: the main job is now invoked viamockTriggerJobFunction, so it appears intriggeredJobsand honorssetJobHandler/enqueueResultuniformly with other jobs.docs/testing.mdto describe the new uniform behavior, the body-fallback for the Workflow Mock when no handler/result is configured, and thetailor-runtimeenvironment requirement.