Skip to content

Commit

Permalink
Stop using MinInterval = 2 in tests that expect reports within 3 seco…
Browse files Browse the repository at this point in the history 
…nds. (project-chip#35707)

We had a bunch of subscription Darwin tests that set a 2-second MinInterval but
expected reports within 3 seconds.  This seems to have worked because for some
reason the 2-second timers fired pretty early (at closer to 1.5 seconds) on the
existing runners, but on the new ARM runners it actually fires at 2 seconds, and
then we have a very high test timeout rate.

The fix is to just use a MinInterval of 0 for the relevant subscriptions.

test012_SubscribeKeepingPreviousSubscription turned out to be relying on not
getting reports from when it sends the Off command until it sends the followup
On command, but now we do in fact send those reports.  So we need to wait for
them to clear through before we send the On command.
  • Loading branch information
@bzbarsky-apple
bzbarsky-apple authored Sep 23, 2024
1 parent 17f25a1 commit 8750c55
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 14 deletions.
4 changes: 2 additions & 2 deletions src/darwin/Framework/CHIPTests/MTRDeviceTests.m
View file
Original file line number Diff line number Diff line change
Expand Up @@ -815,7 +815,7 @@ - (void)test011_ReadCachedAttribute
NSLog(@"Subscribing...");
// reportHandler returns TRUE if it got the things it was looking for or if there's an error.
__block BOOL (^reportHandler)(NSArray * _Nullable value, NSError * _Nullable error);
__auto_type * params = [[MTRSubscribeParams alloc] initWithMinInterval:@(2) maxInterval:@(60)];
__auto_type * params = [[MTRSubscribeParams alloc] initWithMinInterval:@(0) maxInterval:@(60)];
params.resubscribeAutomatically = NO;
[device subscribeWithQueue:queue
params:params
Expand Down Expand Up @@ -881,7 +881,7 @@ - (void)test011_ReadCachedAttribute
// Add another subscriber of the attribute to verify that attribute cache still works when there are other subscribers.
NSLog(@"New subscription...");
XCTestExpectation * newSubscriptionEstablished = [self expectationWithDescription:@"New subscription established"];
MTRSubscribeParams * newParams = [[MTRSubscribeParams alloc] initWithMinInterval:@(2) maxInterval:@(60)];
MTRSubscribeParams * newParams = [[MTRSubscribeParams alloc] initWithMinInterval:@(0) maxInterval:@(60)];
newParams.replaceExistingSubscriptions = NO;
newParams.resubscribeAutomatically = NO;
[cluster subscribeAttributeOnOffWithParams:newParams
Expand Down
72 changes: 60 additions & 12 deletions src/darwin/Framework/CHIPTests/MTRXPCListenerSampleTests.m
View file
Original file line number Diff line number Diff line change
Expand Up @@ -758,7 +758,7 @@ - (void)test004_Subscribe
MTRBaseDevice * device = GetConnectedDevice();
dispatch_queue_t queue = dispatch_get_main_queue();

__auto_type * params = [[MTRSubscribeParams alloc] initWithMinInterval:@(2) maxInterval:@(10)];
__auto_type * params = [[MTRSubscribeParams alloc] initWithMinInterval:@(0) maxInterval:@(10)];
[device subscribeToAttributesWithEndpointID:@1
clusterID:@6
attributeID:@0
Expand Down Expand Up @@ -958,7 +958,7 @@ - (void)test008_SubscribeFailure
MTRBaseDevice * device = GetConnectedDevice();
dispatch_queue_t queue = dispatch_get_main_queue();

__auto_type * params = [[MTRSubscribeParams alloc] initWithMinInterval:@(2) maxInterval:@(10)];
__auto_type * params = [[MTRSubscribeParams alloc] initWithMinInterval:@(0) maxInterval:@(10)];
params.resubscribeAutomatically = NO;
[device subscribeToAttributesWithEndpointID:@10000
clusterID:@6
Expand Down Expand Up @@ -1039,7 +1039,7 @@ - (void)test010_SubscribeWithNoParams

// Subscribe
XCTestExpectation * subscribeExpectation = [self expectationWithDescription:@"subscribe OnOff attribute"];
__auto_type * params = [[MTRSubscribeParams alloc] initWithMinInterval:@(2) maxInterval:@(10)];
__auto_type * params = [[MTRSubscribeParams alloc] initWithMinInterval:@(0) maxInterval:@(10)];
[device subscribeToAttributesWithEndpointID:@1
clusterID:@6
attributeID:@0
Expand All @@ -1064,7 +1064,7 @@ - (void)test010_SubscribeWithNoParams

// Setup 2nd subscriber
subscribeExpectation = [self expectationWithDescription:@"subscribe CurrentLevel attribute"];
params = [[MTRSubscribeParams alloc] initWithMinInterval:@(2) maxInterval:@(10)];
params = [[MTRSubscribeParams alloc] initWithMinInterval:@(0) maxInterval:@(10)];
[device subscribeToAttributesWithEndpointID:@1
clusterID:@8
attributeID:@0
Expand Down Expand Up @@ -1212,7 +1212,7 @@ - (void)test011_SubscribeWithParams

// Subscribe
XCTestExpectation * subscribeExpectation = [self expectationWithDescription:@"subscribe OnOff attribute"];
__auto_type * params = [[MTRSubscribeParams alloc] initWithMinInterval:@(2) maxInterval:@(10)];
__auto_type * params = [[MTRSubscribeParams alloc] initWithMinInterval:@(0) maxInterval:@(10)];
[device subscribeToAttributesWithEndpointID:@1
clusterID:@6
attributeID:@0
Expand All @@ -1236,7 +1236,7 @@ - (void)test011_SubscribeWithParams
[self waitForExpectations:@[ subscribeExpectation ] timeout:kTimeoutInSeconds];

// Setup 2nd subscriber
MTRSubscribeParams * myParams = [[MTRSubscribeParams alloc] initWithMinInterval:@(2) maxInterval:@(10)];
MTRSubscribeParams * myParams = [[MTRSubscribeParams alloc] initWithMinInterval:@(0) maxInterval:@(10)];
myParams.replaceExistingSubscriptions = YES;
subscribeExpectation = [self expectationWithDescription:@"subscribe CurrentLevel attribute"];
[device subscribeToAttributesWithEndpointID:@1
Expand Down Expand Up @@ -1390,10 +1390,31 @@ - (void)test012_SubscribeKeepingPreviousSubscription

__block void (^firstReportHandler)(id _Nullable values, NSError * _Nullable error) = nil;
__block void (^secondReportHandler)(id _Nullable values, NSError * _Nullable error) = nil;
// Depending on how this test is run (alone or after other tests), we might
// be either in the "On" or "Off" state when we start. Track that, so we
// can ensure we're in the "Off" state correctly later.
__block BOOL initialOnOffState;

XCTestExpectation * initialOnOffReportExpectation = [self expectationWithDescription:@"initial OnOff report expectation"];
firstReportHandler = ^(id _Nullable values, NSError * _Nullable error) {
XCTAssertNil(error);

XCTAssertTrue([values isKindOfClass:[NSArray class]]);
NSDictionary * result = values[0];
MTRAttributePath * path = result[@"attributePath"];
XCTAssertEqual([path.endpoint unsignedIntegerValue], 1);
XCTAssertEqual([path.cluster unsignedIntegerValue], 6);
XCTAssertEqual([path.attribute unsignedIntegerValue], 0);
XCTAssertTrue([result[@"data"] isKindOfClass:[NSDictionary class]]);
XCTAssertTrue([result[@"data"][@"type"] isEqualToString:@"Boolean"]);
initialOnOffState = [result[@"data"][@"value"] boolValue];

[initialOnOffReportExpectation fulfill];
};

// Subscribe
XCTestExpectation * subscribeExpectation = [self expectationWithDescription:@"subscribe OnOff attribute"];
__auto_type * params = [[MTRSubscribeParams alloc] initWithMinInterval:@(2) maxInterval:@(10)];
__auto_type * params = [[MTRSubscribeParams alloc] initWithMinInterval:@(0) maxInterval:@(10)];
[device subscribeToAttributesWithEndpointID:@1
clusterID:@6
attributeID:@0
Expand All @@ -1414,11 +1435,11 @@ - (void)test012_SubscribeKeepingPreviousSubscription
[subscribeExpectation fulfill];
}];

[self waitForExpectations:@[ subscribeExpectation ] timeout:kTimeoutInSeconds];
[self waitForExpectations:@[ subscribeExpectation, initialOnOffReportExpectation ] timeout:kTimeoutInSeconds];

// Setup 2nd subscriber
subscribeExpectation = [self expectationWithDescription:@"subscribe CurrentLevel attribute"];
MTRSubscribeParams * myParams = [[MTRSubscribeParams alloc] initWithMinInterval:@(2) maxInterval:@(10)];
MTRSubscribeParams * myParams = [[MTRSubscribeParams alloc] initWithMinInterval:@(0) maxInterval:@(10)];
myParams.replaceExistingSubscriptions = NO;
[device subscribeToAttributesWithEndpointID:@1
clusterID:@8
Expand All @@ -1443,7 +1464,31 @@ - (void)test012_SubscribeKeepingPreviousSubscription
// Wait till establishment
[self waitForExpectations:@[ subscribeExpectation ] timeout:kTimeoutInSeconds];

// Send command to clear attribute state
// If we were initially on, set up expectations for report that we have been
// turned off, so we make sure that comes through before we do the rest of
// the test.
XCTestExpectation * offReportExpectation;
if (initialOnOffState == YES) {
offReportExpectation = [self expectationWithDescription:@"OnOff attribute has become false."];
firstReportHandler = ^(id _Nullable values, NSError * _Nullable error) {
XCTAssertNil(error);

{
XCTAssertTrue([values isKindOfClass:[NSArray class]]);
NSDictionary * result = values[0];
MTRAttributePath * path = result[@"attributePath"];
XCTAssertEqual([path.endpoint unsignedIntegerValue], 1);
XCTAssertEqual([path.cluster unsignedIntegerValue], 6);
XCTAssertEqual([path.attribute unsignedIntegerValue], 0);
XCTAssertTrue([result[@"data"] isKindOfClass:[NSDictionary class]]);
XCTAssertTrue([result[@"data"][@"type"] isEqualToString:@"Boolean"]);
XCTAssertEqual([result[@"data"][@"value"] boolValue], NO);
}
[offReportExpectation fulfill];
};
}

// Ensure that we are in the "off" state.
XCTestExpectation * clearCommandExpectation = [self expectationWithDescription:@"Clearing command invoked"];
[device invokeCommandWithEndpointID:@1
clusterID:@6
Expand All @@ -1452,7 +1497,7 @@ - (void)test012_SubscribeKeepingPreviousSubscription
timedInvokeTimeout:nil
queue:queue
completion:^(id _Nullable values, NSError * _Nullable error) {
NSLog(@"invoke command: On values: %@, error: %@", values, error);
NSLog(@"invoke command: Off values: %@, error: %@", values, error);

XCTAssertNil(error);

Expand All @@ -1471,6 +1516,9 @@ - (void)test012_SubscribeKeepingPreviousSubscription
[clearCommandExpectation fulfill];
}];
[self waitForExpectations:@[ clearCommandExpectation ] timeout:kTimeoutInSeconds];
if (offReportExpectation) {
[self waitForExpectations:@[ offReportExpectation ] timeout:kTimeoutInSeconds];
}

// Set up expectations for report
XCTestExpectation * reportExpectation = [self expectationWithDescription:@"The 1st subscriber received OnOff attribute report"];
Expand Down Expand Up @@ -1622,7 +1670,7 @@ - (void)test013_TimedWriteAttribute
// subscribe, which should get the new value at the timeout
expectation = [self expectationWithDescription:@"Subscribed"];
__block void (^reportHandler)(id _Nullable values, NSError * _Nullable error);
__auto_type * params = [[MTRSubscribeParams alloc] initWithMinInterval:@(2) maxInterval:@(10)];
__auto_type * params = [[MTRSubscribeParams alloc] initWithMinInterval:@(0) maxInterval:@(10)];
[device subscribeToAttributesWithEndpointID:@1
clusterID:@8
attributeID:@17
Expand Down

0 comments on commit 8750c55

Please sign in to comment.