perf(core): lazy load runtime diff formatting and fake timers#1079
perf(core): lazy load runtime diff formatting and fake timers#1079
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 88cb488e7c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Capture error.expected and error.actual into local variables before the await point in formatTestError. This prevents microtask callbacks from mutating the values between the yield and the diff/format calls.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 37fa4f9971
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| parentNames: test.parentNames, | ||
| name: test.name, | ||
| errors: formatTestError(error, test), | ||
| errors: await formatTestError(error, test), |
There was a problem hiding this comment.
Keep failure formatting out of the test-hook critical path
Even though formatTestError now snapshots expected/actual, the new await here still yields before runTestsCase reaches afterEachFns, test.context.task.result, and test.onFailed. If a failing test queued a microtask/promise callback before throwing, that callback can now run before cleanup and failure hooks, so afterEach/onTestFailed handlers may observe side effects or extra rejections that happened after the assertion failed. This changes hook ordering for ordinary test failures in packages/core/src/runtime/runner/runner.ts and can make cleanup logic flaky.
Useful? React with 👍 / 👎.
| err: any, | ||
| test?: Test, | ||
| ): Promise<FormattedError[]> => { | ||
| const errors = Array.isArray(err) ? err : [err]; |
There was a problem hiding this comment.
Snapshot mutable error arrays before awaiting diff imports
formatTestError now keeps a live reference to any array passed in, but callers like packages/core/src/runtime/worker/index.ts pass the shared unhandledErrors array while the process-level error listeners stay installed until teardown(). Once loadDiffModules() awaits, any late unhandledRejection/uncaughtException pushed into that same array during the import window is omitted from the formatted result, so a test file can log multiple late errors but only report the ones present before formatting started.
Useful? React with 👍 / 👎.
bench diff
~19% improvement for part of E2E testing in Rstest codebase
Summary
Background
Worker startup eagerly loads diff-formatting and fake-timer internals even when a test run never uses them, which adds avoidable runtime setup work.
Implementation
@sinonjs/fake-timerswhenFakeTimersis constructed so unused workers skip that module initializationUser Impact
Test runs that do not exercise diff output or fake timers pay less worker startup overhead.
Checklist