Make it easier to start "cross-test" helper apps in Darwin tests. (#3…

…5435) * Make it easier to start "cross-test" helper apps in Darwin tests. We shouldn't require a testcase instance to start such an app, since it's associated with the test suite, not a specific test. * Address review comment. Switch to just having app-starting functionality as a category on MTRTestCase. This is much easier to use. * Rename MTRTestServerAppRunner to MTRTestCase+ServerAppRunner.
project-chip · Jan 14, 2025 · 1601088 · 1601088
1 parent e9a3ca7
commit 1601088
Show file tree

Hide file tree

Showing 10 changed files with 99 additions and 100 deletions.
diff --git a/src/darwin/Framework/CHIPTests/MTRCommissionableBrowserTests.m b/src/darwin/Framework/CHIPTests/MTRCommissionableBrowserTests.m
@@ -17,9 +17,9 @@
 
 #import <Matter/Matter.h>
 
+#import "MTRTestCase+ServerAppRunner.h"
 #import "MTRTestCase.h"
 #import "MTRTestKeys.h"
-#import "MTRTestServerAppRunner.h"
 #import "MTRTestStorage.h"
 
 // Fixture 1: chip-all-clusters-app --KVS "$(mktemp -t chip-test-kvs)" --interface-id -1
@@ -31,8 +31,6 @@
 static const uint16_t kDiscoverDeviceTimeoutInSeconds = 10;
 static const uint16_t kExpectedDiscoveredDevicesCount = 3;
 
-static bool sHelperAppsStarted = false;
-
 // Singleton controller we use.
 static MTRDeviceController * sController = nil;
 
@@ -180,6 +178,17 @@ + (void)setUp
     XCTAssertNotNil(controller);
 
     sController = controller;
+
+    // Start the helper apps our tests use.
+    for (NSString * payload in @[
+             @"MT:Y.K90SO527JA0648G00",
+             @"MT:-24J0AFN00I40648G00",
+         ]) {
+        BOOL started = [self startAppWithName:@"all-clusters"
+                                    arguments:@[]
+                                      payload:payload];
+        XCTAssertTrue(started);
+    }
 }
 
 + (void)tearDown
@@ -197,21 +206,6 @@ + (void)tearDown
 - (void)setUp
 {
     [super setUp];
-
-    if (!sHelperAppsStarted) {
-        for (NSString * payload in @[
-                 @"MT:Y.K90SO527JA0648G00",
-                 @"MT:-24J0AFN00I40648G00",
-             ]) {
-            __auto_type * appRunner = [[MTRTestServerAppRunner alloc] initCrossTestWithAppName:@"all-clusters"
-                                                                                     arguments:@[]
-                                                                                       payload:payload
-                                                                                      testcase:self];
-            XCTAssertNotNil(appRunner);
-        }
-        sHelperAppsStarted = true;
-    }
-
     [self setContinueAfterFailure:NO];
 }
 

diff --git a/src/darwin/Framework/CHIPTests/MTROTAProviderTests.m b/src/darwin/Framework/CHIPTests/MTROTAProviderTests.m
@@ -20,10 +20,10 @@
 
 #import "MTRDeviceTestDelegate.h"
 #import "MTRErrorTestUtils.h"
+#import "MTRTestCase+ServerAppRunner.h"
 #import "MTRTestCase.h"
 #import "MTRTestKeys.h"
 #import "MTRTestResetCommissioneeHelper.h"
-#import "MTRTestServerAppRunner.h"
 #import "MTRTestStorage.h"
 
 // system dependencies
@@ -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;
@@ -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;
     }
 

diff --git a/src/darwin/Framework/CHIPTests/MTRPairingTests.m b/src/darwin/Framework/CHIPTests/MTRPairingTests.m
@@ -19,9 +19,9 @@
 #import <Matter/Matter.h>
 
 #import "MTRErrorTestUtils.h"
+#import "MTRTestCase+ServerAppRunner.h"
 #import "MTRTestCase.h"
 #import "MTRTestKeys.h"
-#import "MTRTestServerAppRunner.h"
 #import "MTRTestStorage.h"
 
 static const uint16_t kPairingTimeoutInSeconds = 10;
@@ -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

diff --git a/src/darwin/Framework/CHIPTests/MTRPerControllerStorageTests.m b/src/darwin/Framework/CHIPTests/MTRPerControllerStorageTests.m
@@ -24,12 +24,12 @@
 #import "MTRDevice_Internal.h"
 #import "MTRErrorTestUtils.h"
 #import "MTRFabricInfoChecker.h"
+#import "MTRTestCase+ServerAppRunner.h"
 #import "MTRTestCase.h"
 #import "MTRTestDeclarations.h"
 #import "MTRTestKeys.h"
 #import "MTRTestPerControllerStorage.h"
 #import "MTRTestResetCommissioneeHelper.h"
-#import "MTRTestServerAppRunner.h"
 
 static const uint16_t kPairingTimeoutInSeconds = 10;
 static const uint16_t kTimeoutInSeconds = 3;
@@ -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];

diff --git a/src/darwin/Framework/CHIPTests/MTRSwiftPairingTests.swift b/src/darwin/Framework/CHIPTests/MTRSwiftPairingTests.swift
@@ -3,7 +3,7 @@ import XCTest
 
 // 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
+// it ever starts needing to do that, we should figure out a way to use MTRTestCase+ServerAppRunner from
 // here.
 
 struct PairingConstants {

diff --git a/...ests/TestHelpers/MTRTestServerAppRunner.h → ...TestHelpers/MTRTestCase+ServerAppRunner.h b/...ests/TestHelpers/MTRTestServerAppRunner.h → ...TestHelpers/MTRTestCase+ServerAppRunner.h
@@ -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.
@@ -43,13 +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).
+ * 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 testcase:(MTRTestCase *)testcase;
++ (BOOL)startAppWithName:(NSString *)name arguments:(NSArray<NSString *> *)arguments payload:(NSString *)payload;
 
 /**
  * Get the unique index that will be used for the next initialization.  This

diff --git a/...ests/TestHelpers/MTRTestServerAppRunner.m → ...TestHelpers/MTRTestCase+ServerAppRunner.m b/...ests/TestHelpers/MTRTestServerAppRunner.m → ...TestHelpers/MTRTestCase+ServerAppRunner.m
@@ -16,8 +16,7 @@
 
 #import <Matter/Matter.h>
 
-#import "MTRTestCase.h"
-#import "MTRTestServerAppRunner.h"
+#import "MTRTestCase+ServerAppRunner.h"
 
 static unsigned sAppRunnerIndex = 1;
 
@@ -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 testcase:(MTRTestCase *)testcase isCrossTest:(BOOL)isCrossTest
+#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];
@@ -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 = [testcase createTaskForPath:executable];
+    __auto_type * appTask = [self createTaskForPath:executable];
 
     __auto_type * forcedArguments = @[
         // Make sure we only advertise on the local interface.
@@ -74,44 +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];
 
-    if (isCrossTest) {
-        [testcase launchCrossTestTask:_appTask];
-    } else {
-        [testcase launchTask:_appTask];
-    }
+    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
 {
-    return [self initInternalWithAppName:name arguments:arguments payload:payload testcase:testcase isCrossTest:NO];
+#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 testcase:(MTRTestCase *)testcase
++ (BOOL)startAppWithName:(NSString *)name arguments:(NSArray<NSString *> *)arguments payload:(NSString *)payload
 {
-    return [self initInternalWithAppName:name arguments:arguments payload:payload testcase:testcase isCrossTest:YES];
+#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

diff --git a/src/darwin/Framework/CHIPTests/TestHelpers/MTRTestCase.h b/src/darwin/Framework/CHIPTests/TestHelpers/MTRTestCase.h
@@ -37,6 +37,11 @@ NS_ASSUME_NONNULL_BEGIN
  */
 - (NSTask *)createTaskForPath:(NSString *)path;
 
+/**
+ * Same thing, but not tied to a specific testcase instance.
+ */
++ (NSTask *)createTaskForPath:(NSString *)path;
+
 /**
  * Run a task to completion and make sure it succeeds.
  */
@@ -52,14 +57,19 @@ 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
 
 /**
  * Get an absolute path from a path relative to the Matter SDK root.
  */
 - (NSString *)absolutePathFor:(NSString *)matterRootRelativePath;
 
+/**
+ * Same thing, but not tied to a specific testcase instance.
+ */
++ (NSString *)absolutePathFor:(NSString *)matterRootRelativePath;
+
 @end
 
 NS_ASSUME_NONNULL_END