fix tests

getsentry · Oct 11, 2023 · 51d8267 · 51d8267
1 parent b22b7ba
commit 51d8267
Show file tree

Hide file tree

Showing 4 changed files with 67 additions and 40 deletions.
diff --git a/Sources/Sentry/SentryReachability.m b/Sources/Sentry/SentryReachability.m
@@ -150,12 +150,22 @@ - (instancetype)init
 - (void)addObserver:(id<SentryReachabilityObserver>)observer;
 {
     @synchronized(sentry_reachability_observers) {
+        if ([sentry_reachability_observers containsObject:observer]) {
+            SENTRY_LOG_DEBUG(@"Observer already added. Doing nothing.");
+            return;
+        }
+
         [sentry_reachability_observers addObject:observer];
 
         if (sentry_reachability_observers.count > 1) {
             return;
         }
 
+        if (!self.setReachabilityCallback) {
+            SENTRY_LOG_DEBUG(@"Skipping setting reachability callback.");
+            return;
+        }
+
         sentry_reachability_queue
             = dispatch_queue_create("io.sentry.cocoa.connectivity", DISPATCH_QUEUE_SERIAL);
         _sentry_reachability_ref = SCNetworkReachabilityCreateWithName(NULL, "sentry.io");
@@ -164,10 +174,8 @@ - (void)addObserver:(id<SentryReachabilityObserver>)observer;
         }
 
         SENTRY_LOG_DEBUG(@"registering callback for reachability ref %@", _sentry_reachability_ref);
-        if (self.setReachabilityCallback) {
-            SCNetworkReachabilitySetCallback(
-                _sentry_reachability_ref, SentryConnectivityCallback, NULL);
-        }
+        SCNetworkReachabilitySetCallback(
+            _sentry_reachability_ref, SentryConnectivityCallback, NULL);
         SCNetworkReachabilitySetDispatchQueue(_sentry_reachability_ref, sentry_reachability_queue);
     }
 }
@@ -199,6 +207,11 @@ - (void)unsetReachabilityCallbackIfNeeded
         return;
     }
 
