✅ Introduce a new way to mock values in unit tests#4129
✅ Introduce a new way to mock values in unit tests#4129BenoitZugmeyer wants to merge 12 commits intomainfrom
Conversation
|
55a8c24 to
7ab0afe
Compare
Bundles Sizes Evolution
🚀 CPU PerformancePending... 🧠 Memory PerformancePending... |
2fbac02 to
9182076
Compare
This spec initialization is unnecessarily complex. This commit cleans that spec and moves one higher level assertion in `recorderApi.spec.ts`
A few test cases is logsPublicApi and preStartRum run test initialization multiple times (due to nested describe/beforeEach). This commit cleans this up
c9c4f10 to
a339f5d
Compare
1fe7847 to
518ac81
Compare
|
/to-staging |
|
View all feedbacks in Devflow UI.
Commit 518ac81fe5 will soon be integrated into staging-07.
Commit 518ac81fe5 has been merged into staging-07 in merge commit 378f13e872. Check out the triggered DDCI request. If you need to revert this integration, you can use the following command: |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 518ac81fe5
ℹ️ 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".
good bot 🤖
518ac81 to
0ed26a5
Compare
Integrated commit sha: 518ac81 Co-authored-by: BenoitZugmeyer <benoit.zugmeyer@datadoghq.com>
| strategy = createPostStartStrategy(initConfiguration, startLogsResult) | ||
| return startLogsResult | ||
| }, | ||
| startTelemetryImpl |
There was a problem hiding this comment.
👏 praise: Nice! It was just argument drilling!
| ### Unit Tests | ||
|
|
||
| - Spec files co-located with implementation: `feature.ts` → `feature.spec.ts` | ||
| - Test framework: Jasmine + Karma. Spec files co-located with implementation: `feature.ts` → `feature.spec.ts` | ||
| - Focus tests with `fit()` / `fdescribe()`, skip with `xit()` / `xdescribe()` | ||
| - Use `registerCleanupTask()` for cleanup, NOT `afterEach()` | ||
| - Test framework: Jasmine + Karma | ||
| - Mock values/functions: wrap with `mockable()` in source, use `replaceMockable()` or `replaceMockableWithSpy()` in tests (auto-cleanup) |
This change was generated by Claude to test coding agents capabilities to use the new mocking pattern. We used the following prompt: > Dear claude. In @packages/rum-core/src/domain/waitPageActivityEnd.ts and its spec, we have the `doWaitPageActivityEnd` function for testing purpose, because we don't want to run `createPageActivityObservable` in tests. Let's remove this extra function and use mocks instead instead.
|
/to-staging |
|
View all feedbacks in Devflow UI.
Commit b98ddf140b will soon be integrated into staging-07.
Commit b98ddf140b has been merged into staging-07 in merge commit 687815cd88. Check out the triggered DDCI request. If you need to revert this integration, you can use the following command: |
Integrated commit sha: b98ddf1 Co-authored-by: BenoitZugmeyer <benoit.zugmeyer@datadoghq.com>
Motivation
This is a proposal to allow mocking values (mostly functions) within tests.
The current way to mock values used within a function is to pass them as argument of that function. For example:
However, forces to have unecessary extra arguments in functions. Impls also need to be propagated within subfunctions:
This PR introduce a new way to mock values that is similar to a lightweight dependency injection system. Values that can be mocked should be wrapped in a
mocked()call when used, and tests can usemockValue()to replace the value ormockWithSpy()to replace it with a jasmine spy. The example above would be implemented as:Changes
The first 2 commits does a bit of spec cleaning.
Then, we introduce
mockablecapabilities and use it in tests, mostly to replace all*Implarguments and location mocking.Please review commit by commit.
Test instructions
If tests passes, it should be good.
Checklist