From 1a18a07274009305e7062d750413140f8b8b3bc4 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Wed, 5 Jun 2024 16:12:32 -0400 Subject: [PATCH] Fix some random Darwin test failures due to timing dependency. Some Darwin tests failed randomly if a subscription took longer than 10s to establish. Add test-only conditionals around the relevant code so we suppress that behavior in tests. --- src/darwin/Framework/CHIP/MTRDevice.mm | 28 +++++++++++++------ .../TestHelpers/MTRDeviceTestDelegate.m | 9 ++++++ 2 files changed, 28 insertions(+), 9 deletions(-) diff --git a/src/darwin/Framework/CHIP/MTRDevice.mm b/src/darwin/Framework/CHIP/MTRDevice.mm index dcb6a9acd8da64..8f29b38f4cfb1e 100644 --- a/src/darwin/Framework/CHIP/MTRDevice.mm +++ b/src/darwin/Framework/CHIP/MTRDevice.mm @@ -376,6 +376,7 @@ - (BOOL)unitTestPretendThreadEnabled:(MTRDevice *)device; - (void)unitTestSubscriptionPoolDequeue:(MTRDevice *)device; - (void)unitTestSubscriptionPoolWorkComplete:(MTRDevice *)device; - (void)unitTestClusterDataPersisted:(MTRDevice *)device; +- (BOOL)unitTestSuppressTimeBasedReachabilityChanges:(MTRDevice *)device; @end #endif @@ -2014,15 +2015,24 @@ - (void)_setupSubscriptionWithReason:(NSString *)reason MTR_LOG("%@ setting up subscription with reason: %@", self, reason); - // Set up a timer to mark as not reachable if it takes too long to set up a subscription - MTRWeakReference * weakSelf = [MTRWeakReference weakReferenceWithObject:self]; - dispatch_after(dispatch_time(DISPATCH_TIME_NOW, static_cast(kSecondsToWaitBeforeMarkingUnreachableAfterSettingUpSubscription) * static_cast(NSEC_PER_SEC)), self.queue, ^{ - MTRDevice * strongSelf = weakSelf.strongObject; - if (strongSelf != nil) { - std::lock_guard lock(strongSelf->_lock); - [strongSelf _markDeviceAsUnreachableIfNeverSubscribed]; - } - }); + bool markUnreachableAfterWait = true; +#ifdef DEBUG + if (delegate && [delegate respondsToSelector:@selector(unitTestSuppressTimeBasedReachabilityChanges:)]) { + markUnreachableAfterWait = ![delegate unitTestSuppressTimeBasedReachabilityChanges:self]; + } +#endif + + if (markUnreachableAfterWait) { + // Set up a timer to mark as not reachable if it takes too long to set up a subscription + MTRWeakReference * weakSelf = [MTRWeakReference weakReferenceWithObject:self]; + dispatch_after(dispatch_time(DISPATCH_TIME_NOW, static_cast(kSecondsToWaitBeforeMarkingUnreachableAfterSettingUpSubscription) * static_cast(NSEC_PER_SEC)), self.queue, ^{ + MTRDevice * strongSelf = weakSelf.strongObject; + if (strongSelf != nil) { + std::lock_guard lock(strongSelf->_lock); + [strongSelf _markDeviceAsUnreachableIfNeverSubscribed]; + } + }); + } [_deviceController getSessionForNode:_nodeID.unsignedLongLongValue diff --git a/src/darwin/Framework/CHIPTests/TestHelpers/MTRDeviceTestDelegate.m b/src/darwin/Framework/CHIPTests/TestHelpers/MTRDeviceTestDelegate.m index d896906fa99c01..740f5f71cf8b7c 100644 --- a/src/darwin/Framework/CHIPTests/TestHelpers/MTRDeviceTestDelegate.m +++ b/src/darwin/Framework/CHIPTests/TestHelpers/MTRDeviceTestDelegate.m @@ -103,6 +103,15 @@ - (void)unitTestClusterDataPersisted:(MTRDevice *)device } } +- (BOOL)unitTestSuppressTimeBasedReachabilityChanges:(MTRDevice *)device +{ + // Allowing time-based reachability changes just makes the tests + // non-deterministic and can lead to random failures. Suppress them + // unconditionally for now. If we ever add tests that try to exercise that + // codepath, we can make this configurable. + return YES; +} + @end @implementation MTRDeviceTestDelegateWithSubscriptionSetupOverride