Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Darwin CI random failures. #35337

Merged
merged 2 commits into from
Aug 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion .github/workflows/darwin.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,6 @@ jobs:
mkdir -p /tmp/darwin/framework-tests
echo "This is a simple log" > /tmp/darwin/framework-tests/end_user_support_log.txt
../../../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) &
../../../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) &

export TEST_RUNNER_ASAN_OPTIONS=__CURRENT_VALUE__:detect_stack_use_after_return=1

Expand Down
12 changes: 5 additions & 7 deletions src/darwin/Framework/CHIPTests/MTRCommissionableBrowserTests.m
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,10 @@

static const uint16_t kLocalPort = 5541;
static const uint16_t kTestVendorId = 0xFFF1u;
static const uint16_t kTestProductId1 = 0x8000u;
static const uint16_t kTestProductId2 = 0x8001u;
static const uint16_t kTestDiscriminator1 = 3840u;
static const uint16_t kTestDiscriminator2 = 3839u;
static const __auto_type kTestProductIds = @[ @(0x8001u) ];
static const __auto_type kTestDiscriminators = @[ @(3840u) ];
static const uint16_t kDiscoverDeviceTimeoutInSeconds = 10;
static const uint16_t kExpectedDiscoveredDevicesCount = 2;
static const uint16_t kExpectedDiscoveredDevicesCount = 1;

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

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

NSLog(@"Found Device (%@) with discriminator: %@ (vendor: %@, product: %@)", instanceName, discriminator, vendorId, productId);
Expand Down
50 changes: 36 additions & 14 deletions src/darwin/Framework/CHIPTests/MTRPairingTests.m
Original file line number Diff line number Diff line change
Expand Up @@ -19,18 +19,11 @@
#import <Matter/Matter.h>

#import "MTRErrorTestUtils.h"
#import "MTRTestCase.h"
#import "MTRTestKeys.h"
#import "MTRTestResetCommissioneeHelper.h"
#import "MTRTestServerAppRunner.h"
#import "MTRTestStorage.h"

// system dependencies
#import <XCTest/XCTest.h>

// Fixture: chip-all-clusters-app --KVS "$(mktemp -t chip-test-kvs)" --interface-id -1 \
--dac_provider credentials/development/commissioner_dut/struct_cd_origin_pid_vid_correct/test_case_vector.json \
--product-id 32768 --discriminator 3839
// For manual testing, CASE retry code paths can be tested by adding --faults chip_CASEServerBusy_f1 (or similar)

