Skip to content

Commit 867ba6c

Browse files
bzbarsky-appleshgutte
authored andcommitted
Fix Darwin CI random failures. (project-chip#35337)
* Fix Darwin CI random failures. We were starting an example app up front, then commissioning it when the test ran, but sometimes that takes longer than 15 minutes. The fix is to start example apps as needed as parts of the pairing tests that need them. The logging changes in MTRTestCase and MTRTestServerAppRunner are from trying to figure out why things were not being torn down right. That's also what the added super-calls for setUp and tearDown fix. * Adjust MTRCommissionableBrowserTests too.
1 parent 148d913 commit 867ba6c

File tree

6 files changed

+52
-29
lines changed

6 files changed

+52
-29
lines changed

.github/workflows/darwin.yaml

-1
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,6 @@ jobs:
114114
mkdir -p /tmp/darwin/framework-tests
115115
echo "This is a simple log" > /tmp/darwin/framework-tests/end_user_support_log.txt
116116
../../../out/debug/all-clusters-app/chip-all-clusters-app --interface-id -1 --end_user_support_log /tmp/darwin/framework-tests/end_user_support_log.txt > >(tee /tmp/darwin/framework-tests/all-cluster-app.log) 2> >(tee /tmp/darwin/framework-tests/all-cluster-app-err.log >&2) &
117-
../../../out/debug/all-clusters-app/chip-all-clusters-app --interface-id -1 --dac_provider ../../../credentials/development/commissioner_dut/struct_cd_origin_pid_vid_correct/test_case_vector.json --product-id 32768 --discriminator 3839 --secured-device-port 5539 --KVS /tmp/chip-all-clusters-app-kvs2 > >(tee /tmp/darwin/framework-tests/all-cluster-app-origin-vid.log) 2> >(tee /tmp/darwin/framework-tests/all-cluster-app-origin-vid-err.log >&2) &
118117
119118
export TEST_RUNNER_ASAN_OPTIONS=__CURRENT_VALUE__:detect_stack_use_after_return=1
120119

src/darwin/Framework/CHIPTests/MTRCommissionableBrowserTests.m

+5-7
Original file line numberDiff line numberDiff line change
@@ -30,12 +30,10 @@
3030

3131
static const uint16_t kLocalPort = 5541;
3232
static const uint16_t kTestVendorId = 0xFFF1u;
33-
static const uint16_t kTestProductId1 = 0x8000u;
34-
static const uint16_t kTestProductId2 = 0x8001u;
35-
static const uint16_t kTestDiscriminator1 = 3840u;
36-
static const uint16_t kTestDiscriminator2 = 3839u;
33+
static const __auto_type kTestProductIds = @[ @(0x8001u) ];
34+
static const __auto_type kTestDiscriminators = @[ @(3840u) ];
3735
static const uint16_t kDiscoverDeviceTimeoutInSeconds = 10;
38-
static const uint16_t kExpectedDiscoveredDevicesCount = 2;
36+
static const uint16_t kExpectedDiscoveredDevicesCount = 1;
3937

4038
// Singleton controller we use.
4139
static MTRDeviceController * sController = nil;
@@ -96,8 +94,8 @@ - (void)controller:(MTRDeviceController *)controller didFindCommissionableDevice
9694

9795
XCTAssertEqual(instanceName.length, 16); // The instance name is random, so just ensure the len is right.
9896
XCTAssertEqualObjects(vendorId, @(kTestVendorId));
99-
XCTAssertTrue([productId isEqual:@(kTestProductId1)] || [productId isEqual:@(kTestProductId2)]);
100-
XCTAssertTrue([discriminator isEqual:@(kTestDiscriminator1)] || [discriminator isEqual:@(kTestDiscriminator2)]);
97+
XCTAssertTrue([kTestProductIds containsObject:productId]);
98+
XCTAssertTrue([kTestDiscriminators containsObject:discriminator]);
10199
XCTAssertEqual(commissioningMode, YES);
102100

103101
NSLog(@"Found Device (%@) with discriminator: %@ (vendor: %@, product: %@)", instanceName, discriminator, vendorId, productId);

src/darwin/Framework/CHIPTests/MTRPairingTests.m

+36-14
Original file line numberDiff line numberDiff line change
@@ -19,18 +19,11 @@
1919
#import <Matter/Matter.h>
2020

2121
#import "MTRErrorTestUtils.h"
22+
#import "MTRTestCase.h"
2223
#import "MTRTestKeys.h"
23-
#import "MTRTestResetCommissioneeHelper.h"
24+
#import "MTRTestServerAppRunner.h"
2425
#import "MTRTestStorage.h"
2526

26-
// system dependencies
27-
#import <XCTest/XCTest.h>
28-
29-
// Fixture: chip-all-clusters-app --KVS "$(mktemp -t chip-test-kvs)" --interface-id -1 \
30-
--dac_provider credentials/development/commissioner_dut/struct_cd_origin_pid_vid_correct/test_case_vector.json \
31-
--product-id 32768 --discriminator 3839
32-
// For manual testing, CASE retry code paths can be tested by adding --faults chip_CASEServerBusy_f1 (or similar)
33-
3427
static const uint16_t kPairingTimeoutInSeconds = 10;
3528
static const uint16_t kTimeoutInSeconds = 3;
3629
static uint64_t sDeviceId = 100000000;
@@ -138,14 +131,16 @@ - (void)controller:(MTRDeviceController *)controller commissioningComplete:(NSEr
138131

139132
@end
140133

141-
@interface MTRPairingTests : XCTestCase
134+
@interface MTRPairingTests : MTRTestCase
142135
@property (nullable) MTRPairingTestControllerDelegate * controllerDelegate;
143136
@end
144137

145138
@implementation MTRPairingTests
146139

147140
+ (void)setUp
148141
{
142+
[super setUp];
143+
149144
__auto_type * factory = [MTRDeviceControllerFactory sharedInstance];
150145
XCTAssertNotNil(factory);
151146

@@ -170,6 +165,8 @@ + (void)tearDown
170165
sController = nil;
171166

172167
[[MTRDeviceControllerFactory sharedInstance] stopControllerFactory];
168+
169+
[super tearDown];
173170
}
174171

175172
- (void)setUp
@@ -182,11 +179,37 @@ - (void)tearDown
182179
{
183180
[sController setDeviceControllerDelegate:(id _Nonnull) nil queue:dispatch_get_main_queue()]; // TODO: do we need a clearDeviceControllerDelegate API?
184181
self.controllerDelegate = nil;
182+
183+
[super tearDown];
184+
}
185+
186+
- (void)startServerApp
187+
{
188+
// For manual testing, CASE retry code paths can be tested by adding --faults chip_CASEServerBusy_f1 (or similar)
189+
__auto_type * appRunner = [[MTRTestServerAppRunner alloc] initWithAppName:@"all-clusters"
190+
arguments:@[
191+
@"--dac_provider",
192+
[self absolutePathFor:@"credentials/development/commissioner_dut/struct_cd_origin_pid_vid_correct/test_case_vector.json"],
193+
@"--product-id",
194+
@"32768",
195+
]
196+
payload:kOnboardingPayload
197+
testcase:self];
198+
XCTAssertNotNil(appRunner);
185199
}
186200

187201
// attestationDelegate and failSafeExtension can both be nil
188202
- (void)doPairingTestWithAttestationDelegate:(id<MTRDeviceAttestationDelegate>)attestationDelegate failSafeExtension:(NSNumber *)failSafeExtension
189203
{
204+
[self doPairingTestWithAttestationDelegate:attestationDelegate failSafeExtension:failSafeExtension startServerApp:YES];
205+
}
206+
207+
- (void)doPairingTestWithAttestationDelegate:(id<MTRDeviceAttestationDelegate>)attestationDelegate failSafeExtension:(NSNumber *)failSafeExtension startServerApp:(BOOL)startServerApp
208+
{
209+
if (startServerApp) {
210+
[self startServerApp];
211+
}
212+
190213
// Don't reuse node ids, because that will confuse us.
191214
++sDeviceId;
192215
XCTestExpectation * expectation = [self expectationWithDescription:@"Commissioning Complete"];
@@ -209,9 +232,6 @@ - (void)doPairingTestWithAttestationDelegate:(id<MTRDeviceAttestationDelegate>)a
209232

210233
[self waitForExpectations:@[ expectation ] timeout:kPairingTimeoutInSeconds];
211234
XCTAssertNil(controllerDelegate.commissioningCompleteError);
212-
213-
ResetCommissionee([MTRBaseDevice deviceWithNodeID:@(sDeviceId) controller:sController], dispatch_get_main_queue(), self,
214-
kTimeoutInSeconds);
215235
}
216236

217237
- (void)test001_PairWithoutAttestationDelegate
@@ -297,6 +317,8 @@ - (void)doPairingAndWaitForProgress:(NSString *)trigger attestationDelegate:(nul
297317

298318
- (void)doPairingTestAfterCancellationAtProgress:(NSString *)trigger attestationDelegate:(nullable id<MTRDeviceAttestationDelegate>)attestationDelegate
299319
{
320+
[self startServerApp];
321+
300322
// Run pairing up and wait for the trigger
301323
[self doPairingAndWaitForProgress:trigger attestationDelegate:attestationDelegate];
302324

@@ -314,7 +336,7 @@ - (void)doPairingTestAfterCancellationAtProgress:(NSString *)trigger attestation
314336
XCTAssertEqual(error.code, MTRErrorCodeCancelled);
315337

316338
// Now pair again. If the previous attempt was cancelled correctly this should work fine.
317-
[self doPairingTestWithAttestationDelegate:nil failSafeExtension:nil];
339+
[self doPairingTestWithAttestationDelegate:nil failSafeExtension:nil startServerApp:NO];
318340
}
319341

320342
- (void)doPairingTestAfterCancellationAtProgress:(NSString *)trigger

src/darwin/Framework/CHIPTests/MTRSwiftPairingTests.swift

+5-2
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,15 @@
11
import Matter
22
import XCTest
33

4-
// This more or less parallels the "no delegate" case in MTRPairingTests
4+
// This more or less parallels the "no delegate" case in MTRPairingTests, but uses the "normal"
5+
// all-clusters-app, since it does not do any of the "interesting" VID/PID notification so far. If
6+
// it ever starts needing to do that, we should figure out a way to use MTRTestServerAppRunner from
7+
// here.
58

69
struct PairingConstants {
710
static let localPort = 5541
811
static let vendorID = 0xFFF1
9-
static let onboardingPayload = "MT:Y.K90SO527JA0648G00"
12+
static let onboardingPayload = "MT:-24J0AFN00KA0648G00"
1013
static let deviceID = 0x12344321
1114
static let timeoutInSeconds : UInt16 = 3
1215
}

src/darwin/Framework/CHIPTests/TestHelpers/MTRTestCase.mm

+5-4
Original file line numberDiff line numberDiff line change
@@ -32,23 +32,24 @@ - (void)setUp
3232
#endif // HAVE_NSTASK
3333
}
3434

35-
/**
36-
* Unfortunately, doing this in "+ (void)tearDown" (the global suite teardown)
37-
* does not trigger a test failure even if the XCTAssertEqual fails.
38-
*/
3935
- (void)tearDown
4036
{
4137
#if defined(ENABLE_LEAK_DETECTION) && ENABLE_LEAK_DETECTION
4238
if (_detectLeaks) {
4339
int pid = getpid();
4440
__auto_type * cmd = [NSString stringWithFormat:@"leaks %d", pid];
4541
int ret = system(cmd.UTF8String);
42+
/**
43+
* Unfortunately, doing this in "+ (void)tearDown" (the global suite teardown)
44+
* does not trigger a test failure even if the XCTAssertEqual fails.
45+
*/
4646
XCTAssertEqual(ret, 0, "LEAKS DETECTED");
4747
}
4848
#endif
4949

5050
#if HAVE_NSTASK
5151
for (NSTask * task in _runningTasks) {
52+
NSLog(@"Terminating task %@", task);
5253
[task terminate];
5354
}
5455
_runningTasks = nil;

src/darwin/Framework/CHIPTests/TestHelpers/MTRTestServerAppRunner.m

+1-1
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ - (instancetype)initWithAppName:(NSString *)name arguments:(NSArray<NSString *>
9292

9393
[testcase launchTask:_appTask];
9494

95-
NSLog(@"Started chip-%@-app with arguments %@ stdout=%@ and stderr=%@", name, allArguments, outFile, errorFile);
95+
NSLog(@"Started chip-%@-app (%@) with arguments %@ stdout=%@ and stderr=%@", name, _appTask, allArguments, outFile, errorFile);
9696

9797
return self;
9898
#endif // HAVE_NSTASK

0 commit comments

Comments
 (0)