+    if (!self.setReachabilityCallback) {
+        SENTRY_LOG_DEBUG(@"Skipping unsetting reachability callback.");
+        return;
+    }
+
     sentry_current_reachability_state = kSCNetworkReachabilityFlagsUninitialized;
 
     if (_sentry_reachability_ref != nil) {

diff --git a/Sources/Sentry/include/SentryReachability.h b/Sources/Sentry/include/SentryReachability.h
@@ -64,7 +64,7 @@ SENTRY_EXTERN NSString *const SentryConnectivityNone;
 /**
  * Only needed for testing.
  */
-@property (nonatomic) BOOL setReachabilityCallback;
+@property (nonatomic, assign) BOOL setReachabilityCallback;
 
 /**
  * Add an observer which is called each time network connectivity changes.

diff --git a/Tests/SentryTests/Networking/SentryReachabilitySwiftTests.swift b/Tests/SentryTests/Networking/SentryReachabilitySwiftTests.swift
@@ -1,20 +1,19 @@
 import XCTest
 
-final class SentryReachabilitySwiftTests: XCTestCase, SentryReachabilityObserver {
+final class SentryReachabilitySwiftTests: XCTestCase {
 
     func testAddRemoveFromMultipleThreads() throws {
         let sut = SentryReachability()
         testConcurrentModifications(writeWork: {_ in
-            sut.add(self)
+            sut.add(TestReachabilityObserver())
         }, readWork: {
-            sut.remove(self)
+            sut.removeAllObservers()
         })
-
-        sut.removeAllObservers()
     }
-
+}
+
+class TestReachabilityObserver: NSObject, SentryReachabilityObserver {
     func connectivityChanged(_ connected: Bool, typeDescription: String) {
         // Do nothing
     }
-
 }
diff --git a/Tests/SentryTests/Networking/SentryReachabilityTests.m b/Tests/SentryTests/Networking/SentryReachabilityTests.m
@@ -38,6 +38,9 @@ @implementation SentryReachabilityTest
 - (void)setUp
 {
     self.reachability = [[SentryReachability alloc] init];
+    // Disable the reachability callbacks, cause we call the callbacks manually.
+    // Otherwise, the reachability callbacks are called during later unrelated tests causing flakes.
+    self.reachability.setReachabilityCallback = NO;
 }
 
 - (void)tearDown
@@ -68,88 +71,100 @@ - (void)testConnectivityRepresentations
 
 - (void)testMultipleReachabilityObservers
 {
-    SentryReachability *reachability = [[SentryReachability alloc] init];
-    // Disable the reachability callbacks, cause we call the callbacks manually.
-    // Otherwise, the reachability callbacks are called during later unrelated tests causing flakes.
-    reachability.setReachabilityCallback = NO;
-
     XCTestExpectation *aExp =
         [self expectationWithDescription:
                   @"reachability state change for observer monitoring https://sentry.io"];
     aExp.expectedFulfillmentCount = 5;
     TestSentryReachabilityObserver *a =
         [[TestSentryReachabilityObserver alloc] initWithExpectation:aExp];
-    [reachability addObserver:a];
+    [self.reachability addObserver:a];
 
-    SentryConnectivityCallback(reachability.sentry_reachability_ref,
+    SentryConnectivityCallback(self.reachability.sentry_reachability_ref,
         kSCNetworkReachabilityFlagsReachable, nil); // ignored, as it's the first callback
-    SentryConnectivityCallback(
-        reachability.sentry_reachability_ref, kSCNetworkReachabilityFlagsInterventionRequired, nil);
+    SentryConnectivityCallback(self.reachability.sentry_reachability_ref,
+        kSCNetworkReachabilityFlagsInterventionRequired, nil);
 
     XCTestExpectation *bExp =
         [self expectationWithDescription:
                   @"reachability state change for observer monitoring https://google.io"];
     bExp.expectedFulfillmentCount = 2;
     TestSentryReachabilityObserver *b =
         [[TestSentryReachabilityObserver alloc] initWithExpectation:bExp];
-    [reachability addObserver:b];
+    [self.reachability addObserver:b];
 
     SentryConnectivityCallback(
-        reachability.sentry_reachability_ref, kSCNetworkReachabilityFlagsReachable, nil);
-    SentryConnectivityCallback(
-        reachability.sentry_reachability_ref, kSCNetworkReachabilityFlagsInterventionRequired, nil);
+        self.reachability.sentry_reachability_ref, kSCNetworkReachabilityFlagsReachable, nil);
+    SentryConnectivityCallback(self.reachability.sentry_reachability_ref,
+        kSCNetworkReachabilityFlagsInterventionRequired, nil);
 
-    [reachability removeObserver:b];
+    [self.reachability removeObserver:b];
 
     SentryConnectivityCallback(
-        reachability.sentry_reachability_ref, kSCNetworkReachabilityFlagsReachable, nil);
+        self.reachability.sentry_reachability_ref, kSCNetworkReachabilityFlagsReachable, nil);
 
     [self waitForExpectations:@[ aExp, bExp ] timeout:1.0];
 
-    [reachability removeObserver:a];
+    [self.reachability removeObserver:a];
 }
 
 - (void)testNoObservers
 {
-    SentryReachability *reachability = [[SentryReachability alloc] init];
-
     XCTestExpectation *aExp =
         [self expectationWithDescription:
                   @"reachability state change for observer monitoring https://sentry.io"];
     [aExp setInverted:YES];
     TestSentryReachabilityObserver *a =
         [[TestSentryReachabilityObserver alloc] initWithExpectation:aExp];
-    [reachability addObserver:a];
-    [reachability removeObserver:a];
+    [self.reachability addObserver:a];
+    [self.reachability removeObserver:a];
 
     SentryConnectivityCallback(
-        reachability.sentry_reachability_ref, kSCNetworkReachabilityFlagsReachable, nil);
+        self.reachability.sentry_reachability_ref, kSCNetworkReachabilityFlagsReachable, nil);
 
     [self waitForExpectations:@[ aExp ] timeout:1.0];
+
+    [self.reachability removeAllObservers];
 }
 
-- (void)testReportSameReachabilityState_OnlyCalledOnce
+- (void)testReportSameObserver_OnlyCalledOnce
 {
-    SentryReachability *reachability = [[SentryReachability alloc] init];
+    XCTestExpectation *aExp =
+        [self expectationWithDescription:
+                  @"reachability state change for observer monitoring https://sentry.io"];
+    aExp.expectedFulfillmentCount = 1;
+    TestSentryReachabilityObserver *a =
+        [[TestSentryReachabilityObserver alloc] initWithExpectation:aExp];
+    [self.reachability addObserver:a];
+    [self.reachability addObserver:a];
+
+    SentryConnectivityCallback(
+        self.reachability.sentry_reachability_ref, kSCNetworkReachabilityFlagsReachable, nil);
 
+    [self waitForExpectations:@[ aExp ] timeout:1.0];
+
+    [self.reachability removeObserver:a];
+}
+
+- (void)testReportSameReachabilityState_OnlyCalledOnce
+{
     XCTestExpectation *aExp =
         [self expectationWithDescription:
                   @"reachability state change for observer monitoring https://sentry.io"];
     aExp.expectedFulfillmentCount = 1;
     TestSentryReachabilityObserver *a =
         [[TestSentryReachabilityObserver alloc] initWithExpectation:aExp];
-    [reachability addObserver:a];
+    [self.reachability addObserver:a];
 
     SentryConnectivityCallback(
-        reachability.sentry_reachability_ref, kSCNetworkReachabilityFlagsReachable, nil);
+        self.reachability.sentry_reachability_ref, kSCNetworkReachabilityFlagsReachable, nil);
     SentryConnectivityCallback(
-        reachability.sentry_reachability_ref, kSCNetworkReachabilityFlagsReachable, nil);
+        self.reachability.sentry_reachability_ref, kSCNetworkReachabilityFlagsReachable, nil);
     SentryConnectivityCallback(
-        reachability.sentry_reachability_ref, kSCNetworkReachabilityFlagsReachable, nil);
+        self.reachability.sentry_reachability_ref, kSCNetworkReachabilityFlagsReachable, nil);
 
     [self waitForExpectations:@[ aExp ] timeout:1.0];
 
-    [reachability removeObserver:a];
+    [self.reachability removeObserver:a];
 }
 
 @end