This will wait for the process and cleanup ping-out file --- autotests/util/testutil.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/autotests/util/testutil.py b/autotests/util/testutil.py index f50223b1..c649b387 100644 --- a/autotests/util/testutil.py +++ b/autotests/util/testutil.py @@ -164,7 +164,7 @@ def test_ip_connected(tup0, tup1): ip1, ns1 = tup1 try: - ns0.start_process(['ping', '-c', '5', '-i', '0.2', ip1], check=True) - ns1.start_process(['ping', '-c', '5', '-i', '0.2', ip0], check=True) + ns0.start_process(['ping', '-c', '5', '-i', '0.2', ip1], wait=True, check=True) + ns1.start_process(['ping', '-c', '5', '-i', '0.2', ip0], wait=True, check=True) except: raise Exception('Could not ping between %s and %s' % (ip0, ip1)) -- 2.26.2

The cls object is part of the unittest framework and its lifespan is out of test-runner's control. Setting objects into the cls object sometimes keeps those objects around longer than desired. Its best to unset anything set in cls when the test is tore down. --- autotests/testEAP-WPS-Frag/wps_frag_test.py | 1 + autotests/testEAP-WPS/four_digit_pin_test.py | 1 + autotests/testEAP-WPS/pin_test.py | 1 + autotests/testEAP-WPS/push_button_test.py | 1 + autotests/testFT-8021x-roam/connection_test.py | 2 ++ autotests/testFT-FILS-SHA256/connection_test.py | 2 ++ autotests/testFT-FILS-SHA384/connection_test.py | 3 +++ autotests/testFT-PSK-over-DS/connection_test.py | 2 ++ autotests/testFT-PSK-roam/connection_test.py | 2 ++ autotests/testFT-SAE-roam/connection_test.py | 2 ++ 10 files changed, 17 insertions(+) diff --git a/autotests/testEAP-WPS-Frag/wps_frag_test.py b/autotests/testEAP-WPS-Frag/wps_frag_test.py index b9be0159..dcaf1ae2 100644 --- a/autotests/testEAP-WPS-Frag/wps_frag_test.py +++ b/autotests/testEAP-WPS-Frag/wps_frag_test.py @@ -38,6 +38,7 @@ class Test(unittest.TestCase): @classmethod def tearDownClass(cls): IWD.clear_storage() + cls.hostapd = None if __name__ == '__main__': unittest.main(exit=True) diff --git a/autotests/testEAP-WPS/four_digit_pin_test.py b/autotests/testEAP-WPS/four_digit_pin_test.py index 10afaa5d..7bdb57cb 100644 --- a/autotests/testEAP-WPS/four_digit_pin_test.py +++ b/autotests/testEAP-WPS/four_digit_pin_test.py @@ -43,6 +43,7 @@ class Test(unittest.TestCase): @classmethod def tearDownClass(cls): IWD.clear_storage() + cls.hostapd = None if __name__ == '__main__': unittest.main(exit=True) diff --git a/autotests/testEAP-WPS/pin_test.py b/autotests/testEAP-WPS/pin_test.py index 706dc20b..e2fef335 100644 --- a/autotests/testEAP-WPS/pin_test.py +++ b/autotests/testEAP-WPS/pin_test.py @@ -46,6 +46,7 @@ class Test(unittest.TestCase): @classmethod def tearDownClass(cls): IWD.clear_storage() + cls.hostapd = None if __name__ == '__main__': unittest.main(exit=True) diff --git a/autotests/testEAP-WPS/push_button_test.py b/autotests/testEAP-WPS/push_button_test.py index f82a56b5..08463f15 100644 --- a/autotests/testEAP-WPS/push_button_test.py +++ b/autotests/testEAP-WPS/push_button_test.py @@ -41,6 +41,7 @@ class Test(unittest.TestCase): @classmethod def tearDownClass(cls): IWD.clear_storage() + cls.hostapd = None if __name__ == '__main__': unittest.main(exit=True) diff --git a/autotests/testFT-8021x-roam/connection_test.py b/autotests/testFT-8021x-roam/connection_test.py index c50c90ee..f83c430d 100644 --- a/autotests/testFT-8021x-roam/connection_test.py +++ b/autotests/testFT-8021x-roam/connection_test.py @@ -143,6 +143,8 @@ class Test(unittest.TestCase): @classmethod def tearDownClass(cls): IWD.clear_storage() + cls.bss_hostapd = None + cls.bss_radio = None if __name__ == '__main__': unittest.main(exit=True) diff --git a/autotests/testFT-FILS-SHA256/connection_test.py b/autotests/testFT-FILS-SHA256/connection_test.py index 81201e08..c9e8ac7d 100644 --- a/autotests/testFT-FILS-SHA256/connection_test.py +++ b/autotests/testFT-FILS-SHA256/connection_test.py @@ -174,6 +174,8 @@ class Test(unittest.TestCase): @classmethod def tearDownClass(cls): IWD.clear_storage() + cls.bss_hostapd = None + cls.bss_radio = None if __name__ == '__main__': unittest.main(exit=True) diff --git a/autotests/testFT-FILS-SHA384/connection_test.py b/autotests/testFT-FILS-SHA384/connection_test.py index 81201e08..0495d77e 100644 --- a/autotests/testFT-FILS-SHA384/connection_test.py +++ b/autotests/testFT-FILS-SHA384/connection_test.py @@ -174,6 +174,9 @@ class Test(unittest.TestCase): @classmethod def tearDownClass(cls): IWD.clear_storage() + cls.bss_hostapd = None + cls.bss_radio = None + if __name__ == '__main__': unittest.main(exit=True) diff --git a/autotests/testFT-PSK-over-DS/connection_test.py b/autotests/testFT-PSK-over-DS/connection_test.py index ad186d07..4c751841 100644 --- a/autotests/testFT-PSK-over-DS/connection_test.py +++ b/autotests/testFT-PSK-over-DS/connection_test.py @@ -147,6 +147,8 @@ class Test(unittest.TestCase): @classmethod def tearDownClass(cls): IWD.clear_storage() + cls.bss_hostapd = None + cls.bss_radio = None if __name__ == '__main__': unittest.main(exit=True) diff --git a/autotests/testFT-PSK-roam/connection_test.py b/autotests/testFT-PSK-roam/connection_test.py index c0eec0ae..37c6da18 100644 --- a/autotests/testFT-PSK-roam/connection_test.py +++ b/autotests/testFT-PSK-roam/connection_test.py @@ -140,6 +140,8 @@ class Test(unittest.TestCase): @classmethod def tearDownClass(cls): IWD.clear_storage() + cls.bss_hostapd = None + cls.bss_radio = None if __name__ == '__main__': unittest.main(exit=True) diff --git a/autotests/testFT-SAE-roam/connection_test.py b/autotests/testFT-SAE-roam/connection_test.py index f2c694dd..da1ef731 100644 --- a/autotests/testFT-SAE-roam/connection_test.py +++ b/autotests/testFT-SAE-roam/connection_test.py @@ -197,6 +197,8 @@ class Test(unittest.TestCase): @classmethod def tearDownClass(cls): IWD.clear_storage() + cls.bss_hostapd = None + cls.bss_radio = None if __name__ == '__main__': unittest.main(exit=True) -- 2.26.2

