fix: Inaccurate number of frames for transactions

[philipphofmann]

📜 Description

The total frames were too high for transactions because the SDK subscribed twice to the CADisplayLink. This is fixed now by only subscribing once. Furthermore, this PR adds a UI test to validate that the number of total frames is not entirely off.

I also added Nimble to the SwiftUI tests and had to raise the minimum deployment target of the UI tests to iOS 13, which is okay because we don't run our UI tests lower than iOS 13.

💡 Motivation and Context

Somebody reported too high frame rates for transactions.

💚 How did you test it?

Unit tests, UI tests, and manual testing with a simulator.

📝 Checklist

You have to check all boxes before merging:

• I reviewed the submitted code.
• 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.
• 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.

🔮 Next steps

[krystofwoldrich]

Looks good 🚀

[kahest]

good catch, also thanks for the (tricky) UI tests

[kahest]

that is subtle 🔍

[philipphofmann]

Thanks, @krystofwoldrich, but I am also waiting for @brustolin for the iOS-related part.

[codecov]

Codecov Report

Merging #3439 (ddfd38e) into main (59ea421) will decrease coverage by 0.031%.
The diff coverage is 100.000%.

Additional details and impacted files

@@              Coverage Diff              @@
##              main     #3439       +/-   ##
=============================================
- Coverage   89.024%   88.994%   -0.031%     
=============================================
  Files          525       525               
  Lines        56574     56610       +36     
  Branches     20361     20380       +19     
=============================================
+ Hits         50365     50380       +15     
- Misses        5293      5309       +16     
- Partials       916       921        +5     

Files	Coverage Δ
SentryTestUtils/TestDisplayLinkWrapper.swift	94.871% <100.000%> (+0.134%)	⬆️
Sources/Sentry/SentryFramesTracker.m	98.550% <100.000%> (+0.032%)	⬆️
...ance/FramesTracking/SentryFramesTrackerTests.swift	97.422% <100.000%> (+0.247%)	⬆️

... and 22 files with indirect coverage changes

---

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 59ea421...ddfd38e. Read the comment docs.

[github-actions]

Performance metrics 🚀

	Plain	With Sentry	Diff
Startup time	1225.58 ms	1235.52 ms	9.94 ms
Size	22.85 KiB	413.42 KiB	390.57 KiB

Baseline results on branch: main

Startup times

Revision	Plain	With Sentry	Diff
ecd9ecd	1191.76 ms	1216.92 ms	25.16 ms
14a1101	1242.71 ms	1272.02 ms	29.31 ms
608755c	1220.23 ms	1239.76 ms	19.52 ms
9e6665f	1233.33 ms	1257.14 ms	23.81 ms
881a955	1209.47 ms	1225.94 ms	16.47 ms
760d953	1226.81 ms	1230.22 ms	3.41 ms
94b89eb	1236.08 ms	1264.58 ms	28.50 ms
32e64d1	1224.45 ms	1238.94 ms	14.49 ms
2cf1b84	1221.10 ms	1230.34 ms	9.24 ms
9acdca2	1242.45 ms	1249.63 ms	7.18 ms

App size

Revision	Plain	With Sentry	Diff
ecd9ecd	20.76 KiB	420.23 KiB	399.47 KiB
14a1101	20.76 KiB	436.25 KiB	415.49 KiB
608755c	20.76 KiB	436.33 KiB	415.57 KiB
9e6665f	22.84 KiB	403.52 KiB	380.67 KiB
881a955	22.85 KiB	407.63 KiB	384.78 KiB
760d953	20.76 KiB	434.94 KiB	414.18 KiB
94b89eb	20.76 KiB	399.20 KiB	378.43 KiB
32e64d1	20.76 KiB	433.18 KiB	412.42 KiB
2cf1b84	20.76 KiB	431.91 KiB	411.15 KiB
9acdca2	22.84 KiB	401.44 KiB	378.59 KiB

Previous results on branch: fix/frames-tracking

Startup times

Revision	Plain	With Sentry	Diff
637076b	1222.80 ms	1259.10 ms	36.31 ms

App size

Revision	Plain	With Sentry	Diff
637076b	22.85 KiB	413.42 KiB	390.57 KiB

[brustolin]

LGTM

[brustolin]

This is kinda weird codewise speaking.
A function call that is compared to a value, but the comparison result is "not used", so weird.

Not advocating for changing back to not use nimble.
Just that, this is the first time I actually seeing this being used, it was weird.

[kahest]

with Nimble you can also write it like this

expect(actual).to(beGreaterThanOrEqualTo(expected))

but I prefer the shorthand, even though it needs a bit getting used to

[philipphofmann]

Yes, me too. It needs some time, but I also prefer it. Anyways, I had to remove Nimble for UI tests cause we run them on iOS 12 and our Nimble version only works on iOS 13 and above.