Commit 1c17de0
test: fix flaky debugModeExpiresBasedOnServerTime test (#165)
**Requirements**
- [x] I have added test coverage for new or changed functionality
- [x] I have followed the repository's [pull request submission
guidelines](../blob/main/CONTRIBUTING.md#submitting-pull-requests)
- [x] I have validated my changes against all supported platform
versions
**Related issues**
Fixes a flaky test in `DefaultEventProcessorOutputTest`.
**Describe the solution you've provided**
Added `ep.waitUntilInactive()` calls after `es.awaitRequest()` in both
`debugModeExpires*` tests. This ensures the flush worker has fully
completed processing the HTTP response — including calling
`handleResponse()` which sets `lastKnownPastTime` — before the test
proceeds to send the next event.
**Root cause**
`flushBlocking()` only waits for the FLUSH message to be processed by
the dispatcher thread, which puts the payload on the worker queue and
returns. `awaitRequest()` returns as soon as `MockEventSender.receive()`
adds to `receivedParams`, which happens before `SendEventsTask.run()`
calls `responseListener.handleResponse(result)`. This means
`lastKnownPastTime` may not be set when the test sends the second event,
causing the debug mode expiration check to use only client time (which
is 19s behind `debugUntil`), resulting in an unexpected debug event.
**Describe alternatives you've considered**
- Adding a `Thread.sleep()` after `awaitRequest()` — fragile and
non-deterministic.
- Modifying the application code — not appropriate since this is a test
synchronization issue.
**Additional context**
This test has been flaking at ~6% rate across the last 100 CI runs. All
observed failures show the same pattern: a `debug` event appears where
only a `summary` event was expected.
Link to Devin session:
https://app.devin.ai/sessions/4b0d94a56a834efb9c0a758755e6653e
Requested by: @kinyoklion
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> **Low Risk**
> Test-only synchronization changes with no production or API impact.
>
> **Overview**
> Adds **`ep.waitUntilInactive()`** after the initial flush and
**`es.awaitRequest()`** in
**`debugModeExpiresBasedOnClientTimeIfClientTimeIsLaterThanServerTime`**
and
**`debugModeExpiresBasedOnServerTimeIfServerTimeIsLaterThanClientTime`**.
>
> The tests were racing: **`awaitRequest()`** could return before the
flush worker ran **`handleResponse()`** and set **`lastKnownPastTime`**,
so the second feature event sometimes still emitted a **debug** event
instead of only a **summary**. Waiting until the processor is inactive
ties the setup step to that response handling.
>
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
3e2086b. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
---------
Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>1 parent 3cba5ef commit 1c17de0
1 file changed
Lines changed: 2 additions & 0 deletions
File tree
Lines changed: 2 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
258 | 258 | | |
259 | 259 | | |
260 | 260 | | |
| 261 | + | |
261 | 262 | | |
262 | 263 | | |
263 | 264 | | |
| |||
289 | 290 | | |
290 | 291 | | |
291 | 292 | | |
| 293 | + | |
292 | 294 | | |
293 | 295 | | |
294 | 296 | | |
| |||
0 commit comments