From 3419d6cca46c73be29070111bac7bb496566528f Mon Sep 17 00:00:00 2001 From: Philipp Hofmann Date: Mon, 13 Nov 2023 10:34:58 +0100 Subject: [PATCH 1/2] test: Improve SentryReachabilityTests The SentryReachabilityTests are still sometimes failing in CI. We now skip registering and unregistering the actual callbacks to SCNetworkReachability for all tests except one to minimize side effects. --- Sources/Sentry/SentryReachability.m | 19 +++++++++++++++++++ Sources/Sentry/include/SentryReachability.h | 6 ++++++ .../Networking/SentryReachabilityTests.m | 18 +++++++++++++++++- 3 files changed, 42 insertions(+), 1 deletion(-) diff --git a/Sources/Sentry/SentryReachability.m b/Sources/Sentry/SentryReachability.m index 930b5fea345..3d30019ab78 100644 --- a/Sources/Sentry/SentryReachability.m +++ b/Sources/Sentry/SentryReachability.m @@ -44,6 +44,7 @@ void SentrySetReachabilityIgnoreActualCallback(BOOL value) { + SENTRY_LOG_DEBUG(@"Setting ignore actual callback to %@", value ? @"YES" : @"NO"); sentry_reachability_ignore_actual_callback = value; } @@ -155,6 +156,15 @@ + (void)initialize } } +- (instancetype)init +{ + if (self = [super init]) { + self.skipRegisteringActualCallbacks = NO; + } + + return self; +} + - (void)addObserver:(id)observer; { SENTRY_LOG_DEBUG(@"Adding observer: %@", observer); @@ -171,6 +181,11 @@ - (void)addObserver:(id)observer; return; } + if (self.skipRegisteringActualCallbacks) { + SENTRY_LOG_DEBUG(@"Skip registering actual callbacks"); + return; + } + sentry_reachability_queue = dispatch_queue_create("io.sentry.cocoa.connectivity", DISPATCH_QUEUE_SERIAL); // Ensure to call CFRelease for the return value of SCNetworkReachabilityCreateWithName, see @@ -214,6 +229,10 @@ - (void)removeAllObservers - (void)unsetReachabilityCallback { + if (self.skipRegisteringActualCallbacks) { + SENTRY_LOG_DEBUG(@"Skip unsetting actual callbacks"); + } + sentry_current_reachability_state = kSCNetworkReachabilityFlagsUninitialized; if (_sentry_reachability_ref != nil) { diff --git a/Sources/Sentry/include/SentryReachability.h b/Sources/Sentry/include/SentryReachability.h index 4a645df7491..e8e21f617f2 100644 --- a/Sources/Sentry/include/SentryReachability.h +++ b/Sources/Sentry/include/SentryReachability.h @@ -65,6 +65,12 @@ SENTRY_EXTERN NSString *const SentryConnectivityNone; */ @interface SentryReachability : NSObject +/** + * Only needed for testing. Use this flag to skip registering and unregistering the actual callbacks + * to SCNetworkReachability to minimize side effects. + */ +@property (nonatomic, assign) BOOL skipRegisteringActualCallbacks; + /** * Add an observer which is called each time network connectivity changes. */ diff --git a/Tests/SentryTests/Networking/SentryReachabilityTests.m b/Tests/SentryTests/Networking/SentryReachabilityTests.m index ca0b28dc094..147951c05e5 100644 --- a/Tests/SentryTests/Networking/SentryReachabilityTests.m +++ b/Tests/SentryTests/Networking/SentryReachabilityTests.m @@ -35,11 +35,13 @@ @implementation SentryReachabilityTest - (void)setUp { - self.reachability = [[SentryReachability alloc] init]; // Ignore the actual reachability callbacks, cause we call the callbacks manually. // Otherwise, the actual reachability callbacks are called during later unrelated tests causing // flakes. SentrySetReachabilityIgnoreActualCallback(YES); + + self.reachability = [[SentryReachability alloc] init]; + self.reachability.skipRegisteringActualCallbacks = YES; } - (void)tearDown @@ -145,5 +147,19 @@ - (void)testReportSameReachabilityState_OnlyCalledOnce [self.reachability removeObserver:observer]; } +/** + * We only want to make sure running the actual registering and unregistering callbacks doesn't + * crash. + */ +- (void)testRegisteringActualCallbacks +{ + self.reachability.skipRegisteringActualCallbacks = NO; + + TestSentryReachabilityObserver *observer = [[TestSentryReachabilityObserver alloc] init]; + + [self.reachability addObserver:observer]; + [self.reachability removeObserver:observer]; +} + @end #endif // !TARGET_OS_WATCH From 7802da226929914a56673d9859e3d638efbcab25 Mon Sep 17 00:00:00 2001 From: Philipp Hofmann Date: Tue, 14 Nov 2023 11:12:35 +0100 Subject: [PATCH 2/2] use if test --- Sources/Sentry/SentryReachability.m | 16 +++++++++++++++- Sources/Sentry/include/SentryReachability.h | 7 +++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/Sources/Sentry/SentryReachability.m b/Sources/Sentry/SentryReachability.m index 3d30019ab78..fd94a7d8559 100644 --- a/Sources/Sentry/SentryReachability.m +++ b/Sources/Sentry/SentryReachability.m @@ -35,18 +35,21 @@ static SCNetworkReachabilityFlags sentry_current_reachability_state = kSCNetworkReachabilityFlagsUninitialized; static dispatch_queue_t sentry_reachability_queue; -static BOOL sentry_reachability_ignore_actual_callback = NO; NSString *const SentryConnectivityCellular = @"cellular"; NSString *const SentryConnectivityWiFi = @"wifi"; NSString *const SentryConnectivityNone = @"none"; +# if TEST || TESTCI +static BOOL sentry_reachability_ignore_actual_callback = NO; + void SentrySetReachabilityIgnoreActualCallback(BOOL value) { SENTRY_LOG_DEBUG(@"Setting ignore actual callback to %@", value ? @"YES" : @"NO"); sentry_reachability_ignore_actual_callback = value; } +# endif // TEST || TESTCI /** * Check whether the connectivity change should be noted or ignored. @@ -133,10 +136,13 @@ { SENTRY_LOG_DEBUG( @"SentryConnectivityCallback called with target: %@; flags: %u", target, flags); +# if TEST || TESTCI if (sentry_reachability_ignore_actual_callback) { SENTRY_LOG_DEBUG(@"Ignoring actual callback."); return; } +# endif // TEST || TESTCI + SentryConnectivityCallback(flags); } @@ -156,6 +162,8 @@ + (void)initialize } } +# if TEST || TESTCI + - (instancetype)init { if (self = [super init]) { @@ -165,6 +173,8 @@ - (instancetype)init return self; } +# endif // TEST || TESTCI + - (void)addObserver:(id)observer; { SENTRY_LOG_DEBUG(@"Adding observer: %@", observer); @@ -181,10 +191,12 @@ - (void)addObserver:(id)observer; return; } +# if TEST || TESTCI if (self.skipRegisteringActualCallbacks) { SENTRY_LOG_DEBUG(@"Skip registering actual callbacks"); return; } +# endif // TEST || TESTCI sentry_reachability_queue = dispatch_queue_create("io.sentry.cocoa.connectivity", DISPATCH_QUEUE_SERIAL); @@ -229,9 +241,11 @@ - (void)removeAllObservers - (void)unsetReachabilityCallback { +# if TEST || TESTCI if (self.skipRegisteringActualCallbacks) { SENTRY_LOG_DEBUG(@"Skip unsetting actual callbacks"); } +# endif // TEST || TESTCI sentry_current_reachability_state = kSCNetworkReachabilityFlagsUninitialized; diff --git a/Sources/Sentry/include/SentryReachability.h b/Sources/Sentry/include/SentryReachability.h index e8e21f617f2..a40566d3cfe 100644 --- a/Sources/Sentry/include/SentryReachability.h +++ b/Sources/Sentry/include/SentryReachability.h @@ -34,11 +34,14 @@ NS_ASSUME_NONNULL_BEGIN void SentryConnectivityCallback(SCNetworkReachabilityFlags flags); +# if TEST || TESTCI /** * Needed for testing. */ void SentrySetReachabilityIgnoreActualCallback(BOOL value); +# endif // TEST || TESTCI + NSString *SentryConnectivityFlagRepresentation(SCNetworkReachabilityFlags flags); BOOL SentryConnectivityShouldReportChange(SCNetworkReachabilityFlags flags); @@ -65,12 +68,16 @@ SENTRY_EXTERN NSString *const SentryConnectivityNone; */ @interface SentryReachability : NSObject +# if TEST || TESTCI + /** * Only needed for testing. Use this flag to skip registering and unregistering the actual callbacks * to SCNetworkReachability to minimize side effects. */ @property (nonatomic, assign) BOOL skipRegisteringActualCallbacks; +# endif // TEST || TESTCI + /** * Add an observer which is called each time network connectivity changes. */