Though multi-test processes seemed like a good idea in terms of efficiency, the additional code/special cases was not worth it for the only two multi-test processes (dbus/haveged). Intead this concept was removed completely and TestContext/Namespaces will now start all processes for each individual test. This also is fair to all tests as a previous failed test could end up bleeding into future tests. --- tools/test-runner | 32 +++++++++++--------------------- 1 file changed, 11 insertions(+), 21 deletions(-) diff --git a/tools/test-runner b/tools/test-runner index 662931b1..d63faf8c 100755 --- a/tools/test-runner +++ b/tools/test-runner @@ -157,16 +157,13 @@ class Process: ''' Start a process. If 'wait' is True the constructor will start the process and wait for it to exit. No PID is tracked in this - case. If 'multi_test' is True this indicates the process is - run over the entire test run and will not be killed after each - test exits. + case. ''' - def __init__(self, args, wait=False, multi_test=False, env=None, ctx=None, check=False, + def __init__(self, args, wait=False, env=None, ctx=None, check=False, outfile=None, namespace=None, need_out=False): self.args = args self.wait = wait self.name = args[0] - self.multi_test = multi_test self.ret = None self.ctx = ctx self.write_fds = [] @@ -311,9 +308,7 @@ class Process: raise Exception("Timed out waiting for socket") def __str__(self): - ret = str(self.args) + ' multi_test=%s' % str(self.multi_test) - ret += '\n' - return ret + return str(self.args) + '\n' class Interface: def __init__(self, name, config): @@ -528,7 +523,6 @@ class Namespace: dbus_address = None processes = [] radios = [] - dbus_pid = None def __init__(self, args, name, radios): self.name = name @@ -539,7 +533,7 @@ class Namespace: for r in radios: Process(['iw', 'phy', r.name, 'set', 'netns', 'name', name], wait=True) - self.dbus_pid = self.start_dbus(multi_test=False) + self.start_dbus() def reset(self): self.radios = [] @@ -548,12 +542,11 @@ class Namespace: if self.name == "root": self._bus = dbus.bus.BusConnection(address_or_type=self.dbus_address) - for p in [p for p in self.processes if p.multi_test is False]: + for p in self.processes: print("Killing process %s" % p.name) - self.stop_process(p) + p.kill() - if self.dbus_pid and not self.dbus_pid.multi_test: - os.remove(self.dbus_cfg) + self.processes = [] def __del__(self): print("Removing namespace %s" % self.name) @@ -607,14 +600,12 @@ class Namespace: f.write('</busconfig>\n') p = self.start_process(['dbus-daemon', '--config-file=%s' % self.dbus_cfg], - wait=False, multi_test=multi_test) + wait=False) p.wait_for_socket(self.dbus_address.split('=')[1], wait=5) self._bus = dbus.bus.BusConnection(address_or_type=self.dbus_address) - return p - def start_iwd(self, config_dir = '/tmp', storage_dir = '/tmp/iwd'): args = [] iwd_radios = ','.join([r.name for r in self.radios if r.use == 'iwd']) @@ -726,7 +717,7 @@ class TestContext(Namespace): self.start_process(['dbus-monitor', '--address', self.dbus_address]) def start_haveged(self): - self.start_process(['haveged'], multi_test=True) + self.start_process(['haveged']) def create_radios(self): setup = self.hw_config['SETUP'] @@ -1085,6 +1076,8 @@ def pre_test(ctx, test, copied): subtests = pruned + ctx.start_dbus() + ctx.start_haveged() ctx.start_dbus_monitor() ctx.start_radios() ctx.create_namespaces() @@ -1177,9 +1170,6 @@ def print_results(results): def run_auto_tests(ctx, args): tests = build_test_list(args) - ctx.start_dbus(multi_test=True) - ctx.start_haveged() - # Copy autotests/misc/{certs,secrets,phonesim} so any test can refer to them shutil.copytree(args.testhome + '/autotests/misc/certs', '/tmp/certs') shutil.copytree(args.testhome + '/autotests/misc/secrets', '/tmp/secrets') -- 2.26.2

