From f1b5d9e161f071844ded18c830e3467b67dac47e Mon Sep 17 00:00:00 2001 From: Carol Yang Date: Mon, 18 Apr 2022 11:08:17 -0700 Subject: [PATCH 1/3] [CI] Allow test suite applications to be launched with custom command line options --- scripts/tests/chiptest/accessories.py | 24 ++++++++++--- scripts/tests/chiptest/test_definition.py | 35 +++++++++---------- .../suites/commands/system/SystemCommands.cpp | 4 +-- .../suites/commands/system/scripts/Reboot.py | 4 ++- .../suites/commands/system/scripts/Start.py | 4 ++- 5 files changed, 44 insertions(+), 27 deletions(-) diff --git a/scripts/tests/chiptest/accessories.py b/scripts/tests/chiptest/accessories.py index 44ae550b2f9b91..3ef75dd8e3dd8d 100644 --- a/scripts/tests/chiptest/accessories.py +++ b/scripts/tests/chiptest/accessories.py @@ -57,10 +57,13 @@ def killAll(self): for accessory in self.__accessories.values(): accessory.kill() - def start(self, name, discriminator): + def start(self, name, args): accessory = self.__accessories[name] if accessory: - return accessory.start(discriminator) + # The args param comes directly from the sys.argv[1:] Start.py and should contain a list of strings in + # key-value pair, e.g. [option1, value1, option2, value2, ...] + commandLineOptions = self.__createCommandLineOptions(args) + return accessory.start(commandLineOptions) return False def stop(self, name): @@ -69,10 +72,13 @@ def stop(self, name): return accessory.stop() return False - def reboot(self, name, discriminator): + def reboot(self, name, args): accessory = self.__accessories[name] if accessory: - return accessory.stop() and accessory.start(discriminator) + # The args param comes directly from the sys.argv[1:] Reboot.py and should contain a list of strings in + # key-value pair, e.g. [option1, value1, option2, value2, ...] + commandLineOptions = self.__createCommandLineOptions(args) + return accessory.stop() and accessory.start(commandLineOptions) return False def factoryResetAll(self): @@ -116,3 +122,13 @@ def __startXMLRPCServer(self): def __stopXMLRPCServer(self): self.server.shutdown() + + def __createCommandLineOptions(self, args): + # args should contain a list of strings in key-value pair, e.g. [option1, value1, option2, value2, ...] + commandLineOptions = {} + i = 0 + while (i + 1) < len(args): + commandLineOptions[args[i]] = args[i + 1] + i += 2 + + return commandLineOptions diff --git a/scripts/tests/chiptest/test_definition.py b/scripts/tests/chiptest/test_definition.py index ca2c4e58bab172..71b5d4e1f25260 100644 --- a/scripts/tests/chiptest/test_definition.py +++ b/scripts/tests/chiptest/test_definition.py @@ -38,12 +38,12 @@ def __init__(self, runner, command): self.stopped = False self.lastLogIndex = 0 - def start(self, discriminator): + def start(self, commandLineArgs): if not self.process: # Make sure to assign self.process before we do any operations that # might fail, so attempts to kill us on failure actually work. self.process, self.outpipe, errpipe = self.__startServer( - self.runner, self.command, discriminator) + self.runner, self.command, commandLineArgs) self.waitForAnyAdvertisement() self.__updateSetUpCode() with self.cv_stopped: @@ -64,17 +64,11 @@ def stop(self): return True return False - def reboot(self, discriminator): - if self.process: - self.stop() - self.start(discriminator) - return True - return False - def factoryReset(self): - storage = '/tmp/chip_kvs' - if os.path.exists(storage): - os.unlink(storage) + tempFiles = os.listdir("/tmp") + for file in tempFiles: + if not os.path.isdir(file) and "chip_kvs" in file: + os.unlink("/tmp/" + file) return True @@ -107,12 +101,13 @@ def wait(self, timeout=None): while self.stopped: self.cv_stopped.wait() - def __startServer(self, runner, command, discriminator): - logging.debug( - 'Executing application under test with discriminator %s.' % - discriminator) - app_cmd = command + ['--discriminator', str(discriminator)] - app_cmd = app_cmd + ['--interface-id', str(-1)] + def __startServer(self, runner, command, commandLineOptions): + app_cmd = command + ['--interface-id', str(-1)] + + logging.debug('Executing application under test with the following args:') + for option, value in commandLineOptions.items(): + logging.debug(' %s: %s' % (option, value)) + app_cmd = app_cmd + [option, value] return runner.RunSubprocess(app_cmd, name='APP ', wait=False) def __waitFor(self, waitForString, server_process, outpipe): @@ -238,7 +233,9 @@ def Run(self, runner, apps_register, paths: ApplicationPaths, pics_file: str): # Remove server application storage (factory reset), # so it will be commissionable again. app.factoryReset() - app.start(str(randrange(1, 4096))) + # Create dictionary for command line options for starting the App + commandLineOptions = {"--discriminator": str(randrange(1, 4096))} + app.start(commandLineOptions) runner.RunSubprocess( tool_cmd + ['pairing', 'qrcode', TEST_NODE_ID, app.setupCode] + diff --git a/src/app/tests/suites/commands/system/SystemCommands.cpp b/src/app/tests/suites/commands/system/SystemCommands.cpp index c6885b6199b18f..5b25be836b2c63 100644 --- a/src/app/tests/suites/commands/system/SystemCommands.cpp +++ b/src/app/tests/suites/commands/system/SystemCommands.cpp @@ -32,7 +32,7 @@ CHIP_ERROR SystemCommands::Start(uint16_t discriminator) constexpr const char * scriptName = "Start.py"; char command[128]; - VerifyOrReturnError(snprintf(command, sizeof(command), "%s%s %u", scriptDir, scriptName, discriminator) >= 0, + VerifyOrReturnError(snprintf(command, sizeof(command), "%s%s --discriminator %u", scriptDir, scriptName, discriminator) >= 0, CHIP_ERROR_INTERNAL); return RunInternal(command); } @@ -53,7 +53,7 @@ CHIP_ERROR SystemCommands::Reboot(uint16_t discriminator) constexpr const char * scriptName = "Reboot.py"; char command[128]; - VerifyOrReturnError(snprintf(command, sizeof(command), "%s%s %u", scriptDir, scriptName, discriminator) >= 0, + VerifyOrReturnError(snprintf(command, sizeof(command), "%s%s --discriminator %u", scriptDir, scriptName, discriminator) >= 0, CHIP_ERROR_INTERNAL); return RunInternal(command); } diff --git a/src/app/tests/suites/commands/system/scripts/Reboot.py b/src/app/tests/suites/commands/system/scripts/Reboot.py index eb8414a5ea6827..d6baf3737c43de 100755 --- a/src/app/tests/suites/commands/system/scripts/Reboot.py +++ b/src/app/tests/suites/commands/system/scripts/Reboot.py @@ -23,5 +23,7 @@ if sys.platform == 'linux': IP = '10.10.10.5' +# Passing in sys.argv[1:] gets rid of the script name with the remaining values in the list as +# key-value pairs, e.g. [option1, value1, option2, value2, ...] with xmlrpc.client.ServerProxy('http://' + IP + ':' + str(PORT) + '/', allow_none=True) as proxy: - proxy.reboot('default', sys.argv[1]) + proxy.reboot('default', sys.argv[1:]) diff --git a/src/app/tests/suites/commands/system/scripts/Start.py b/src/app/tests/suites/commands/system/scripts/Start.py index ae340f1ca5691e..38e4f992c67bdc 100755 --- a/src/app/tests/suites/commands/system/scripts/Start.py +++ b/src/app/tests/suites/commands/system/scripts/Start.py @@ -23,5 +23,7 @@ if sys.platform == 'linux': IP = '10.10.10.5' +# Passing in sys.argv[1:] gets rid of the script name with the remaining values in the list as +# key-value pairs, e.g. [option1, value1, option2, value2, ...] with xmlrpc.client.ServerProxy('http://' + IP + ':' + str(PORT) + '/', allow_none=True) as proxy: - proxy.start('default', sys.argv[1]) + proxy.start('default', sys.argv[1:]) From 0d7626f7fbe77a267a3f15dce4116ea336b083d4 Mon Sep 17 00:00:00 2001 From: Carol Yang Date: Tue, 19 Apr 2022 11:30:15 -0700 Subject: [PATCH 2/3] Address code review comments - Only delete the storage file used to launch the application - Do not pass in discriminator when initially launch application --- scripts/tests/chiptest/accessories.py | 24 ++++++++++----------- scripts/tests/chiptest/test_definition.py | 26 +++++++++++------------ 2 files changed, 23 insertions(+), 27 deletions(-) diff --git a/scripts/tests/chiptest/accessories.py b/scripts/tests/chiptest/accessories.py index 3ef75dd8e3dd8d..ddb3e7aa496b33 100644 --- a/scripts/tests/chiptest/accessories.py +++ b/scripts/tests/chiptest/accessories.py @@ -60,10 +60,10 @@ def killAll(self): def start(self, name, args): accessory = self.__accessories[name] if accessory: - # The args param comes directly from the sys.argv[1:] Start.py and should contain a list of strings in + # The args param comes directly from the sys.argv[1:] of Start.py and should contain a list of strings in # key-value pair, e.g. [option1, value1, option2, value2, ...] - commandLineOptions = self.__createCommandLineOptions(args) - return accessory.start(commandLineOptions) + options = self.__createCommandLineOptions(args) + return accessory.start(options) return False def stop(self, name): @@ -75,10 +75,10 @@ def stop(self, name): def reboot(self, name, args): accessory = self.__accessories[name] if accessory: - # The args param comes directly from the sys.argv[1:] Reboot.py and should contain a list of strings in + # The args param comes directly from the sys.argv[1:] of Reboot.py and should contain a list of strings in # key-value pair, e.g. [option1, value1, option2, value2, ...] - commandLineOptions = self.__createCommandLineOptions(args) - return accessory.stop() and accessory.start(commandLineOptions) + options = self.__createCommandLineOptions(args) + return accessory.stop() and accessory.start(options) return False def factoryResetAll(self): @@ -125,10 +125,8 @@ def __stopXMLRPCServer(self): def __createCommandLineOptions(self, args): # args should contain a list of strings in key-value pair, e.g. [option1, value1, option2, value2, ...] - commandLineOptions = {} - i = 0 - while (i + 1) < len(args): - commandLineOptions[args[i]] = args[i + 1] - i += 2 - - return commandLineOptions + options = {} + if (len(args) % 2) == 0: + # Create a dictionary from the key-value pair list + options = { args[i]: args[i+1] for i in range(0, len(args), 2)} + return options diff --git a/scripts/tests/chiptest/test_definition.py b/scripts/tests/chiptest/test_definition.py index 71b5d4e1f25260..abf938a0bc858c 100644 --- a/scripts/tests/chiptest/test_definition.py +++ b/scripts/tests/chiptest/test_definition.py @@ -37,13 +37,14 @@ def __init__(self, runner, command): self.cv_stopped = threading.Condition() self.stopped = False self.lastLogIndex = 0 + self.kvs = '/tmp/chip_kvs' - def start(self, commandLineArgs): + def start(self, options): if not self.process: # Make sure to assign self.process before we do any operations that # might fail, so attempts to kill us on failure actually work. self.process, self.outpipe, errpipe = self.__startServer( - self.runner, self.command, commandLineArgs) + self.runner, self.command, options) self.waitForAnyAdvertisement() self.__updateSetUpCode() with self.cv_stopped: @@ -65,11 +66,8 @@ def stop(self): return False def factoryReset(self): - tempFiles = os.listdir("/tmp") - for file in tempFiles: - if not os.path.isdir(file) and "chip_kvs" in file: - os.unlink("/tmp/" + file) - + if os.path.exists(self.kvs): + os.unlink(self.kvs) return True def waitForAnyAdvertisement(self): @@ -101,13 +99,15 @@ def wait(self, timeout=None): while self.stopped: self.cv_stopped.wait() - def __startServer(self, runner, command, commandLineOptions): + def __startServer(self, runner, command, options): app_cmd = command + ['--interface-id', str(-1)] logging.debug('Executing application under test with the following args:') - for option, value in commandLineOptions.items(): - logging.debug(' %s: %s' % (option, value)) - app_cmd = app_cmd + [option, value] + for key, value in options.items(): + logging.debug(' %s: %s' % (key, value)) + app_cmd = app_cmd + [key, value] + if key == '--KVS': + self.kvs = value return runner.RunSubprocess(app_cmd, name='APP ', wait=False) def __waitFor(self, waitForString, server_process, outpipe): @@ -233,9 +233,7 @@ def Run(self, runner, apps_register, paths: ApplicationPaths, pics_file: str): # Remove server application storage (factory reset), # so it will be commissionable again. app.factoryReset() - # Create dictionary for command line options for starting the App - commandLineOptions = {"--discriminator": str(randrange(1, 4096))} - app.start(commandLineOptions) + app.start({}) runner.RunSubprocess( tool_cmd + ['pairing', 'qrcode', TEST_NODE_ID, app.setupCode] + From c5a170c8e967500e90afb07c966a55f35145ed01 Mon Sep 17 00:00:00 2001 From: "Restyled.io" Date: Tue, 19 Apr 2022 18:33:55 +0000 Subject: [PATCH 3/3] Restyled by autopep8 --- scripts/tests/chiptest/accessories.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/tests/chiptest/accessories.py b/scripts/tests/chiptest/accessories.py index ddb3e7aa496b33..6e9ce9a1a26022 100644 --- a/scripts/tests/chiptest/accessories.py +++ b/scripts/tests/chiptest/accessories.py @@ -128,5 +128,5 @@ def __createCommandLineOptions(self, args): options = {} if (len(args) % 2) == 0: # Create a dictionary from the key-value pair list - options = { args[i]: args[i+1] for i in range(0, len(args), 2)} + options = {args[i]: args[i+1] for i in range(0, len(args), 2)} return options