ref(profiling): move test helpers to separate code unit; prefix with _sentry

[armcknight]

Following on #3872, for #3555, #skip-changelog

Move some functions only used for testing purposes to a separate code unit SentryProfilerTestHelpers

[codecov]

Codecov Report

Attention: Patch coverage is 71.79487% with 22 lines in your changes are missing coverage. Please review.

Project coverage is 90.653%. Comparing base (94e1968) to head (9d0cb71).
Report is 4 commits behind head on main.

Additional details and impacted files

@@              Coverage Diff              @@
##              main     #3873       +/-   ##
=============================================
+ Coverage   90.639%   90.653%   +0.013%     
=============================================
  Files          579       580        +1     
  Lines        45298     45289        -9     
  Branches     16121     16126        +5     
=============================================
- Hits         41058     41056        -2     
+ Misses        4170      4055      -115     
- Partials        70       178      +108     

Files	Coverage Δ
Sources/Sentry/SentryProfiler.mm	89.036% <100.000%> (+3.703%)	⬆️
Tests/SentryProfilerTests/SentryProfilerTests.mm	98.773% <100.000%> (ø)
...ance/FramesTracking/SentryFramesTrackerTests.swift	99.501% <100.000%> (ø)
Tests/SentryTests/PrivateSentrySDKOnlyTests.swift	99.236% <100.000%> (ø)
...sts/SentryCrash/SentryStacktraceBuilderTests.swift	98.000% <100.000%> (ø)
...ts/SentryTests/Transaction/SentryTracerTests.swift	98.339% <100.000%> (ø)
...urces/Sentry/Profiling/SentryProfilerTestHelpers.m	67.647% <67.647%> (ø)
Sources/Sentry/SentryProfileTimeseries.mm	57.831% <64.516%> (+3.985%)	⬆️

... and 31 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 94e1968...9d0cb71. Read the comment docs.

[github-actions]

Performance metrics 🚀

	Plain	With Sentry	Diff
Startup time	1230.20 ms	1248.71 ms	18.51 ms
Size	21.58 KiB	615.03 KiB	593.45 KiB

[brustolin]

Personally, I don't like this approach of adding so many test compiler declaratives; it makes it harder to read, and we will be testing code that will not be the same as what we ship.

That being said, it works, so I won't oppose it.

[philipphofmann]

Personally, I don't like this approach of adding so many test compiler declaratives; it makes it harder to read, and we will be testing code that will not be the same as what we ship.

I agree with you @brustolin 👍 , but this PR only moves the existing stuff around. We should discuss this somewhere else.

[armcknight]

Personally, I don't like this approach of adding so many test compiler declaratives; it makes it harder to read, and we will be testing code that will not be the same as what we ship.

That being said, it works, so I won't oppose it.

I understand where you're coming from. In this case, I think it's fine because it's not code that is pertinent to how the SDK works in production on our customers' enduser devices. It's only code that is pertinent to the tests:

• One function disables the profiler when TSAN is attached. This will never be the case in production.
• The other function writes a file containing profile data when testing. This one is riskier because it's theoretically possible that someone could mutate the profiling data structures in this function thereby changing the behavior of the SDK only in tests, but I think that risk is very low given the nature of the function itself and the safeguards we've built around copying profiling data when it is accessed, and protecting the underlying data structures against such mutation (we built that in response to previous inadvertent mutations).

This is I think much less risky than the debate over whether to enable NSAssert in test and prod or not.