Skip to content

Commit

Permalink
Address review comment.
Browse files Browse the repository at this point in the history
Switch to just having app-starting functionality as a category on MTRTestCase.
This is much easier to use.
  • Loading branch information
bzbarsky-apple committed Sep 6, 2024
1 parent 763487d commit da8d550
Show file tree
Hide file tree
Showing 8 changed files with 68 additions and 96 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -184,11 +184,10 @@ + (void)setUp
@"MT:Y.K90SO527JA0648G00",
@"MT:-24J0AFN00I40648G00",
]) {
__auto_type * appRunner = [[MTRTestServerAppRunner alloc] initCrossTestWithAppName:@"all-clusters"
arguments:@[]
payload:payload
testsuite:self];
XCTAssertNotNil(appRunner);
BOOL started = [self startAppWithName:@"all-clusters"
arguments:@[]
payload:payload];
XCTAssertTrue(started);
}
}

Expand Down
7 changes: 4 additions & 3 deletions src/darwin/Framework/CHIPTests/MTROTAProviderTests.m
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ - (NSString *)createImageFromRawImage:(NSString *)rawImage withVersion:(NSNumber
- (MTRDevice *)commissionDeviceWithPayload:(NSString *)payloadString nodeID:(NSNumber *)nodeID;
@end

@interface MTROTARequestorAppRunner : MTRTestServerAppRunner
@interface MTROTARequestorAppRunner : NSObject
@property (nonatomic, copy) NSString * downloadFilePath;

- (instancetype)initWithPayload:(NSString *)payload testcase:(MTROTAProviderTests *)testcase;
Expand All @@ -99,14 +99,15 @@ - (MTRDevice *)commissionWithNodeID:(NSNumber *)nodeID

- (instancetype)initWithPayload:(NSString *)payload testcase:(MTROTAProviderTests *)testcase
{
__auto_type * downloadFilePath = [NSString stringWithFormat:@"/tmp/chip-ota-requestor-downloaded-image%u", [MTRTestServerAppRunner nextUniqueIndex]];
__auto_type * downloadFilePath = [NSString stringWithFormat:@"/tmp/chip-ota-requestor-downloaded-image%u", [MTROTAProviderTests nextUniqueIndex]];
__auto_type * extraArguments = @[
@"--otaDownloadPath",
downloadFilePath,
@"--autoApplyImage",
];

if (!(self = [super initWithAppName:@"ota-requestor" arguments:extraArguments payload:payload testcase:testcase])) {
BOOL started = [testcase startAppWithName:@"ota-requestor" arguments:extraArguments payload:payload];
if (!started) {
return nil;
}

Expand Down
15 changes: 7 additions & 8 deletions src/darwin/Framework/CHIPTests/MTRPairingTests.m
Original file line number Diff line number Diff line change
Expand Up @@ -186,14 +186,13 @@ - (void)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"],
]
payload:kOnboardingPayload
testcase:self];
XCTAssertNotNil(appRunner);
BOOL started = [self startAppWithName:@"all-clusters"
arguments:@[
@"--dac_provider",
[self absolutePathFor:@"credentials/development/commissioner_dut/struct_cd_origin_pid_vid_correct/test_case_vector.json"],
]
payload:kOnboardingPayload];
XCTAssertTrue(started);
}

// attestationDelegate and failSafeExtension can both be nil
Expand Down
9 changes: 4 additions & 5 deletions src/darwin/Framework/CHIPTests/MTRPerControllerStorageTests.m
Original file line number Diff line number Diff line change
Expand Up @@ -2437,11 +2437,10 @@ - (void)testSubscriptionPool
// Start our helper apps.
__auto_type * sortedKeys = [[deviceOnboardingPayloads allKeys] sortedArrayUsingSelector:@selector(compare:)];
for (NSNumber * deviceID in sortedKeys) {
__auto_type * appRunner = [[MTRTestServerAppRunner alloc] initWithAppName:@"all-clusters"
arguments:@[]
payload:deviceOnboardingPayloads[deviceID]
testcase:self];
XCTAssertNotNil(appRunner);
BOOL started = [self startAppWithName:@"all-clusters"
arguments:@[]
payload:deviceOnboardingPayloads[deviceID]];
XCTAssertTrue(started);
}

[self doTestSubscriptionPoolWithSize:1 deviceOnboardingPayloads:deviceOnboardingPayloads];
Expand Down
2 changes: 1 addition & 1 deletion src/darwin/Framework/CHIPTests/TestHelpers/MTRTestCase.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ NS_ASSUME_NONNULL_BEGIN
* Launch a cross-test task. The task will be automatically terminated when the testsuite
* tearDown happens.
*/
+ (void)launchCrossTestTask:(NSTask *)task;
+ (void)launchTask:(NSTask *)task;
#endif // HAVE_NSTASK

