Skip to content

Commit

Permalink
Fix some random Darwin test failures due to timing dependency. (#33761)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
bzbarsky-apple authored Jun 5, 2024
1 parent d4d9054 commit 8b8dadd
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 9 deletions.
28 changes: 19 additions & 9 deletions src/darwin/Framework/CHIP/MTRDevice.mm
Original file line number Diff line number Diff line change
Expand Up @@ -383,6 +383,7 @@ - (BOOL)unitTestPretendThreadEnabled:(MTRDevice *)device;
- (void)unitTestSubscriptionPoolDequeue:(MTRDevice *)device;
- (void)unitTestSubscriptionPoolWorkComplete:(MTRDevice *)device;
- (void)unitTestClusterDataPersisted:(MTRDevice *)device;
- (BOOL)unitTestSuppressTimeBasedReachabilityChanges:(MTRDevice *)device;
@end
#endif

Expand Down Expand Up @@ -2026,15 +2027,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<MTRDevice *> * weakSelf = [MTRWeakReference weakReferenceWithObject:self];
dispatch_after(dispatch_time(DISPATCH_TIME_NOW, static_cast<int64_t>(kSecondsToWaitBeforeMarkingUnreachableAfterSettingUpSubscription) * static_cast<int64_t>(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<MTRDevice *> * weakSelf = [MTRWeakReference weakReferenceWithObject:self];
dispatch_after(dispatch_time(DISPATCH_TIME_NOW, static_cast<int64_t>(kSecondsToWaitBeforeMarkingUnreachableAfterSettingUpSubscription) * static_cast<int64_t>(NSEC_PER_SEC)), self.queue, ^{
MTRDevice * strongSelf = weakSelf.strongObject;
if (strongSelf != nil) {
std::lock_guard lock(strongSelf->_lock);
[strongSelf _markDeviceAsUnreachableIfNeverSubscribed];
}
});
}

[_deviceController
getSessionForNode:_nodeID.unsignedLongLongValue
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 8b8dadd

Please sign in to comment.