Skip to content

test: isolate shake detector cooldown - #8626

Merged
supervacuus merged 2 commits into
mainfrom
test/isolate_shake_detector_cooldown
Jul 30, 2026
Merged

test: isolate shake detector cooldown#8626
supervacuus merged 2 commits into
mainfrom
test/isolate_shake_detector_cooldown

Conversation

@supervacuus

Copy link
Copy Markdown
Collaborator

📜 Description

  • add a test-only reset for SentryShakeDetector's process-wide cooldown state
  • reset the cooldown before and after each SentryShakeDetectorTests test
  • prevent shake events from one test suppressing events in a subsequent test

💡 Motivation and Context

SentryShakeDetectorTests.testEnable_whenShakeOccurs_shouldPostNotification can fail depending on test order and timing. A shake from the preceding test updates the process-wide lastShakeTimestamp, and disable() does not reset it. If the next shake occurs within the one-second cooldown, its notification is suppressed.

This was observed in a CI run for #8589.

Resetting the cooldown in disable() itself may be reasonable because re-enabling could be considered a new detection session. However, doing so would change production behavior by allowing a shake immediately after a disable/re-enable cycle, which could make sense, but I am not sure whether separating the life-cycle from the cooldown wasn't intentional. If it wasn't, I am happy to integrate it into disable() instead.

💚 How did you test it?

I only ran the affected tests to check that nothing broke there. No production code was modified (yet, see above).

📝 Checklist

You have to check all boxes before merging:

  • I added tests to verify the changes.
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled.
  • I updated the docs if needed.
  • I updated the wizard if needed.
  • Review from the native team if needed.
  • No breaking change or entry added to the changelog.
  • No breaking change for hybrid SDKs or communicated to hybrid SDKs.
  • Public API changes reviewed by another Mobile SDK team member or implemented according to the develop docs spec.
  • If I added a new public API, I also added it to the SentryObjC wrapper.

@supervacuus supervacuus added the run-full-ci Allows gated GitHub Action workflows to run for a labelled pull request label Jul 30, 2026
Comment thread Sources/Swift/Integrations/UserFeedback/SentryShakeDetector.swift
@sentry

sentry Bot commented Jul 30, 2026

Copy link
Copy Markdown

📲 Install Builds

iOS

🔗 App Name App ID Version Configuration
SDK-Size io.sentry.sample.SDK-Size 9.24.0 (1) Release

⚙️ sentry-cocoa Build Distribution Settings

@github-actions

github-actions Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1203.07 ms 1236.90 ms 33.83 ms
Size 24.14 KiB 1.25 MiB 1.23 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
2fe9650 1228.33 ms 1260.78 ms 32.46 ms
a8fc1c8 1226.83 ms 1248.19 ms 21.36 ms
27850be 1222.65 ms 1254.23 ms 31.58 ms
241d6ee 1228.92 ms 1258.47 ms 29.55 ms
6307f8b 1216.23 ms 1247.74 ms 31.52 ms
66415de 1221.21 ms 1253.28 ms 32.07 ms
62cb5fd 1219.45 ms 1250.56 ms 31.11 ms
c4d02bd 1219.63 ms 1248.68 ms 29.05 ms
8bb6aeb 1232.15 ms 1260.78 ms 28.63 ms
dff20e3 1225.19 ms 1258.66 ms 33.47 ms

App size

Revision Plain With Sentry Diff
2fe9650 24.14 KiB 1.17 MiB 1.15 MiB
a8fc1c8 24.14 KiB 1.23 MiB 1.20 MiB
27850be 24.14 KiB 1.18 MiB 1.16 MiB
241d6ee 24.14 KiB 1.24 MiB 1.22 MiB
6307f8b 24.14 KiB 1.23 MiB 1.20 MiB
66415de 24.14 KiB 1.16 MiB 1.13 MiB
62cb5fd 24.14 KiB 1.22 MiB 1.20 MiB
c4d02bd 24.14 KiB 1.17 MiB 1.14 MiB
8bb6aeb 24.14 KiB 1.17 MiB 1.15 MiB
dff20e3 24.14 KiB 1.15 MiB 1.13 MiB

Previous results on branch: test/isolate_shake_detector_cooldown

Startup times

Revision Plain With Sentry Diff
2a8c954 1221.54 ms 1266.57 ms 45.02 ms

App size

Revision Plain With Sentry Diff
2a8c954 24.14 KiB 1.26 MiB 1.23 MiB

@supervacuus supervacuus self-assigned this Jul 30, 2026

@itaybre itaybre left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@supervacuus
supervacuus merged commit 53b5a77 into main Jul 30, 2026
274 checks passed
@supervacuus
supervacuus deleted the test/isolate_shake_detector_cooldown branch July 30, 2026 14:39
philprime pushed a commit that referenced this pull request Jul 30, 2026
* test: isolate shake detector cooldown

* guard the test against NoUIFramework compilation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved run-full-ci Allows gated GitHub Action workflows to run for a labelled pull request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants