ref: Properly teardown frames tracker (#3545)

The teardown of the SentryFramesTracker didn't work properly in tests. The clearTestState method continuously initialized a new frames tracker cause it accessed the property dependency container. This is fixed now by calling SentryFramesTracker.stop directly in SentryDependencyContainer.reset which is also called in clearTestState. Furthermore, the SentryFramesTracker.stop now resets all frames and removes all observers.
getsentry · Jan 5, 2024 · 1734d1b · 1734d1b
1 parent 253f628
commit 1734d1b
Show file tree

Hide file tree

Showing 6 changed files with 62 additions and 11 deletions.
diff --git a/Sentry.xcodeproj/project.pbxproj b/Sentry.xcodeproj/project.pbxproj
@@ -79,6 +79,7 @@
 		620379DD2AFE1432005AC0C1 /* SentryBuildAppStartSpans.m in Sources */ = {isa = PBXBuildFile; fileRef = 620379DC2AFE1432005AC0C1 /* SentryBuildAppStartSpans.m */; };
 		622C08D829E546F4002571D4 /* SentryTraceOrigins.h in Headers */ = {isa = PBXBuildFile; fileRef = 622C08D729E546F4002571D4 /* SentryTraceOrigins.h */; };
 		622C08DB29E554B9002571D4 /* SentrySpanContext+Private.h in Headers */ = {isa = PBXBuildFile; fileRef = 622C08D929E554B9002571D4 /* SentrySpanContext+Private.h */; };
+		62375FB92B47F9F000CC55F1 /* SentryDependencyContainerTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 62375FB82B47F9F000CC55F1 /* SentryDependencyContainerTests.swift */; };
 		623C45B02A651D8200D9E88B /* SentryCoreDataTracker+Test.m in Sources */ = {isa = PBXBuildFile; fileRef = 623C45AF2A651D8200D9E88B /* SentryCoreDataTracker+Test.m */; };
 		627E7589299F6FE40085504D /* SentryInternalDefines.h in Headers */ = {isa = PBXBuildFile; fileRef = 627E7588299F6FE40085504D /* SentryInternalDefines.h */; };
 		62862B1C2B1DDBC8009B16E3 /* SentryDelayedFrame.h in Headers */ = {isa = PBXBuildFile; fileRef = 62862B1B2B1DDBC8009B16E3 /* SentryDelayedFrame.h */; };
@@ -92,9 +93,9 @@
 		62A456E52B0370E0003F19A1 /* SentryUIEventTrackerTransactionMode.m in Sources */ = {isa = PBXBuildFile; fileRef = 62A456E42B0370E0003F19A1 /* SentryUIEventTrackerTransactionMode.m */; };
 		62B86CFC29F052BB008F3947 /* SentryTestLogConfig.m in Sources */ = {isa = PBXBuildFile; fileRef = 62B86CFB29F052BB008F3947 /* SentryTestLogConfig.m */; };
 		62C25C862B075F4900C68CBD /* TestOptions.swift in Sources */ = {isa = PBXBuildFile; fileRef = 62C25C852B075F4900C68CBD /* TestOptions.swift */; };
-		62C3168B2B1F865A000D7031 /* SentryTimeSwiftTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 62C3168A2B1F865A000D7031 /* SentryTimeSwiftTests.swift */; };
 		62C316812B1F2E93000D7031 /* SentryDelayedFramesTracker.h in Headers */ = {isa = PBXBuildFile; fileRef = 62C316802B1F2E93000D7031 /* SentryDelayedFramesTracker.h */; };
 		62C316832B1F2EA1000D7031 /* SentryDelayedFramesTracker.m in Sources */ = {isa = PBXBuildFile; fileRef = 62C316822B1F2EA1000D7031 /* SentryDelayedFramesTracker.m */; };
+		62C3168B2B1F865A000D7031 /* SentryTimeSwiftTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 62C3168A2B1F865A000D7031 /* SentryTimeSwiftTests.swift */; };
 		62E081A929ED4260000F69FC /* SentryBreadcrumbDelegate.h in Headers */ = {isa = PBXBuildFile; fileRef = 62E081A829ED4260000F69FC /* SentryBreadcrumbDelegate.h */; };
 		62E081AB29ED4322000F69FC /* SentryBreadcrumbTestDelegate.swift in Sources */ = {isa = PBXBuildFile; fileRef = 62E081AA29ED4322000F69FC /* SentryBreadcrumbTestDelegate.swift */; };
 		62F226B729A37C120038080D /* SentryBooleanSerialization.m in Sources */ = {isa = PBXBuildFile; fileRef = 62F226B629A37C120038080D /* SentryBooleanSerialization.m */; };