This is somewhat of an open issue/TODO but for now this avoids the exception caused by trying to remove a radio that has been moved to a namespace. Once a radio is moved hwsim loses that phy and can no longer interact with it. This causes the Destroy() method call to fail. --- tools/test-runner | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tools/test-runner b/tools/test-runner index 2fb7a613..62c4bd75 100755 --- a/tools/test-runner +++ b/tools/test-runner @@ -395,7 +395,12 @@ class VirtualRadio(Radio): def __del__(self): super().__del__() - self._radio.remove() + # If the radio was moved into a namespace this will fail + try: + self._radio.remove() + except: + pass + self._radio = None def __str__(self): -- 2.26.2

Use a cleanup routine to remove the dbus_address/dbus_cfg files --- tools/test-runner | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/tools/test-runner b/tools/test-runner index f6d7784d..fccb6233 100755 --- a/tools/test-runner +++ b/tools/test-runner @@ -615,7 +615,15 @@ class Namespace: return True return False - def start_dbus(self, multi_test=True): + def _cleanup_dbus(self): + try: + os.remove(self.dbus_address.split('=')[1]) + except: + pass + + os.remove(self.dbus_cfg) + + def start_dbus(self): global dbus_count self.dbus_address = 'unix:path=/tmp/dbus%d' % dbus_count @@ -628,7 +636,7 @@ class Namespace: f.write('</busconfig>\n') p = self.start_process(['dbus-daemon', '--config-file=%s' % self.dbus_cfg], - wait=False) + wait=False, cleanup=self._cleanup_dbus) p.wait_for_socket(self.dbus_address.split('=')[1], wait=5) -- 2.26.2

