✅ fix flaky tests related to experimental features#4207
Conversation
Some tests cases did not use `mockExperimentalFeatures` to enable experimental features, and didn't properly clean up after themselves. This was not an issue before #4149, but now that the experimental feature set is shared between spec files, it becomes important to reset features after each test. This commit simplifies how we deal with experimental features in unit tests by: * always clearing experimental features in 'forEach.spec' * replace `mockExperimentalFeatures` with `addExperimentalFeatures`, since it's not useful anymore * remove now unnecessary calls to `resetExperimentalFeatures`
Bundles Sizes Evolution
🚀 CPU PerformancePending... 🧠 Memory PerformancePending... |
|
✅ Tests 🎉 All green!❄️ No new flaky tests detected 🎯 Code Coverage (details) 🔗 Commit SHA: f2e6789 | Docs | Datadog PR Page | Was this helpful? Give us feedback! |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f2e67890b7
ℹ️ 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".
| isExperimentalFeatureEnabled, | ||
| addExperimentalFeatures, | ||
| resetExperimentalFeatures, | ||
| getExperimentalFeatures, | ||
| initFeatureFlags, | ||
| ExperimentalFeature, |
There was a problem hiding this comment.
Re-export resetExperimentalFeatures from browser-core index
Removing resetExperimentalFeatures from the root @datadog/browser-core export list introduces an API break: any downstream package or test utility that imports { resetExperimentalFeatures } from the package entrypoint will now fail to compile (or error on missing export at runtime). Because this change set is aimed at test flakiness rather than a declared breaking-release change, keeping this export in packages/core/src/index.ts avoids unexpectedly breaking consumers.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
core is an internal package.
Motivation
Some tests cases did not use
mockExperimentalFeaturesto enable experimental features, and didn't properly clean up after themselves.This was not an issue before #4149, but now that the experimental feature set is shared between spec files, it becomes important to reset features after each test.
Changes
This commit simplifies how we deal with experimental features in unit tests by:
mockExperimentalFeatureswithaddExperimentalFeatures, since it's not useful anymoreresetExperimentalFeaturesTest instructions
This impacts only unit test. If CI is green, it should be good.
Checklist