[PATCH 1/4] test-runner: fix verbose arguments as single string

[PATCH 1/4] test-runner: fix verbose arguments as single string

[James Prestwood]

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

The verbose arguments come in from the QEMU command line as a
single string. This should have been split into an array immediately
but was not. This led to issues like hostapd debug being enabled
when "-v hostapd_cli" was passed in.
---
 tools/test-runner | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/test-runner b/tools/test-runner
index 3ba49953..fba589e3 100755
--- a/tools/test-runner
+++ b/tools/test-runner
@@ -1212,6 +1212,9 @@ def run_tests():
 	if not args.debug:
 		sys.stdout = open(os.devnull, 'w')
 
+	if args.verbose != []:
+		args.verbose = args.verbose.split(',')
+
 	os.environ['PATH'] = '%s/src' % args.testhome
 	os.environ['PATH'] += ':%s/tools' % args.testhome
 	os.environ['PATH'] += ':%s/client' % args.testhome
-- 
2.26.2

[PATCH 2/4] test-runner: don't always print "verbose on for ..."

[James Prestwood]

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

This shouldn't have been a dbg print, but rather a normal print
which will only be printed when '-d' is used.
---
 tools/test-runner | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/test-runner b/tools/test-runner
index fba589e3..c0fc85ed 100755
--- a/tools/test-runner
+++ b/tools/test-runner
@@ -180,7 +180,7 @@ class Process:
 			set_stdout = False
 
 			if ctx.is_verbose(args[0]):
-				dbg("Verbose on for %s" % args[0])
+				print("Verbose on for %s" % args[0])
 				set_stdout = True
 
 			if os.path.basename(args[0]) == ctx.args.gdb:
-- 
2.26.2

[PATCH 3/4] test-runner: fix logging, verbose, and process output

[James Prestwood]

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

There were some major problems related to logging and process
output. Tests which required output from start_process would
break if used with '--log/--verbose'. This is because we relied
on 'communicate' to retrieve the process output, but Popen does
not store process output when stdout/stderr are anything other
than PIPE.

Intead, in the case of logging or outfiles, we can simply read
from the file we just wrote to.

For an explicit --verbose application we must handle things
slightly different. A keyword argument was added to Process,
'need_out' which will ensure the process output is kept
regardless of --log or --verbose.

Now a user should be able to use --log/--verbose without any
tests failing.
---
 autotests/util/hostapd.py |  2 +-
 tools/test-runner         | 24 ++++++++++++++----------
 2 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/autotests/util/hostapd.py b/autotests/util/hostapd.py
index 3f48bd57..019d23ba 100644
--- a/autotests/util/hostapd.py
+++ b/autotests/util/hostapd.py
@@ -152,7 +152,7 @@ class HostapdCLI:
         ctx.start_process(self.cmdline + ['enable'], wait=True)
 
     def list_sta(self):
-        proc = ctx.start_process(self.cmdline + ['list_sta'], wait=True)
+        proc = ctx.start_process(self.cmdline + ['list_sta'], wait=True, need_out=True)
 
         if not proc.out:
             return []
diff --git a/tools/test-runner b/tools/test-runner
index c0fc85ed..3a766ef5 100755
--- a/tools/test-runner
+++ b/tools/test-runner
@@ -162,7 +162,7 @@ class Process:
 		test exits.
 	'''
 	def __init__(self, args, wait=False, multi_test=False, env=None, ctx=None, check=False,
-				outfile=None, namespace=None):
+				outfile=None, namespace=None, need_out=False):
 		self.args = args
 		self.wait = wait
 		self.name = args[0]
@@ -171,14 +171,13 @@ class Process:
 		self.stderr = subprocess.PIPE
 		self.ret = None
 		self.ctx = ctx
+		set_stdout = False
 
 		if namespace:
 			self.args = ['ip', 'netns', 'exec', namespace]
 			self.args.extend(args)
 
 		if ctx:
-			set_stdout = False
-
 			if ctx.is_verbose(args[0]):
 				print("Verbose on for %s" % args[0])
 				set_stdout = True
@@ -204,12 +203,12 @@ class Process:
 						os.chown(test_dir, int(ctx.args.log_uid), \
 								int(ctx.args.log_gid))
 
-					self.stdout = open('%s/%s' % (test_dir, args[0]), 'w')
-					self.stderr = open('%s/%s' % (test_dir, args[0]), 'w')
+					self.stdout = open('%s/%s' % (test_dir, args[0]), 'w+')
+					self.stderr = open('%s/%s' % (test_dir, args[0]), 'w+')
 				elif outfile:
 					self.stdout = open(outfile, 'w')
 					self.stderr = open(outfile, 'w')
-				else:
+				elif not need_out:
 					self.stdout = sys.__stdout__
 					self.stderr = sys.__stderr__
 
@@ -224,11 +223,16 @@ class Process:
 
 		self.pid.wait()
 
-		self.out, _ = self.pid.communicate()
-		self.ret = self.pid.returncode
+		if ctx and ctx.args.log or outfile:
+			self.stdout.seek(0)
+			self.out = self.stdout.read()
+		else:
+			self.out, _ = self.pid.communicate()
 
-		if self.out:
-			self.out = self.out.decode('utf-8')
+			if self.out:
+				self.out = self.out.decode('utf-8')
+
+		self.ret = self.pid.returncode
 
 		print("%s returned %d" % (args[0], self.ret))
 		if check and self.ret != 0:
-- 
2.26.2

[PATCH 4/4] station: fix improper state transition

[James Prestwood]

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

At some point the non-interactive client tests began failing.
This was due to a bug in station where it would transition from
'connected' to 'autoconnect' due to a failed scan request. The
scan request was started prior to the connection, but the work
queue delayed it and tries triggering it after we are connected.
The kernel gives back an error, which results in station
transitioning to autoconnect_full because the triggered callback
(station_quick_scan_triggered) assumed station was in a non
connected state. The sequence can be seen here:

src/station.c:station_connect_cb() 13, result: 0
src/station.c:station_enter_state() Old State: connecting, new state: connected
src/wiphy.c:wiphy_radio_work_done() Work item 6 done
src/wiphy.c:wiphy_radio_work_next() Starting work item 5
src/station.c:station_quick_scan_triggered() Quick scan trigger failed: -95
src/station.c:station_enter_state() Old State: connected, new state: autoconnect_full
---
 src/station.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/station.c b/src/station.c
index b600f37e..e7f78f2b 100644
--- a/src/station.c
+++ b/src/station.c
@@ -1113,6 +1113,9 @@ static void station_quick_scan_triggered(int err, void *user_data)
 	if (err < 0) {
 		l_debug("Quick scan trigger failed: %i", err);
 
+		if (station_is_busy(station))
+			return;
+
 		station_enter_state(station, STATION_STATE_AUTOCONNECT_FULL);
 
 		return;
-- 
2.26.2

Re: [PATCH 1/4] test-runner: fix verbose arguments as single string

[Denis Kenzior]

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

Hi James,

On 1/26/21 11:46 AM, James Prestwood wrote:
> The verbose arguments come in from the QEMU command line as a
> single string. This should have been split into an array immediately
> but was not. This led to issues like hostapd debug being enabled
> when "-v hostapd_cli" was passed in.
> ---
>   tools/test-runner | 3 +++
>   1 file changed, 3 insertions(+)
> 

Patches 1-3 applied, thanks.

Regards,
-Denis