This moves all the de-init code into kill(), which fixes a few reference issues causing processes to hang around longer than desired. If the process terminates on its own and/or the last reference is lost __del__ will kill the process and clean up. --- tools/test-runner | 48 +++++++++++++++++++++++++++++++---------------- 1 file changed, 32 insertions(+), 16 deletions(-) diff --git a/tools/test-runner b/tools/test-runner index 80bdae30..ae02578e 100755 --- a/tools/test-runner +++ b/tools/test-runner @@ -161,6 +161,7 @@ class Process: ''' def __init__(self, args, wait=False, env=None, ctx=None, check=False, outfile=None, namespace=None, need_out=False, cleanup=None): + self.killed = False self.args = args self.wait = wait self.name = args[0] @@ -238,6 +239,7 @@ class Process: return self.pid.wait() + self.killed = True self.stdout.seek(self.io_position) self.out = self.stdout.read() @@ -282,34 +284,49 @@ class Process: def __del__(self): print("Del process %s" % self.args) - self.stdout.close() - if self.io_watch: - GLib.source_remove(self.io_watch) + if not self.killed: + self.kill() - for f in self.write_fds: - f.close() - - try: - os.remove(self.output_name) - except: - pass - - if self.cleanup: - self.cleanup() + os.remove(self.output_name) def kill(self, force=False): print("Killing process %s" % self.args) + if self.killed: + return + if force: - os.kill(self.pid.pid, signal.SIGKILL) - else: self.pid.kill() + else: + self.pid.terminate() self.pid.wait(timeout=15) self.pid = None + + if self.ctx and self in self.ctx.processes: + self.ctx.processes.remove(self) + + self.ctx = None + + for f in self.write_fds: + f.close() + self.write_fds = [] + if self.io_watch: + GLib.source_remove(self.io_watch) + self.io_watch = None + + if self.cleanup: + self.cleanup() + + self.cleanup = None + + self.stdout.close() + + self.killed = True + def wait_for_socket(self, socket, wait): waited = 0 while not os.path.exists(socket): @@ -607,7 +624,6 @@ class Namespace: def stop_process(self, p, force=False): p.kill(force) - self.processes.remove(p) def is_process_running(self, process): for p in self.processes: -- 2.26.2

haveged was forking itself which then prevented test-runner from cleaning it up. --- tools/test-runner | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/test-runner b/tools/test-runner index 19272774..412998ba 100755 --- a/tools/test-runner +++ b/tools/test-runner @@ -770,7 +770,7 @@ class TestContext(Namespace): self.start_process(['dbus-monitor', '--address', self.dbus_address]) def start_haveged(self): - self.start_process(['haveged']) + self.start_process(['haveged', '-F']) def create_radios(self): setup = self.hw_config['SETUP'] -- 2.26.2

This gives the dev some inidication their test did not clean up correctly and running processes remain. --- tools/test-runner | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tools/test-runner b/tools/test-runner index 7c1498d0..eab0f0ab 100755 --- a/tools/test-runner +++ b/tools/test-runner @@ -56,6 +56,10 @@ def exit_vm(): if config: for p in config.ctx.processes: print("Process %s still running!" % p.name) + p.kill() + p = None + + config.ctx.processes = [] if config.ctx and config.ctx.results: print_results(config.ctx.results) -- 2.26.2