static const uint16_t kPairingTimeoutInSeconds = 10;
static const uint16_t kTimeoutInSeconds = 3;
static uint64_t sDeviceId = 100000000;
Expand Down Expand Up @@ -138,14 +131,16 @@ - (void)controller:(MTRDeviceController *)controller commissioningComplete:(NSEr

@end

@interface MTRPairingTests : XCTestCase
@interface MTRPairingTests : MTRTestCase
@property (nullable) MTRPairingTestControllerDelegate * controllerDelegate;
@end

@implementation MTRPairingTests

+ (void)setUp
{
[super setUp];

__auto_type * factory = [MTRDeviceControllerFactory sharedInstance];
XCTAssertNotNil(factory);

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

[[MTRDeviceControllerFactory sharedInstance] stopControllerFactory];

[super tearDown];
}

- (void)setUp
Expand All @@ -182,11 +179,37 @@ - (void)tearDown
{
[sController setDeviceControllerDelegate:(id _Nonnull) nil queue:dispatch_get_main_queue()]; // TODO: do we need a clearDeviceControllerDelegate API?
self.controllerDelegate = nil;

[super tearDown];
}

- (void)startServerApp
{
// For manual testing, CASE retry code paths can be tested by adding --faults chip_CASEServerBusy_f1 (or similar)
__auto_type * appRunner = [[MTRTestServerAppRunner alloc] initWithAppName:@"all-clusters"
arguments:@[
@"--dac_provider",
[self absolutePathFor:@"credentials/development/commissioner_dut/struct_cd_origin_pid_vid_correct/test_case_vector.json"],
@"--product-id",
@"32768",
]
payload:kOnboardingPayload
testcase:self];
XCTAssertNotNil(appRunner);
}

// attestationDelegate and failSafeExtension can both be nil
- (void)doPairingTestWithAttestationDelegate:(id<MTRDeviceAttestationDelegate>)attestationDelegate failSafeExtension:(NSNumber *)failSafeExtension
{
[self doPairingTestWithAttestationDelegate:attestationDelegate failSafeExtension:failSafeExtension startServerApp:YES];
}

- (void)doPairingTestWithAttestationDelegate:(id<MTRDeviceAttestationDelegate>)attestationDelegate failSafeExtension:(NSNumber *)failSafeExtension startServerApp:(BOOL)startServerApp
{
if (startServerApp) {
[self startServerApp];
}

// Don't reuse node ids, because that will confuse us.
++sDeviceId;
XCTestExpectation * expectation = [self expectationWithDescription:@"Commissioning Complete"];
Expand All @@ -209,9 +232,6 @@ - (void)doPairingTestWithAttestationDelegate:(id<MTRDeviceAttestationDelegate>)a

[self waitForExpectations:@[ expectation ] timeout:kPairingTimeoutInSeconds];
XCTAssertNil(controllerDelegate.commissioningCompleteError);

ResetCommissionee([MTRBaseDevice deviceWithNodeID:@(sDeviceId) controller:sController], dispatch_get_main_queue(), self,
kTimeoutInSeconds);
}

- (void)test001_PairWithoutAttestationDelegate
Expand Down Expand Up @@ -297,6 +317,8 @@ - (void)doPairingAndWaitForProgress:(NSString *)trigger attestationDelegate:(nul

- (void)doPairingTestAfterCancellationAtProgress:(NSString *)trigger attestationDelegate:(nullable id<MTRDeviceAttestationDelegate>)attestationDelegate
{
[self startServerApp];

// Run pairing up and wait for the trigger
[self doPairingAndWaitForProgress:trigger attestationDelegate:attestationDelegate];

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

// Now pair again. If the previous attempt was cancelled correctly this should work fine.
[self doPairingTestWithAttestationDelegate:nil failSafeExtension:nil];
[self doPairingTestWithAttestationDelegate:nil failSafeExtension:nil startServerApp:NO];
}

- (void)doPairingTestAfterCancellationAtProgress:(NSString *)trigger
Expand Down
7 changes: 5 additions & 2 deletions src/darwin/Framework/CHIPTests/MTRSwiftPairingTests.swift
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
import Matter
import XCTest

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

struct PairingConstants {
static let localPort = 5541
static let vendorID = 0xFFF1
static let onboardingPayload = "MT:Y.K90SO527JA0648G00"
static let onboardingPayload = "MT:-24J0AFN00KA0648G00"
static let deviceID = 0x12344321
static let timeoutInSeconds : UInt16 = 3
}
Expand Down
9 changes: 5 additions & 4 deletions src/darwin/Framework/CHIPTests/TestHelpers/MTRTestCase.mm
Original file line number Diff line number Diff line change
Expand Up @@ -32,23 +32,24 @@ - (void)setUp
#endif // HAVE_NSTASK
}

/**
* Unfortunately, doing this in "+ (void)tearDown" (the global suite teardown)
* does not trigger a test failure even if the XCTAssertEqual fails.
*/
- (void)tearDown
{
#if defined(ENABLE_LEAK_DETECTION) && ENABLE_LEAK_DETECTION
if (_detectLeaks) {
int pid = getpid();
__auto_type * cmd = [NSString stringWithFormat:@"leaks %d", pid];
int ret = system(cmd.UTF8String);
/**
* Unfortunately, doing this in "+ (void)tearDown" (the global suite teardown)
* does not trigger a test failure even if the XCTAssertEqual fails.
*/
XCTAssertEqual(ret, 0, "LEAKS DETECTED");
}
#endif

#if HAVE_NSTASK
for (NSTask * task in _runningTasks) {
NSLog(@"Terminating task %@", task);
[task terminate];
}
_runningTasks = nil;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ - (instancetype)initWithAppName:(NSString *)name arguments:(NSArray<NSString *>

[testcase launchTask:_appTask];

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

return self;
#endif // HAVE_NSTASK
Expand Down
Loading