Fix SnapshotSlicerTests verify prefix - #9009
Conversation
BenchmarksBenchmark execution time: 2026-08-07 10:00:03 Comparing candidate commit e58816a in PR branch Found 0 performance improvements and 2 performance regressions! Performance is the same for 70 metrics, 0 unstable metrics, 67 known flaky benchmarks, 59 flaky benchmarks without significant changes.
|
Execution-Time Benchmarks Report ⏱️Execution-time results for samples comparing This PR (9009) and master. ✅ No regressions detected |
|
@codex review |
|
Codex Review: Didn't find any major issues. What shall we delve into next? Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
andrewlock
left a comment
There was a problem hiding this comment.
Oh, wow, I didn't know that was a thing 😬 That's something we'll have to remember any time we have [Flaky] on a test then 😬
Summary of changes
Allow the three flaky
SnapshotSlicerTestssnapshot tests to reuse their Verify prefix during retries.Reason for change
When one of these tests hits its known JSON flake, the retry runs in the same process. Verify then rejects the second attempt because the snapshot prefix was already registered, causing the retry to fail for an unrelated reason.
Error:
Implementation details
Pass
VerifySettingswithDisableRequireUniquePrefix()to the tests already marked[Flaky].Test coverage
Built
Datadog.Trace.Testsand ranSnapshotBiggerThanMaxSize_TwoLevel_OneSlicedlocally.Other details
None.