Skip to content

Conversation

philprime
Copy link
Member

@philprime philprime commented Aug 14, 2025

This pull request updates the initialization of the SentryFileManager class and its usage throughout the codebase to consistently require a dateProvider dependency, in addition to options and dispatchQueueWrapper. This change improves testability and dependency management by making the date provider explicit rather than relying on implicit or singleton-based injection. The update touches both production and test code, ensuring all usages of SentryFileManager are migrated to the new constructor signature.

#skip-changelog

Closes #6115

@philprime philprime self-assigned this Aug 14, 2025
Copy link

codecov bot commented Aug 14, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.677%. Comparing base (1fecbb8) to head (f299e4e).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##              main     #5969       +/-   ##
=============================================
+ Coverage   86.635%   86.677%   +0.041%     
=============================================
  Files          435       435               
  Lines        36935     36938        +3     
  Branches     17396     17389        -7     
=============================================
+ Hits         31999     32017       +18     
+ Misses        4890      4876       -14     
+ Partials        46        45        -1     
Files with missing lines Coverage Δ
SentryTestUtils/TestClient.swift 83.823% <100.000%> (+1.146%) ⬆️
SentryTestUtils/TestFileManager.swift 100.000% <ø> (ø)
Sources/Sentry/SentryClient.m 98.947% <100.000%> (+0.002%) ⬆️
Sources/Sentry/SentryDependencyContainer.m 89.539% <100.000%> (+0.132%) ⬆️
Sources/Sentry/SentryFileManager.m 91.644% <100.000%> (-0.088%) ⬇️

... and 11 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 1fecbb8...f299e4e. Read the comment docs.

Copy link
Contributor

github-actions bot commented Aug 14, 2025

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1214.46 ms 1227.29 ms 12.83 ms
Size 23.75 KiB 969.23 KiB 945.48 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
b9aacb6 1230.42 ms 1251.00 ms 20.58 ms
5cfc768 1220.74 ms 1245.06 ms 24.32 ms
5db87fa 1218.88 ms 1251.53 ms 32.65 ms
570f725 1206.00 ms 1238.96 ms 32.96 ms
cd9727b 1236.04 ms 1254.41 ms 18.37 ms
7908e84 1224.33 ms 1246.39 ms 22.06 ms
67e8e3e 1220.08 ms 1229.23 ms 9.15 ms
87fb58a 1233.12 ms 1257.17 ms 24.04 ms
e3767a1 1224.20 ms 1257.16 ms 32.96 ms
409a607 1229.57 ms 1251.45 ms 21.88 ms

App size

Revision Plain With Sentry Diff
b9aacb6 23.75 KiB 913.64 KiB 889.89 KiB
5cfc768 23.75 KiB 850.73 KiB 826.98 KiB
5db87fa 23.75 KiB 926.65 KiB 902.90 KiB
570f725 23.74 KiB 913.38 KiB 889.63 KiB
cd9727b 23.75 KiB 879.25 KiB 855.51 KiB
7908e84 23.74 KiB 872.75 KiB 849.00 KiB
67e8e3e 23.75 KiB 919.91 KiB 896.16 KiB
87fb58a 23.75 KiB 919.91 KiB 896.16 KiB
e3767a1 23.75 KiB 913.14 KiB 889.39 KiB
409a607 23.74 KiB 874.08 KiB 850.33 KiB

Previous results on branch: philprime/remove-shared-container

Startup times

Revision Plain With Sentry Diff
2ad3a94 1232.45 ms 1258.14 ms 25.70 ms
77702a7 1221.35 ms 1257.67 ms 36.32 ms
e9e69ca 1232.06 ms 1250.65 ms 18.58 ms
e8a62ad 1227.96 ms 1251.33 ms 23.37 ms

App size

Revision Plain With Sentry Diff
2ad3a94 23.75 KiB 969.22 KiB 945.48 KiB
77702a7 23.75 KiB 963.41 KiB 939.66 KiB
e9e69ca 23.75 KiB 954.09 KiB 930.33 KiB
e8a62ad 23.75 KiB 969.22 KiB 945.48 KiB

@philprime philprime marked this pull request as ready for review September 5, 2025 11:34
cursor[bot]

This comment was marked as outdated.

Copy link
Member

@philipphofmann philipphofmann left a comment

Choose a reason for hiding this comment

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

OMG that simple change surfaced in tons of locations. LGTM, thanks.

@philprime philprime enabled auto-merge (squash) September 10, 2025 09:24
Copy link
Contributor

🚨 Detected changes in high risk code 🚨

High-risk code can easily blow up and is hard to test. We had severe bugs in the past. Be extra careful when changing these files, and have an extra careful look at these:

  • Sources/Sentry/SentryFileManager.m

@philprime philprime merged commit f0e2579 into main Sep 10, 2025
186 of 189 checks passed
@philprime philprime deleted the philprime/remove-shared-container branch September 10, 2025 11:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

refactor: Remove access to shared container from SentryFileManager
2 participants