chore: virtual clock for app hang tests - #1931
Conversation
| } | ||
|
|
||
| void | ||
| sentry__app_hang_monitor_check(uint64_t *last_fired_heartbeat_ms) |
There was a problem hiding this comment.
Extracted for unit test.
| if (!sentry__app_hang_is_active()) { | ||
| sentry__mutex_unlock(&g_wait_mutex); | ||
| break; |
There was a problem hiding this comment.
Covers race between outer while condition and acquiring g_wait_mutex.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 95affbb. Configure here.
|
|
||
| sentry_close(); | ||
| sentry__app_hang_monitor_set_stackwalk_fn(NULL); | ||
| sentry__test_clock_reset(); |
There was a problem hiding this comment.
Tests race with live worker
Medium Severity
App-hang tests still call sentry_init, which starts the watchdog worker, and then also invoke sentry__app_hang_monitor_check on the test thread after advancing the virtual clock. Each caller keeps its own last_fired_heartbeat_ms, so after SENTRY_APP_HANG_POLL_MS the worker can capture the same hang again, including a concurrent real stackwalk in app_hang_end_to_end.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 95affbb. Configure here.
| sentry__mutex_unlock(&spinner_lock); | ||
| TEST_ASSERT(spinner_started); | ||
|
|
||
| sentry__test_clock_advance(1000); |
There was a problem hiding this comment.
The changes revolve around this. No more sleeping and waiting something is finished spinning up, or waking up. Allows for deterministic testing.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #1931 +/- ##
==========================================
- Coverage 75.79% 75.77% -0.02%
==========================================
Files 93 93
Lines 22125 22170 +45
Branches 3939 3945 +6
==========================================
+ Hits 16770 16800 +30
- Misses 4472 4486 +14
- Partials 883 884 +1 🚀 New features to boost your workflow:
|
1 participant
Instead of having to call
sleep_msand adding timeouts to wait for app hangs to be detected this introduces an abstraction for the clock for unit tests. This can then be expanded to be used in other tests as well.This also extracts the
checkingpart out of the worker for unit tests. Checking the full worker functionality i.e. scheduling and wake up are a follow up.#skip-changelog