@@ -996,6 +997,7 @@
 		620379DC2AFE1432005AC0C1 /* SentryBuildAppStartSpans.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = SentryBuildAppStartSpans.m; sourceTree = "<group>"; };
 		622C08D729E546F4002571D4 /* SentryTraceOrigins.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = SentryTraceOrigins.h; path = include/SentryTraceOrigins.h; sourceTree = "<group>"; };
 		622C08D929E554B9002571D4 /* SentrySpanContext+Private.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = "SentrySpanContext+Private.h"; path = "include/SentrySpanContext+Private.h"; sourceTree = "<group>"; };
+		62375FB82B47F9F000CC55F1 /* SentryDependencyContainerTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SentryDependencyContainerTests.swift; sourceTree = "<group>"; };
 		623C45AE2A651C4500D9E88B /* SentryCoreDataTracker+Test.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = "SentryCoreDataTracker+Test.h"; sourceTree = "<group>"; };
 		623C45AF2A651D8200D9E88B /* SentryCoreDataTracker+Test.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = "SentryCoreDataTracker+Test.m"; sourceTree = "<group>"; };
 		627E7588299F6FE40085504D /* SentryInternalDefines.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = SentryInternalDefines.h; path = include/SentryInternalDefines.h; sourceTree = "<group>"; };
@@ -1009,9 +1011,9 @@
 		62A456E42B0370E0003F19A1 /* SentryUIEventTrackerTransactionMode.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = SentryUIEventTrackerTransactionMode.m; sourceTree = "<group>"; };
 		62B86CFB29F052BB008F3947 /* SentryTestLogConfig.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = SentryTestLogConfig.m; sourceTree = "<group>"; };
 		62C25C852B075F4900C68CBD /* TestOptions.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TestOptions.swift; sourceTree = "<group>"; };
-		62C3168A2B1F865A000D7031 /* SentryTimeSwiftTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SentryTimeSwiftTests.swift; sourceTree = "<group>"; };
 		62C316802B1F2E93000D7031 /* SentryDelayedFramesTracker.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = SentryDelayedFramesTracker.h; path = include/SentryDelayedFramesTracker.h; sourceTree = "<group>"; };
 		62C316822B1F2EA1000D7031 /* SentryDelayedFramesTracker.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = SentryDelayedFramesTracker.m; sourceTree = "<group>"; };
+		62C3168A2B1F865A000D7031 /* SentryTimeSwiftTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SentryTimeSwiftTests.swift; sourceTree = "<group>"; };
 		62E081A829ED4260000F69FC /* SentryBreadcrumbDelegate.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = SentryBreadcrumbDelegate.h; path = include/SentryBreadcrumbDelegate.h; sourceTree = "<group>"; };
 		62E081AA29ED4322000F69FC /* SentryBreadcrumbTestDelegate.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SentryBreadcrumbTestDelegate.swift; sourceTree = "<group>"; };
 		62F226B629A37C120038080D /* SentryBooleanSerialization.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = SentryBooleanSerialization.m; sourceTree = "<group>"; };
@@ -2866,6 +2868,7 @@
 				8431EE5A29ADB8EA00D8DC56 /* SentryTimeTests.m */,
 				62C3168A2B1F865A000D7031 /* SentryTimeSwiftTests.swift */,
 				33042A1629DC2C4300C60085 /* SentryExtraContextProviderTests.swift */,
+				62375FB82B47F9F000CC55F1 /* SentryDependencyContainerTests.swift */,
 			);
 			path = Helper;
 			sourceTree = "<group>";
@@ -4460,6 +4463,7 @@
 				7B18DE4A28DA0C8B004845C6 /* SentryNSNotificationCenterWrapperTests.swift in Sources */,
 				7BEFB044270B0F630025F808 /* SentryTracerObjCTests.m in Sources */,
 				7B0002322477F0520035FEF1 /* SentrySessionTests.m in Sources */,
+				62375FB92B47F9F000CC55F1 /* SentryDependencyContainerTests.swift in Sources */,
 				7BC6EC08255C36DE0059822A /* SentryStacktraceTests.swift in Sources */,
 				D88817DD26D72BA500BF2251 /* SentryTraceStateTests.swift in Sources */,
 				7B26BBFB24C0A66D00A79CCC /* SentrySdkInfoNilTests.m in Sources */,

