[PATCH 1/7] test-runner: fix process cleanup loop

[PATCH 1/7] test-runner: fix process cleanup loop

[James Prestwood]

[-- Attachment #1: Type: text/plain, Size: 761 bytes --]

The processes in the list ultimately get removed for each
kill() call. This causes strange behavior since the list is
being iterated and each iteration is removing items. Instead
iterate over a new temporary list so the actual process list
can be cleaned up.
---
 tools/test-runner | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/test-runner b/tools/test-runner
index fce3eed4..bbb25925 100755
--- a/tools/test-runner
+++ b/tools/test-runner
@@ -599,7 +599,7 @@ class Namespace:
 		if self.name == "root":
 			self._bus = dbus.bus.BusConnection(address_or_type=self.dbus_address)
 
-		for p in self.processes:
+		for p in list(self.processes):
 			print("Killing process %s" % p.name)
 			p.kill()
 
-- 
2.26.2

[PATCH 2/7] test-runner: add timeout for waiting for process to finish

[James Prestwood]

[-- Attachment #1: Type: text/plain, Size: 499 bytes --]

If a process hangs on exit test-runner would sit indefinitely
waiting.
---
 tools/test-runner | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/test-runner b/tools/test-runner
index bbb25925..c3a5ff89 100755
--- a/tools/test-runner
+++ b/tools/test-runner
@@ -242,7 +242,7 @@ class Process:
 		if not wait and not check:
 			return
 
-		self.pid.wait()
+		self.pid.wait(timeout=5)
 		self.killed = True
 
 		self.stdout.seek(self.io_position)
-- 
2.26.2

[PATCH 3/7] test-runner: fix process output truncation

[James Prestwood]

[-- Attachment #1: Type: text/plain, Size: 2762 bytes --]

There was a bug with process output where the last bit of data would
never make it into stdout or log files. This was due to the IO watch
being cleaned up when the process was killed and never allowing it
to finish writing any pending data.

Now the IO watch implementation has been moved out into its own
function (io_process) which is now used to write the final bits of
data out on process exit.
---
 tools/test-runner | 40 +++++++++++++++++-----------------------
 1 file changed, 17 insertions(+), 23 deletions(-)

diff --git a/tools/test-runner b/tools/test-runner
index c3a5ff89..799953de 100755
--- a/tools/test-runner
+++ b/tools/test-runner
@@ -244,31 +244,22 @@ class Process:
 
 		self.pid.wait(timeout=5)
 		self.killed = True
+		self.ret = self.pid.returncode
 
 		self.stdout.seek(self.io_position)
 		self.out = self.stdout.read()
 		self.stdout.seek(0, 2)
-		self.ret = self.pid.returncode
 
-		#
-		# No IO callback for wait/check=True processes so write out
-		# the data to any FD's now.
-		#
 		if len(self.write_fds) > 0:
-			for fd in self.write_fds:
-				fd.write(self.out)
-				fd.close()
+			self.process_io(self.stdout)
 
-			self.write_fds = []
-
-		if self.verbose:
-			sys.__stdout__.write(self.out)
+		self.write_fds = []
 
 		print("%s returned %d" % (args[0], self.ret))
 		if check and self.ret != 0:
 			raise subprocess.CalledProcessError(returncode=self.ret, cmd=self.args)
 
-	def io_callback(self, source, cb_condition):
+	def process_io(self, source):
 		#
 		# The file will have already been written to, meaning the seek
 		# position points to EOF. This is why the position is saved so
@@ -286,20 +277,27 @@ class Process:
 
 		for f in self.write_fds:
 			f.write(data)
+			f.flush()
 
 		if self.verbose:
 			sys.__stdout__.write(data)
+			sys.__stdout__.flush()
 
 		return True
 
+	def io_callback(self, source, cb_condition):
+		return self.process_io(source)
+
 	def __del__(self):
 		print("Del process %s" % self.args)
 
+		os.remove(self.output_name)
+
+		self.stdout.close()
+
 		if not self.killed:
 			self.kill()
 
-		os.remove(self.output_name)
-
 	def kill(self, force=False):
 		print("Killing process %s" % self.args)
 
@@ -319,8 +317,10 @@ class Process:
 
 		self.ctx = None
 
-		for f in self.write_fds:
-			f.close()
+		self.process_io(self.stdout)
+
+		if self.cleanup:
+			self.cleanup()
 
 		self.write_fds = []
 
@@ -328,13 +328,7 @@ class Process:
 			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):
-- 
2.26.2

[PATCH 4/7] auto-t: add more cleanup to ofono based tests (again)

[James Prestwood]

[-- Attachment #1: Type: text/plain, Size: 2970 bytes --]

The AuthCenter was still not being fully cleaned up in these
tests. It was being stopped but there was still a reference being
held which prevented __del__ from being called.
---
 autotests/testEAP-AKA-ofono/connection_test.py       |  1 +
 autotests/testEAP-AKA-prime-ofono/connection_test.py |  1 +
 autotests/testEAP-PEAP-SIM/connection_test.py        | 12 ++++++------
 autotests/testEAP-SIM-ofono/connection_test.py       |  1 +
 4 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/autotests/testEAP-AKA-ofono/connection_test.py b/autotests/testEAP-AKA-ofono/connection_test.py
index e667aa8d..7b1a8c37 100644
--- a/autotests/testEAP-AKA-ofono/connection_test.py
+++ b/autotests/testEAP-AKA-ofono/connection_test.py
@@ -61,6 +61,7 @@ class Test(unittest.TestCase):
         IWD.clear_storage()
 
         cls.auth.stop()
+        cls.auth = None
 
 if __name__ == '__main__':
     unittest.main(exit=True)
diff --git a/autotests/testEAP-AKA-prime-ofono/connection_test.py b/autotests/testEAP-AKA-prime-ofono/connection_test.py
index e6dfaee0..dc8f04bf 100644
--- a/autotests/testEAP-AKA-prime-ofono/connection_test.py
+++ b/autotests/testEAP-AKA-prime-ofono/connection_test.py
@@ -60,6 +60,7 @@ class Test(unittest.TestCase):
     def tearDownClass(cls):
         IWD.clear_storage()
         cls.auth.stop()
+        cls.auth = None
 
 if __name__ == '__main__':
     unittest.main(exit=True)
diff --git a/autotests/testEAP-PEAP-SIM/connection_test.py b/autotests/testEAP-PEAP-SIM/connection_test.py
index 37398c15..4183e65e 100644
--- a/autotests/testEAP-PEAP-SIM/connection_test.py
+++ b/autotests/testEAP-PEAP-SIM/connection_test.py
@@ -44,26 +44,26 @@ class Test(unittest.TestCase):
 
 
     def test_connection_success(self):
-        auth = AuthCenter('/tmp/hlrauc.sock', '/tmp/sim.db')
-
         ofono = Ofono()
         ofono.enable_modem('/phonesim')
         ofono.wait_for_sim_auth()
 
         wd = IWD(True)
 
-        try:
-            self.validate_connection(wd)
-        finally:
-            auth.stop()
+        self.validate_connection(wd)
 
     @classmethod
     def setUpClass(cls):
+        cls.auth = AuthCenter('/tmp/hlrauc.sock', '/tmp/sim.db')
+
         IWD.copy_to_storage('ssidEAP-PEAP-SIM.8021x')
 
     @classmethod
     def tearDownClass(cls):
         IWD.clear_storage()
 
+        cls.auth.stop()
+        cls.auth = None
+
 if __name__ == '__main__':
     unittest.main(exit=True)
diff --git a/autotests/testEAP-SIM-ofono/connection_test.py b/autotests/testEAP-SIM-ofono/connection_test.py
index e6a2d290..9f5f2734 100644
--- a/autotests/testEAP-SIM-ofono/connection_test.py
+++ b/autotests/testEAP-SIM-ofono/connection_test.py
@@ -62,6 +62,7 @@ class Test(unittest.TestCase):
         IWD.clear_storage()
 
         cls.auth.stop()
+        cls.auth = None
 
 if __name__ == '__main__':
     unittest.main(exit=True)
-- 
2.26.2

[PATCH 5/7] auto-t: properly print wait_for_object_change exception

[James Prestwood]

[-- Attachment #1: Type: text/plain, Size: 898 bytes --]

---
 autotests/util/iwd.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/autotests/util/iwd.py b/autotests/util/iwd.py
index 6cd7a1a8..e6421088 100755
--- a/autotests/util/iwd.py
+++ b/autotests/util/iwd.py
@@ -1064,7 +1064,8 @@ class IWD(AsyncOpAbstract):
                 # If neither the initial or expected condition evaluate the
                 # object must be in another unexpected state.
                 if not eval(from_str) and not eval(to_str):
-                    raise Exception('unexpected condition between [%s] and [%s]' % from_str, to_str)
+                    raise Exception('unexpected condition between [%s] and [%s]' %
+                                        (from_str, to_str))
 
                 # Initial condition does not evaluate but expected does, pass
                 if not eval(from_str) and eval(to_str):
-- 
2.26.2

[PATCH 6/7] netdev: print error number on CMD_FRAME failure

[James Prestwood]

[-- Attachment #1: Type: text/plain, Size: 737 bytes --]

---
 src/netdev.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/netdev.c b/src/netdev.c
index bd002ed9..28076244 100644
--- a/src/netdev.c
+++ b/src/netdev.c
@@ -3676,9 +3676,10 @@ static void prepare_ft(struct netdev *netdev, struct scan_bss *target_bss)
 static void netdev_ft_request_cb(struct l_genl_msg *msg, void *user_data)
 {
 	struct netdev *netdev = user_data;
+	int err = l_genl_msg_get_error(msg);
 
-	if (l_genl_msg_get_error(msg) < 0) {
-		l_error("Could not send CMD_FRAME");
+	if (err < 0) {
+		l_error("Could not send CMD_FRAME (%d)", err);
 		netdev_connect_failed(netdev,
 					NETDEV_RESULT_AUTHENTICATION_FAILED,
 					MMPDU_STATUS_CODE_UNSPECIFIED);
-- 
2.26.2

[PATCH 7/7] netdev: move prepare_ft call which broke FT

[James Prestwood]

[-- Attachment #1: Type: text/plain, Size: 1118 bytes --]

The prepare_ft patch was an intermediate to a full patch
set and was not fully tested stand alone. Its placement
actually broke FT due to handshake->aa getting overwritten
prior to netdev->prev_bssid being copied out. This caused
FT to fail with "transport endpoint not connected (-107)"
---
 src/netdev.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/netdev.c b/src/netdev.c
index 28076244..1190399d 100644
--- a/src/netdev.c
+++ b/src/netdev.c
@@ -3789,6 +3789,8 @@ static int fast_transition(struct netdev *netdev, struct scan_bss *target_bss,
 			l_get_le16(target_bss->mde))
 		return -EINVAL;
 
+	prepare_ft(netdev, target_bss);
+
 	/*
 	 * We reuse the handshake_state object and reset what's needed.
 	 * Could also create a new object and copy most of the state but
@@ -3805,8 +3807,6 @@ static int fast_transition(struct netdev *netdev, struct scan_bss *target_bss,
 							target_bss->rsne);
 	memcpy(netdev->handshake->mde + 2, target_bss->mde, 3);
 
-	prepare_ft(netdev, target_bss);
-
 	netdev->connect_cb = cb;
 
 	if (over_air)
-- 
2.26.2

Re: [PATCH 1/7] test-runner: fix process cleanup loop

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 468 bytes --]

Hi James,

On 4/22/21 12:37 PM, James Prestwood wrote:
> The processes in the list ultimately get removed for each
> kill() call. This causes strange behavior since the list is
> being iterated and each iteration is removing items. Instead
> iterate over a new temporary list so the actual process list
> can be cleaned up.
> ---
>   tools/test-runner | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 

All applied, thanks.

Regards,
-Denis