/**
Expand Down
21 changes: 11 additions & 10 deletions src/darwin/Framework/CHIPTests/TestHelpers/MTRTestCase.mm
Original file line number Diff line number Diff line change
Expand Up @@ -85,18 +85,18 @@ - (void)tearDown
}

#if HAVE_NSTASK
- (NSTask *)createTaskForPath:(NSString *)path
{
return [self.class createTaskForPath:path];
}

+ (NSTask *)createTaskForPath:(NSString *)path
{
NSTask * task = [[NSTask alloc] init];
[task setLaunchPath:[self absolutePathFor:path]];
return task;
}

- (NSTask *)createTaskForPath:(NSString *)path
{
return [self.class createTaskForPath:path];
}

- (void)runTask:(NSTask *)task
{
NSError * launchError;
Expand All @@ -121,14 +121,19 @@ - (void)launchTask:(NSTask *)task
[_runningTasks addObject:task];
}

+ (void)launchCrossTestTask:(NSTask *)task
+ (void)launchTask:(NSTask *)task
{
[self doLaunchTask:task];

[runningCrossTestTasks addObject:task];
}
#endif // HAVE_NSTASK

- (NSString *)absolutePathFor:(NSString *)matterRootRelativePath
{
return [self.class absolutePathFor:matterRootRelativePath];
}

+ (NSString *)absolutePathFor:(NSString *)matterRootRelativePath
{
// Start with the absolute path to our file, then remove the suffix that
Expand All @@ -140,8 +145,4 @@ + (NSString *)absolutePathFor:(NSString *)matterRootRelativePath
return [NSString pathWithComponents:pathComponents];
}

- (NSString *)absolutePathFor:(NSString *)matterRootRelativePath
{
return [self.class absolutePathFor:matterRootRelativePath];
}
@end
Original file line number Diff line number Diff line change
Expand Up @@ -16,21 +16,14 @@

#import <Foundation/Foundation.h>

@class MTRTestCase;
#import "MTRTestCase.h"

NS_ASSUME_NONNULL_BEGIN

/**
* A representation of a server application instance.
*
* Server applications are assumed to be compiled into out/debug/${APPNAME}-app,
* with the binary being out/debug/${APPNAME}-app/chip-${APPNAME}-app.
*/
@interface MTRTestServerAppRunner : NSObject
@interface MTRTestCase (ServerAppRunner)

/**
* Initialize the app runner with the given app name, arguments, setup payload, and testcase
* instance.
* Start a server app with the given app name, arguments, and setup payload.
*
* The payload will be used to determine the discriminator and passcode
* arguments the app should use, in addition to the provided arguments.
Expand All @@ -43,15 +36,13 @@ NS_ASSUME_NONNULL_BEGIN
* subtracting 1111 from the discriminator and adding 5542 (so as not to collide
* with any existing Matter things running on 5540/5541).
*/
- (instancetype)initWithAppName:(NSString *)name arguments:(NSArray<NSString *> *)arguments payload:(NSString *)payload testcase:(MTRTestCase *)testcase;
- (BOOL)startAppWithName:(NSString *)name arguments:(NSArray<NSString *> *)arguments payload:(NSString *)payload;

/**
* Same thing, but initialize as a "cross test" helper, which is not killed at
* the end of the current test (but is killed at the end of the current suite).
*
* The passed-in testsuite must be a subclass of MTRTestCase.
* Same thing, but the server will be killed at the end of the current suite,
* and is not bound to a particular test in the suite.
*/
- (instancetype)initCrossTestWithAppName:(NSString *)name arguments:(NSArray<NSString *> *)arguments payload:(NSString *)payload testsuite:(Class)testsuite;
+ (BOOL)startAppWithName:(NSString *)name arguments:(NSArray<NSString *> *)arguments payload:(NSString *)payload;

