From: Thomas Huth <thuth@redhat.com>
To: "Cédric Le Goater" <clg@redhat.com>,
qemu-devel@nongnu.org, qemu-arm@nongnu.org
Cc: "Peter Maydell" <peter.maydell@linaro.org>,
"Steven Lee" <steven_lee@aspeedtech.com>,
"Troy Lee" <leetroy@gmail.com>,
"Jamin Lin" <jamin_lin@aspeedtech.com>,
"Andrew Jeffery" <andrew@codeconstruct.com.au>,
"Joel Stanley" <joel@jms.id.au>,
"Philippe Mathieu-Daudé" <philmd@linaro.org>
Subject: Re: [PATCH v2 1/4] tests/functional: Introduce _console_read()
Date: Tue, 12 Nov 2024 14:45:49 +0100 [thread overview]
Message-ID: <519d031b-ed11-4fed-b25b-e73dc28a9e9b@redhat.com> (raw)
In-Reply-To: <20241112130246.970281-2-clg@redhat.com>
On 12/11/2024 14.02, Cédric Le Goater wrote:
> Interaction with the console has been a problem in our avocado
> tests. In some cases, the expected string does not match in the
> output, causing the test to fail with a timeout. These were worked
> around by sleeping before reading the console and even with SSH
> connections in some places.
>
> To fix, process the console output char by char and not with
> readline. This routine was largely inspired by console_wait() in
> tests/vm/basevm.py.
>
> Signed-off-by: Cédric Le Goater <clg@redhat.com>
> ---
> tests/functional/qemu_test/cmd.py | 22 ++++++++++++++++++++--
> 1 file changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/tests/functional/qemu_test/cmd.py b/tests/functional/qemu_test/cmd.py
> index cbabb1ceed3c..5bca29d1d721 100644
> --- a/tests/functional/qemu_test/cmd.py
> +++ b/tests/functional/qemu_test/cmd.py
> @@ -12,6 +12,7 @@
> # later. See the COPYING file in the top-level directory.
>
> import logging
> +import re
> import os
> import os.path
> import subprocess
> @@ -78,6 +79,23 @@ def run_cmd(args):
> def is_readable_executable_file(path):
> return os.path.isfile(path) and os.access(path, os.R_OK | os.X_OK)
>
> +def _console_read(vm, expect):
> + console_logger = logging.getLogger('console')
> + output = ""
> + while True:
> + data = vm.console_socket.recv(1)
> + if not data:
> + break
> + output += data.decode("latin1")
> + if expect in output:
> + break
> + if "\r" in output or "\n" in output:
> + lines = re.split("[\r\n]", output)
> + if lines[0]:
> + console_logger.debug(lines[0])
> + output = lines.pop()
> + return output
> +
> def _console_interaction(test, success_message, failure_message,
> send_string, keep_sending=False, vm=None):
> assert not keep_sending or send_string
> @@ -98,12 +116,12 @@ def _console_interaction(test, success_message, failure_message,
> continue
>
> try:
> - msg = console.readline().decode().strip()
> + msg = _console_read(vm, success_message)
> except UnicodeDecodeError:
> msg = None
> if not msg:
> continue
> - console_logger.debug(msg)
> + console_logger.debug('found "%s"', msg)
Sorry for being picky (yay, bike shed painting!), but it's a little bit hard
to distinguish these "found" markers in the console.log from the lines that
just contain the normal output.
Maybe we could use some funky unicode character instead? For example:
console_logger.debug('🔎 %s', msg)
or:
console_logger.debug('✅ %s', msg)
?
Or maybe just indent the normal lines, i.e. in the _console_read() function
you could do this instead:
console_logger.debug(' %s', lines[0])
so that the "found" string is more visually separated?
Or maybe even combine the two approaches?
Thanks,
Thomas
next prev parent reply other threads:[~2024-11-12 13:46 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-11-12 13:02 [PATCH v2 0/4] tests/functional: Finish conversion of Aspeed tests Cédric Le Goater
2024-11-12 13:02 ` [PATCH v2 1/4] tests/functional: Introduce _console_read() Cédric Le Goater
2024-11-12 13:45 ` Thomas Huth [this message]
2024-11-12 13:52 ` Cédric Le Goater
2024-11-12 13:59 ` Thomas Huth
2024-11-12 14:02 ` Daniel P. Berrangé
2024-11-12 13:02 ` [PATCH v2 2/4] tests/functional: Convert Aspeed aarch64 SDK tests Cédric Le Goater
2024-11-12 13:02 ` [PATCH v2 3/4] tests/functional: Convert Aspeed arm " Cédric Le Goater
2024-11-12 13:58 ` Thomas Huth
2024-11-12 13:02 ` [PATCH v2 4/4] tests/functional: Remove sleep workarounds from Aspeed tests Cédric Le Goater
2024-11-12 13:36 ` Thomas Huth
2024-11-12 13:46 ` Cédric Le Goater
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=519d031b-ed11-4fed-b25b-e73dc28a9e9b@redhat.com \
--to=thuth@redhat.com \
--cc=andrew@codeconstruct.com.au \
--cc=clg@redhat.com \
--cc=jamin_lin@aspeedtech.com \
--cc=joel@jms.id.au \
--cc=leetroy@gmail.com \
--cc=peter.maydell@linaro.org \
--cc=philmd@linaro.org \
--cc=qemu-arm@nongnu.org \
--cc=qemu-devel@nongnu.org \
--cc=steven_lee@aspeedtech.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.