[PATCH v7 0/8] test: Minor fixes to test.py
This series collects together the patches from the Labgrid series which are not related to Labgrid, or at least can be applied independently of using Labgrid to run the lab.
Changes in v7: - Fix 'board' typo - Split out from earlier series - Rebase on -master
Changes in v4: - Expand the commit message
Simon Glass (8): test: Use a constant for the test timeout test: Avoid failing skipped tests test: Create a common function to get the config test: Move the receive code into a function test: Separate out the exception handling test: Detect dead connections test: Tidy up remaining exceptions test: Fix mulptiplex_log typo
test/py/conftest.py | 60 +++++++++++++++++------ test/py/u_boot_console_base.py | 39 ++++++++------- test/py/u_boot_spawn.py | 88 +++++++++++++++++++++++++++++----- test/test-main.c | 16 +++++-- 4 files changed, 154 insertions(+), 49 deletions(-)
Declare a constant rather than open-coding the same value twice.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Tom Rini trini@konsulko.com ---
(no changes since v1)
test/py/u_boot_console_base.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/test/py/u_boot_console_base.py b/test/py/u_boot_console_base.py index 76a550d45a1..4d6cf3f95a4 100644 --- a/test/py/u_boot_console_base.py +++ b/test/py/u_boot_console_base.py @@ -26,6 +26,9 @@ pattern_error_please_reset = re.compile('### ERROR ### Please RESET the board ## PAT_ID = 0 PAT_RE = 1
+# Timeout before expecting the console to be ready (in milliseconds) +TIMEOUT_MS = 30000 + bad_pattern_defs = ( ('spl_signon', pattern_u_boot_spl_signon), ('main_signon', pattern_u_boot_main_signon), @@ -422,7 +425,7 @@ class ConsoleBase(object): # Reset the console timeout value as some tests may change # its default value during the execution if not self.config.gdbserver: - self.p.timeout = 30000 + self.p.timeout = TIMEOUT_MS return try: self.log.start_section('Starting U-Boot') @@ -433,7 +436,7 @@ class ConsoleBase(object): # future, possibly per-test to be optimal. This works for 'help' # on board 'seaboard'. if not self.config.gdbserver: - self.p.timeout = 30000 + self.p.timeout = TIMEOUT_MS self.p.logfile_read = self.logstream if expect_reset: loop_num = 2
When a test returns -EAGAIN this should not be considered a failure. Fix what seems to be a problem case, where the pytests see a failure when a test has merely been skipped.
We cannot squash the -EAGAIN error in ut_run_test() since the failure count is incremented by its caller, ut_run_test_live_flat()
The specific example here is on snow, where a test is compiled into the image but cannot run, so returns -EAGAIN to skip:
test/py/tests/test_ut.py sssnow # ut bdinfo bdinfo_test_eth Test: bdinfo_test_eth: bdinfo.c Skipping: Console recording disabled test/test-main.c:486, ut_run_test_live_flat(): 0 == ut_run_test(uts, test, test->name): Expected 0x0 (0), got 0xfffffff5 (-11) Test bdinfo_test_eth failed 1 times Skipped: 1, Failures: 1 snow # F+u-boot-test-reset snow snow
The fix is simply to respect the return code from ut_run_test(), so do that.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v4)
Changes in v4: - Expand the commit message
test/test-main.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/test/test-main.c b/test/test-main.c index b3d3e24cdce..b613fc6a4d3 100644 --- a/test/test-main.c +++ b/test/test-main.c @@ -486,7 +486,7 @@ static int ut_run_test(struct unit_test_state *uts, struct unit_test *test, static int ut_run_test_live_flat(struct unit_test_state *uts, struct unit_test *test) { - int runs; + int runs, ret;
if ((test->flags & UTF_OTHER_FDT) && !IS_ENABLED(CONFIG_SANDBOX)) return skip_test(uts); @@ -496,8 +496,11 @@ static int ut_run_test_live_flat(struct unit_test_state *uts, if (CONFIG_IS_ENABLED(OF_LIVE)) { if (!(test->flags & UTF_FLAT_TREE)) { uts->of_live = true; - ut_assertok(ut_run_test(uts, test, test->name)); - runs++; + ret = ut_run_test(uts, test, test->name); + if (ret != -EAGAIN) { + ut_assertok(ret); + runs++; + } } }
@@ -521,8 +524,11 @@ static int ut_run_test_live_flat(struct unit_test_state *uts, (!runs || ut_test_run_on_flattree(test)) && !(gd->flags & GD_FLG_FDT_CHANGED)) { uts->of_live = false; - ut_assertok(ut_run_test(uts, test, test->name)); - runs++; + ret = ut_run_test(uts, test, test->name); + if (ret != -EAGAIN) { + ut_assertok(ret); + runs++; + } }
return 0;
The settings are decoded in two places. Combine them into a new function, before (in a future patch) expanding the number of items.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
test/py/conftest.py | 41 ++++++++++++++++++++++++++++------------- 1 file changed, 28 insertions(+), 13 deletions(-)
diff --git a/test/py/conftest.py b/test/py/conftest.py index fc9dd3a83f8..85e1979dc79 100644 --- a/test/py/conftest.py +++ b/test/py/conftest.py @@ -115,14 +115,36 @@ def run_build(config, source_dir, build_dir, board_type, log): runner.close() log.status_pass('OK')
-def pytest_xdist_setupnodes(config, specs): - """Clear out any 'done' file from a previous build""" - global build_done_file - build_dir = config.getoption('build_dir') +def get_details(config): + """Obtain salient details about the board and directories to use + + Args: + config (pytest.Config): pytest configuration + + Returns: + tuple: + str: Board type (U-Boot build name) + str: Identity for the lab board + str: Build directory + str: Source directory + """ board_type = config.getoption('board_type') + board_identity = config.getoption('board_identity') + build_dir = config.getoption('build_dir') + source_dir = os.path.dirname(os.path.dirname(TEST_PY_DIR)) + default_build_dir = source_dir + '/build-' + board_type if not build_dir: - build_dir = source_dir + '/build-' + board_type + build_dir = default_build_dir + + return board_type, board_identity, build_dir, source_dir + +def pytest_xdist_setupnodes(config, specs): + """Clear out any 'done' file from a previous build""" + global build_done_file + + build_dir = get_details(config)[2] + build_done_file = Path(build_dir) / 'build.done' if build_done_file.exists(): os.remove(build_done_file) @@ -161,17 +183,10 @@ def pytest_configure(config): global console global ubconfig
- source_dir = os.path.dirname(os.path.dirname(TEST_PY_DIR)) + board_type, board_identity, build_dir, source_dir = get_details(config)
- board_type = config.getoption('board_type') board_type_filename = board_type.replace('-', '_') - - board_identity = config.getoption('board_identity') board_identity_filename = board_identity.replace('-', '_') - - build_dir = config.getoption('build_dir') - if not build_dir: - build_dir = source_dir + '/build-' + board_type mkdir_p(build_dir)
result_dir = config.getoption('result_dir')
There is quite a bit of code to deal with receiving data from the target so move it into its own receive() function.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
test/py/u_boot_spawn.py | 39 +++++++++++++++++++++++++++------------ 1 file changed, 27 insertions(+), 12 deletions(-)
diff --git a/test/py/u_boot_spawn.py b/test/py/u_boot_spawn.py index 97e95e07c80..69a2cd55816 100644 --- a/test/py/u_boot_spawn.py +++ b/test/py/u_boot_spawn.py @@ -137,6 +137,32 @@ class Spawn:
os.write(self.fd, data.encode(errors='replace'))
+ def receive(self, num_bytes): + """Receive data from the sub-process's stdin. + + Args: + num_bytes (int): Maximum number of bytes to read + + Returns: + str: The data received + + Raises: + ValueError if U-Boot died + """ + try: + c = os.read(self.fd, num_bytes).decode(errors='replace') + except OSError as err: + # With sandbox, try to detect when U-Boot exits when it + # shouldn't and explain why. This is much more friendly than + # just dying with an I/O error + if self.decode_signal and err.errno == 5: # I/O error + alive, _, info = self.checkalive() + if alive: + raise err + raise ValueError('U-Boot exited with %s' % info) + raise + return c + def expect(self, patterns): """Wait for the sub-process to emit specific data.
@@ -193,18 +219,7 @@ class Spawn: events = self.poll.poll(poll_maxwait) if not events: raise Timeout() - try: - c = os.read(self.fd, 1024).decode(errors='replace') - except OSError as err: - # With sandbox, try to detect when U-Boot exits when it - # shouldn't and explain why. This is much more friendly than - # just dying with an I/O error - if self.decode_signal and err.errno == 5: # I/O error - alive, _, info = self.checkalive() - if alive: - raise err - raise ValueError('U-Boot exited with %s' % info) - raise + c = self.receive(1024) if self.logfile_read: self.logfile_read.write(c) self.buf += c
The tests currently catch a very broad Exception in each case. This is thrown even in the event of a coding error.
We want to handle exceptions differently depending on their severity, so that we can avoid hour-long delays waiting for a board that is clearly broken.
As a first step, create some new exception types, separating out those which are simply an unexpected result from executed a command, from those which indicate some kind of hardware failure.
Signed-off-by: Simon Glass sjg@chromium.org ---
Changes in v7: - Fix 'board' typo
test/py/u_boot_console_base.py | 26 ++++++++++++++------------ test/py/u_boot_spawn.py | 11 +++++++++++ 2 files changed, 25 insertions(+), 12 deletions(-)
diff --git a/test/py/u_boot_console_base.py b/test/py/u_boot_console_base.py index 4d6cf3f95a4..0be6c760206 100644 --- a/test/py/u_boot_console_base.py +++ b/test/py/u_boot_console_base.py @@ -14,6 +14,7 @@ import pytest import re import sys import u_boot_spawn +from u_boot_spawn import BootFail, Timeout, Unexpected
# Regexes for text we expect U-Boot to send to the console. pattern_u_boot_spl_signon = re.compile('(U-Boot SPL \d{4}\.\d{2}[^\r\n]*\))') @@ -189,13 +190,13 @@ class ConsoleBase(object): m = self.p.expect([pattern_u_boot_spl_signon] + self.bad_patterns) if m != 0: - raise Exception('Bad pattern found on SPL console: ' + + raise BootFail('Bad pattern found on SPL console: ' + self.bad_pattern_ids[m - 1]) env_spl_banner_times -= 1
m = self.p.expect([pattern_u_boot_main_signon] + self.bad_patterns) if m != 0: - raise Exception('Bad pattern found on console: ' + + raise BootFail('Bad pattern found on console: ' + self.bad_pattern_ids[m - 1]) self.u_boot_version_string = self.p.after while True: @@ -206,13 +207,9 @@ class ConsoleBase(object): if m == 1: self.p.send(' ') continue - raise Exception('Bad pattern found on console: ' + + raise BootFail('Bad pattern found on console: ' + self.bad_pattern_ids[m - 2])
- except Exception as ex: - self.log.error(str(ex)) - self.cleanup_spawn() - raise finally: self.log.timestamp()
@@ -278,7 +275,7 @@ class ConsoleBase(object): m = self.p.expect([chunk] + self.bad_patterns) if m != 0: self.at_prompt = False - raise Exception('Bad pattern found on console: ' + + raise BootFail('Bad pattern found on console: ' + self.bad_pattern_ids[m - 1]) if not wait_for_prompt: return @@ -288,14 +285,18 @@ class ConsoleBase(object): m = self.p.expect([self.prompt_compiled] + self.bad_patterns) if m != 0: self.at_prompt = False - raise Exception('Bad pattern found on console: ' + + raise BootFail('Missing prompt on console: ' + self.bad_pattern_ids[m - 1]) self.at_prompt = True self.at_prompt_logevt = self.logstream.logfile.cur_evt # Only strip \r\n; space/TAB might be significant if testing # indentation. return self.p.before.strip('\r\n') - except Exception as ex: + except Timeout as exc: + self.log.error(str(exc)) + self.cleanup_spawn() + raise + except BootFail as ex: self.log.error(str(ex)) self.cleanup_spawn() raise @@ -354,8 +355,9 @@ class ConsoleBase(object): text = re.escape(text) m = self.p.expect([text] + self.bad_patterns) if m != 0: - raise Exception('Bad pattern found on console: ' + - self.bad_pattern_ids[m - 1]) + raise Unexpected( + "Unexpected pattern found on console (exp '{text}': " + + self.bad_pattern_ids[m - 1])
def drain_console(self): """Read from and log the U-Boot console for a short time. diff --git a/test/py/u_boot_spawn.py b/test/py/u_boot_spawn.py index 69a2cd55816..bcba7b5cd43 100644 --- a/test/py/u_boot_spawn.py +++ b/test/py/u_boot_spawn.py @@ -16,6 +16,17 @@ import traceback class Timeout(Exception): """An exception sub-class that indicates that a timeout occurred."""
+class BootFail(Exception): + """An exception sub-class that indicates that a boot failure occurred. + + This is used when a bad pattern is seen when waiting for the boot prompt. + It is regarded as fatal, to avoid trying to boot the again and again to no + avail. + """ + +class Unexpected(Exception): + """An exception sub-class that indicates that unexpected test was seen.""" + class Spawn: """Represents the stdio of a freshly created sub-process. Commands may be sent to the process, and responses waited for.
When the connection to a board dies, assume it is dead forever until some user action is taken. Skip all remaining tests. This avoids CI runs taking an hour, with hundreds of 30-second timeouts all to no avail.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
test/py/conftest.py | 19 +++++++++++++++++-- test/py/u_boot_spawn.py | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 2 deletions(-)
diff --git a/test/py/conftest.py b/test/py/conftest.py index 85e1979dc79..46a410cf268 100644 --- a/test/py/conftest.py +++ b/test/py/conftest.py @@ -24,6 +24,7 @@ import pytest import re from _pytest.runner import runtestprotocol import sys +from u_boot_spawn import BootFail, Timeout, Unexpected, handle_exception
# Globals: The HTML log file, and the connection to the U-Boot console. log = None @@ -254,6 +255,7 @@ def pytest_configure(config): ubconfig.board_identity = board_identity ubconfig.gdbserver = gdbserver ubconfig.dtb = build_dir + '/arch/sandbox/dts/test.dtb' + ubconfig.connection_ok = True
env_vars = ( 'board_type', @@ -420,8 +422,21 @@ def u_boot_console(request): Returns: The fixture value. """ - - console.ensure_spawned() + if not ubconfig.connection_ok: + pytest.skip('Cannot get target connection') + return None + try: + console.ensure_spawned() + except OSError as err: + handle_exception(ubconfig, console, log, err, 'Lab failure', True) + except Timeout as err: + handle_exception(ubconfig, console, log, err, 'Lab timeout', True) + except BootFail as err: + handle_exception(ubconfig, console, log, err, 'Boot fail', True, + console.get_spawn_output()) + except Unexpected: + handle_exception(ubconfig, console, log, err, 'Unexpected test output', + False) return console
anchors = {} diff --git a/test/py/u_boot_spawn.py b/test/py/u_boot_spawn.py index bcba7b5cd43..24d369035e5 100644 --- a/test/py/u_boot_spawn.py +++ b/test/py/u_boot_spawn.py @@ -8,6 +8,7 @@ Logic to spawn a sub-process and interact with its stdio. import os import re import pty +import pytest import signal import select import time @@ -27,6 +28,43 @@ class BootFail(Exception): class Unexpected(Exception): """An exception sub-class that indicates that unexpected test was seen."""
+ +def handle_exception(ubconfig, console, log, err, name, fatal, output=''): + """Handle an exception from the console + + Exceptions can occur when there is unexpected output or due to the board + crashing or hanging. Some exceptions are likely fatal, where retrying will + just chew up time to no available. In those cases it is best to cause + further tests be skipped. + + Args: + ubconfig (ArbitraryAttributeContainer): ubconfig object + log (Logfile): Place to log errors + console (ConsoleBase): Console to clean up, if fatal + err (Exception): Exception which was thrown + name (str): Name of problem, to log + fatal (bool): True to abort all tests + output (str): Extra output to report on boot failure. This can show the + target's console output as it tried to boot + """ + msg = f'{name}: ' + if fatal: + msg += 'Marking connection bad - no other tests will run' + else: + msg += 'Assuming that lab is healthy' + print(msg) + log.error(msg) + log.error(f'Error: {err}') + + if output: + msg += f'; output {output}' + + if fatal: + ubconfig.connection_ok = False + console.cleanup_spawn() + pytest.exit(msg) + + class Spawn: """Represents the stdio of a freshly created sub-process. Commands may be sent to the process, and responses waited for.
Use the new handle_exception() function from ConsoleBase also.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
test/py/u_boot_console_base.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/test/py/u_boot_console_base.py b/test/py/u_boot_console_base.py index 0be6c760206..aebe0dac099 100644 --- a/test/py/u_boot_console_base.py +++ b/test/py/u_boot_console_base.py @@ -14,7 +14,7 @@ import pytest import re import sys import u_boot_spawn -from u_boot_spawn import BootFail, Timeout, Unexpected +from u_boot_spawn import BootFail, Timeout, Unexpected, handle_exception
# Regexes for text we expect U-Boot to send to the console. pattern_u_boot_spl_signon = re.compile('(U-Boot SPL \d{4}\.\d{2}[^\r\n]*\))') @@ -293,12 +293,12 @@ class ConsoleBase(object): # indentation. return self.p.before.strip('\r\n') except Timeout as exc: - self.log.error(str(exc)) - self.cleanup_spawn() + handle_exception(self.config, self, self.log, exc, 'Lab failure', + True) raise - except BootFail as ex: - self.log.error(str(ex)) - self.cleanup_spawn() + except BootFail as exc: + handle_exception(self.config, self, self.log, exc, 'Boot fail', + True, self.get_spawn_output()) raise finally: self.log.timestamp()
Fix a typo in a comment.
Signed-off-by: Simon Glass sjg@chromium.org ---
Changes in v7: - Split out from earlier series - Rebase on -master
test/py/u_boot_console_base.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/test/py/u_boot_console_base.py b/test/py/u_boot_console_base.py index aebe0dac099..d8d0bdf9fd4 100644 --- a/test/py/u_boot_console_base.py +++ b/test/py/u_boot_console_base.py @@ -113,7 +113,7 @@ class ConsoleBase(object): Can only usefully be called by sub-classes.
Args: - log: A mulptiplex_log.Logfile object, to which the U-Boot output + log: A multiplexed_log.Logfile object, to which the U-Boot output will be logged. config: A configuration data structure, as built by conftest.py. max_fifo_fill: The maximum number of characters to send to U-Boot
On Wed, 09 Oct 2024 18:28:57 -0600, Simon Glass wrote:
This series collects together the patches from the Labgrid series which are not related to Labgrid, or at least can be applied independently of using Labgrid to run the lab.
Changes in v7:
- Fix 'board' typo
- Split out from earlier series
- Rebase on -master
[...]
Applied to u-boot/master, thanks!
participants (2)
-
Simon Glass -
Tom Rini