/**
* Get the unique index that will be used for the next initialization. This
Expand Down
78 changes: 30 additions & 48 deletions src/darwin/Framework/CHIPTests/TestHelpers/MTRTestServerAppRunner.m
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@

#import <Matter/Matter.h>

#import "MTRTestCase.h"
#import "MTRTestServerAppRunner.h"

static unsigned sAppRunnerIndex = 1;
Expand All @@ -29,24 +28,12 @@
static const uint16_t kBasePort = 5542 - kMinDiscriminator;
#endif // HAVE_NSTASK

@implementation MTRTestServerAppRunner {
unsigned _uniqueIndex;
#if HAVE_NSTASK
NSTask * _appTask;
#endif
}
@implementation MTRTestCase (ServerAppRunner)

- (instancetype)initInternalWithAppName:(NSString *)name arguments:(NSArray<NSString *> *)arguments payload:(NSString *)payload
#if HAVE_NSTASK
+ (NSTask *)doStartAppWithName:(NSString *)name arguments:(NSArray<NSString *> *)arguments payload:(NSString *)payload
{
#if !HAVE_NSTASK
XCTFail("Unable to start server app when we do not have NSTask");
return nil;
#else // HAVE_NSTASK
if (!(self = [super init])) {
return nil;
}

_uniqueIndex = sAppRunnerIndex++;
__auto_type uniqueIndex = sAppRunnerIndex++;

NSError * error;
__auto_type * parsedPayload = [MTRSetupPayload setupPayloadWithOnboardingPayload:payload error:&error];
Expand All @@ -61,7 +48,7 @@ - (instancetype)initInternalWithAppName:(NSString *)name arguments:(NSArray<NSSt
NSNumber * passcode = parsedPayload.setupPasscode;

__auto_type * executable = [NSString stringWithFormat:@"out/debug/%@-app/chip-%@-app", name, name];
_appTask = [MTRTestCase createTaskForPath:executable];
__auto_type * appTask = [self createTaskForPath:executable];

__auto_type * forcedArguments = @[
// Make sure we only advertise on the local interface.
Expand All @@ -74,55 +61,50 @@ - (instancetype)initInternalWithAppName:(NSString *)name arguments:(NSArray<NSSt
@"--passcode",
[NSString stringWithFormat:@"%llu", passcode.unsignedLongLongValue],
@"--KVS",
[NSString stringWithFormat:@"/tmp/chip-%@-kvs%u", name, _uniqueIndex],
[NSString stringWithFormat:@"/tmp/chip-%@-kvs%u", name, uniqueIndex],
@"--product-id",
[NSString stringWithFormat:@"%u", parsedPayload.productID.unsignedShortValue],
];

__auto_type * allArguments = [forcedArguments arrayByAddingObjectsFromArray:arguments];
[_appTask setArguments:allArguments];
[appTask setArguments:allArguments];

NSString * outFile = [NSString stringWithFormat:@"/tmp/darwin/framework-tests/%@-app-%u.log", name, _uniqueIndex];
NSString * errorFile = [NSString stringWithFormat:@"/tmp/darwin/framework-tests/%@-app-err-%u.log", name, _uniqueIndex];
NSString * outFile = [NSString stringWithFormat:@"/tmp/darwin/framework-tests/%@-app-%u.log", name, uniqueIndex];
NSString * errorFile = [NSString stringWithFormat:@"/tmp/darwin/framework-tests/%@-app-err-%u.log", name, uniqueIndex];

// Make sure the files exist.
[[NSFileManager defaultManager] createFileAtPath:outFile contents:nil attributes:nil];
[[NSFileManager defaultManager] createFileAtPath:errorFile contents:nil attributes:nil];

_appTask.standardOutput = [NSFileHandle fileHandleForWritingAtPath:outFile];
_appTask.standardError = [NSFileHandle fileHandleForWritingAtPath:errorFile];
appTask.standardOutput = [NSFileHandle fileHandleForWritingAtPath:outFile];
appTask.standardError = [NSFileHandle fileHandleForWritingAtPath:errorFile];

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

return self;
#endif // HAVE_NSTASK
return appTask;
}
#endif // HAVE_NSTASK

- (instancetype)initWithAppName:(NSString *)name arguments:(NSArray<NSString *> *)arguments payload:(NSString *)payload testcase:(MTRTestCase *)testcase
- (BOOL)startAppWithName:(NSString *)name arguments:(NSArray<NSString *> *)arguments payload:(NSString *)payload
{
if (!(self = [self initInternalWithAppName:name arguments:arguments payload:payload])) {
return nil;
}

[testcase launchTask:_appTask];

return self;
#if !HAVE_NSTASK
XCTFail("Unable to start server app when we do not have NSTask");
return NO;
#else
[self launchTask:[self.class doStartAppWithName:name arguments:arguments payload:payload]];
return YES;
#endif // HAVE_NSTASK
}

- (instancetype)initCrossTestWithAppName:(NSString *)name arguments:(NSArray<NSString *> *)arguments payload:(NSString *)payload testsuite:(Class)testsuite
+ (BOOL)startAppWithName:(NSString *)name arguments:(NSArray<NSString *> *)arguments payload:(NSString *)payload
{
if (![testsuite isSubclassOfClass:MTRTestCase.class]) {
NSLog(@"%@ is not a subclass of MTRTestCase", testsuite);
return nil;
}

if (!(self = [self initInternalWithAppName:name arguments:arguments payload:payload])) {
return nil;
}

[MTRTestCase launchCrossTestTask:_appTask];

return self;
#if !HAVE_NSTASK
XCTFail("Unable to start server app when we do not have NSTask");
return NO;
#else
[self launchTask:[self doStartAppWithName:name arguments:arguments payload:payload]];
return YES;
#endif // HAVE_NSTASK
}

+ (unsigned)nextUniqueIndex
Expand Down

0 comments on commit da8d550

Please sign in to comment.