diff --git a/SentryTestUtils/ClearTestState.swift b/SentryTestUtils/ClearTestState.swift
@@ -26,9 +26,6 @@ class TestCleanup: NSObject {
         setTestDefaultLogLevel()
 
         #if os(iOS) || os(tvOS) || targetEnvironment(macCatalyst)
-        let framesTracker = SentryDependencyContainer.sharedInstance().framesTracker
-        framesTracker.stop()
-        framesTracker.resetFrames()
 
         setenv("ActivePrewarm", "0", 1)
         SentryAppStartTracker.load()

diff --git a/Sources/Sentry/SentryDependencyContainer.m b/Sources/Sentry/SentryDependencyContainer.m
@@ -63,12 +63,16 @@ + (instancetype)sharedInstance
 
 + (void)reset
 {
-#if !TARGET_OS_WATCH
     if (instance) {
+#if !TARGET_OS_WATCH
         [instance->_reachability removeAllObservers];
-    }
 #endif // !TARGET_OS_WATCH
 
+#if SENTRY_HAS_UIKIT
+        [instance->_framesTracker stop];
+#endif // SENTRY_HAS_UIKIT
+    }
+
     instance = [[SentryDependencyContainer alloc] init];
 }
 

diff --git a/Sources/Sentry/SentryFramesTracker.m b/Sources/Sentry/SentryFramesTracker.m
@@ -83,7 +83,6 @@ - (void)setDisplayLinkWrapper:(SentryDisplayLinkWrapper *)displayLinkWrapper
     _displayLinkWrapper = displayLinkWrapper;
 }
 
-/** Internal for testing */
 - (void)resetFrames
 {
     _totalFrames = 0;
@@ -277,6 +276,9 @@ - (void)stop
     _isRunning = NO;
     [self.displayLinkWrapper invalidate];
     [self.delayedFramesTracker resetDelayedFramesTimeStamps];
+    @synchronized(self.listeners) {
+        [self.listeners removeAllObjects];
+    }
 }
 
 - (void)dealloc

diff --git a/Tests/SentryTests/Helper/SentryDependencyContainerTests.swift b/Tests/SentryTests/Helper/SentryDependencyContainerTests.swift
@@ -0,0 +1,15 @@
+import Nimble
+import XCTest
+
+#if os(iOS) || os(tvOS) || targetEnvironment(macCatalyst)
+final class SentryDependencyContainerTests: XCTestCase {
+
+    func testReset_CallsFramesTrackerStop() throws {
+        let framesTracker = SentryDependencyContainer.sharedInstance().framesTracker
+        framesTracker.start()
+        SentryDependencyContainer.reset()
+
+        expect(framesTracker.isRunning) == false
+    }
+}
+#endif
diff --git a/Tests/SentryTests/Integrations/Performance/FramesTracking/SentryFramesTrackerTests.swift b/Tests/SentryTests/Integrations/Performance/FramesTracking/SentryFramesTrackerTests.swift
@@ -69,6 +69,19 @@ class SentryFramesTrackerTests: XCTestCase {
         expect(self.fixture.sut.isRunning) == false
     }
 
+    func testKeepFrames_WhenStopped() throws {
+        let sut = fixture.sut
+        sut.start()
+
+        let displayLink = fixture.displayLinkWrapper
+        displayLink.call()
+        displayLink.normalFrame()
+
+        sut.stop()
+
+        try assert(slow: 0, frozen: 0, total: 1)
+    }
+
     func testStartAfterStopped_SubscribesTwiceToDisplayLink() {
         let sut = fixture.sut
         sut.start()
@@ -550,6 +563,22 @@ class SentryFramesTrackerTests: XCTestCase {
 
         expect(listener.newFrameInvocations.count) == 0
     }
+
+    func testListenerNotCalledAfterCallingStop() {
+        let sut = fixture.sut
+        let listener1 = FrameTrackerListener()
+        let listener2 = FrameTrackerListener()
+        sut.start()
+        sut.add(listener1)
+        sut.stop()
+        sut.start()
+        sut.add(listener2)
+
+        fixture.displayLinkWrapper.normalFrame()
+
+        expect(listener1.newFrameInvocations.count) == 0
+        expect(listener2.newFrameInvocations.count) == 1
+    }
 
     func testReleasedListener() {
         let sut = fixture.sut
@@ -617,13 +646,13 @@ private extension SentryFramesTrackerTests {
     func assert(slow: UInt? = nil, frozen: UInt? = nil, total: UInt? = nil, frameRates: UInt? = nil) throws {
         let currentFrames = fixture.sut.currentFrames
         if let total = total {
-            XCTAssertEqual(total, currentFrames.total)
+            expect(currentFrames.total) == total
         }
         if let slow = slow {
-            XCTAssertEqual(slow, currentFrames.slow)
+            expect(currentFrames.slow) == slow
         }
         if let frozen = frozen {
-            XCTAssertEqual(frozen, currentFrames.frozen)
+            expect(currentFrames.frozen) == frozen
         }
 
 #if os(iOS) || os(macOS) || targetEnvironment(macCatalyst)