From 8750c554965758892de96549b34d470ecf528fb0 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Mon, 23 Sep 2024 14:20:36 -0400 Subject: [PATCH] Stop using MinInterval = 2 in tests that expect reports within 3 seconds. (#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. --- .../Framework/CHIPTests/MTRDeviceTests.m | 4 +- .../CHIPTests/MTRXPCListenerSampleTests.m | 72 +++++++++++++++---- 2 files changed, 62 insertions(+), 14 deletions(-) diff --git a/src/darwin/Framework/CHIPTests/MTRDeviceTests.m b/src/darwin/Framework/CHIPTests/MTRDeviceTests.m index dda8986e33aaaa..1fb580488f6c51 100644 --- a/src/darwin/Framework/CHIPTests/MTRDeviceTests.m +++ b/src/darwin/Framework/CHIPTests/MTRDeviceTests.m @@ -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 @@ -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 diff --git a/src/darwin/Framework/CHIPTests/MTRXPCListenerSampleTests.m b/src/darwin/Framework/CHIPTests/MTRXPCListenerSampleTests.m index 78a26dba56e880..f16775af0f9d95 100644 --- a/src/darwin/Framework/CHIPTests/MTRXPCListenerSampleTests.m +++ b/src/darwin/Framework/CHIPTests/MTRXPCListenerSampleTests.m @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -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); @@ -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"]; @@ -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