[PATCH v4 00/19] labgrid: Provide an integration with Labgrid

Labgrid provides access to a hardware lab in an automated way. It is possible to boot U-Boot on boards in the lab without physically touching them. It relies on relays, USB UARTs and SD muxes, among other things.
By way of background, about 4 years ago I wrong a thing called Labman[1] which allowed my lab of about 30 devices to be operated remotely, using tbot for the console and build integration. While it worked OK and I used it for many bisects, I didn't take it any further.
It turns out that there was already an existing program, called Labgrid, which I did not know about at time (thank you Tom for telling me). It is more rounded than Labman and has a number of advantages:
- does not need udev rules, mostly - has several existing users who rely on it - supports multiple machines exporting their devices
It lacks a 'lab check' feature and a few other things, but these can be remedied.
On and off over the past several weeks I have been experimenting with Labgrid. I have managed to create an initial U-Boot integration (this series) by adding various features to Labgrid[2] and the U-Boot test hooks.
I hope that this might inspire others to set up boards and run tests automatically, rather than relying on infrequent, manual test. Perhaps it may even be possible to have a number of labs available.
Included in the integration are a number of simple scripts which make it easy to connect to boards and run tests:
ub-int <target> Build and boot on a target, starting an interactive session
ub-cli <target> Build and boot on a target, ensure U-Boot starts and provide an interactive session from there
ub-smoke <target> Smoke test U-Boot to check that it boots to a prompt on a target
ub-bisect Bisect a git tree to locate a failure on a particular target
ub-pyt <target> <testspec> Run U-Boot pytests on a target
Some of these help to provide the same tbot[4] workflow which I have relied on for several years, albeit much simpler versions.
The goal here is to create some sort of script which can collect patches from the mailing list, apply them and test them on a selection of boards. I suspect that script already exists, so please let me know what you suggest.
I hope you find this interesting and take a look!
[1] https://github.com/sjg20/u-boot/tree/lab6a [2] https://github.com/labgrid-project/labgrid/pull/1411 [3] https://github.com/sjg20/uboot-test-hooks/tree/labgrid [4] https://tbot.tools/index.html
Changes in v4: - Expand the commit message
Changes in v3: - Split out most patches into two new series and update cover letter
Changes in v2: - Only disable echo if a terminal is detected - Avoid running a docker image for skipped lab tests
Simon Glass (19): test: Allow signaling that U-Boot is ready test: Use a constant for the test timeout test: Pass stderr to stdout test: Release board after tests complete test: Allow connecting to a running board test: Avoid failing skipped tests test: Create a common function to get the config test: Introduce the concept of a role test: Move the receive code into a function test: Separate out the exception handling test: Detect dead connections test: Tidy up remaining exceptions test: Introduce lab mode test: Improve handling of sending commands test: Fix mulptiplex_log typo test: Avoid double echo when starting up test: Try to shut down the lab console gracefully test: Add a section for closing the connection CI: Allow running tests on sjg lab
.gitlab-ci.yml | 153 +++++++++++++++++++++++++ test/py/conftest.py | 86 +++++++++++--- test/py/u_boot_console_base.py | 154 ++++++++++++++++++-------- test/py/u_boot_console_exec_attach.py | 31 ++++-- test/py/u_boot_spawn.py | 120 +++++++++++++++++--- test/test-main.c | 16 ++- 6 files changed, 470 insertions(+), 90 deletions(-)

When Labgrid is used, it can get U-Boot ready for running tests. It prints a message when it has done so.
Add logic to detect this message and accept it.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
test/py/u_boot_console_base.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/test/py/u_boot_console_base.py b/test/py/u_boot_console_base.py index 76a550d45a1..882d04cd1e9 100644 --- a/test/py/u_boot_console_base.py +++ b/test/py/u_boot_console_base.py @@ -22,6 +22,7 @@ pattern_stop_autoboot_prompt = re.compile('Hit any key to stop autoboot: ') pattern_unknown_command = re.compile('Unknown command '.*' - try 'help'') pattern_error_notification = re.compile('## Error: ') pattern_error_please_reset = re.compile('### ERROR ### Please RESET the board ###') +pattern_ready_prompt = re.compile('U-Boot is ready')
PAT_ID = 0 PAT_RE = 1 @@ -196,15 +197,15 @@ class ConsoleBase(object): self.bad_pattern_ids[m - 1]) self.u_boot_version_string = self.p.after while True: - m = self.p.expect([self.prompt_compiled, + m = self.p.expect([self.prompt_compiled, pattern_ready_prompt, pattern_stop_autoboot_prompt] + self.bad_patterns) - if m == 0: + if m == 0 or m == 1: break - if m == 1: + if m == 2: self.p.send(' ') continue raise Exception('Bad pattern found on console: ' + - self.bad_pattern_ids[m - 2]) + self.bad_pattern_ids[m - 3])
except Exception as ex: self.log.error(str(ex))

Declare a constant rather than open-coding the same value twice.
Signed-off-by: Simon Glass sjg@chromium.org ---
(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 882d04cd1e9..9cc0af66356 100644 --- a/test/py/u_boot_console_base.py +++ b/test/py/u_boot_console_base.py @@ -27,6 +27,9 @@ pattern_ready_prompt = re.compile('U-Boot is ready') 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), @@ -423,7 +426,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') @@ -434,7 +437,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

Some tests may output things to stderr. Ensure that this output is not dropped, by redirecting it to stdout
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
test/py/u_boot_spawn.py | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/test/py/u_boot_spawn.py b/test/py/u_boot_spawn.py index 97e95e07c80..81a09a9d639 100644 --- a/test/py/u_boot_spawn.py +++ b/test/py/u_boot_spawn.py @@ -10,6 +10,7 @@ import re import pty import signal import select +import sys import time import traceback
@@ -59,6 +60,7 @@ class Spawn: signal.signal(signal.SIGHUP, signal.SIG_DFL) if cwd: os.chdir(cwd) + sys.stderr = sys.stdout os.execvp(args[0], args) except: print('CHILD EXECEPTION:')

When a board is finished with, the lab may want to power it off, or perform some other function. Add a new script which is called when tests are complete.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
test/py/u_boot_console_exec_attach.py | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/test/py/u_boot_console_exec_attach.py b/test/py/u_boot_console_exec_attach.py index 8dd8cc1230c..5f4916b7da2 100644 --- a/test/py/u_boot_console_exec_attach.py +++ b/test/py/u_boot_console_exec_attach.py @@ -70,3 +70,13 @@ class ConsoleExecAttach(ConsoleBase): raise
return s + + def close(self): + super().close() + + self.log.action('Releasing board') + args = [self.config.board_type, self.config.board_identity] + cmd = ['u-boot-test-release'] + args + runner = self.log.get_runner(cmd[0], sys.stdout) + runner.run(cmd) + runner.close()

Sometimes we know that the board is already running the right software, so provide an option to allow running of tests directly, without first resetting the board.
This saves time when re-running a test where only the Python code is changing.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
test/py/conftest.py | 3 +++ test/py/u_boot_console_base.py | 14 ++++++++++---- test/py/u_boot_console_exec_attach.py | 21 ++++++++++++--------- 3 files changed, 25 insertions(+), 13 deletions(-)
diff --git a/test/py/conftest.py b/test/py/conftest.py index fc9dd3a83f8..ca66b9d9e61 100644 --- a/test/py/conftest.py +++ b/test/py/conftest.py @@ -79,6 +79,8 @@ def pytest_addoption(parser): parser.addoption('--gdbserver', default=None, help='Run sandbox under gdbserver. The argument is the channel '+ 'over which gdbserver should communicate, e.g. localhost:1234') + parser.addoption('--no-prompt-wait', default=False, action='store_true', + help="Assume that U-Boot is ready and don't wait for a prompt")
def run_build(config, source_dir, build_dir, board_type, log): """run_build: Build U-Boot @@ -238,6 +240,7 @@ def pytest_configure(config): ubconfig.board_type = board_type ubconfig.board_identity = board_identity ubconfig.gdbserver = gdbserver + ubconfig.no_prompt_wait = config.getoption('no_prompt_wait') ubconfig.dtb = build_dir + '/arch/sandbox/dts/test.dtb'
env_vars = ( diff --git a/test/py/u_boot_console_base.py b/test/py/u_boot_console_base.py index 9cc0af66356..8a9c4a576dc 100644 --- a/test/py/u_boot_console_base.py +++ b/test/py/u_boot_console_base.py @@ -439,11 +439,17 @@ class ConsoleBase(object): if not self.config.gdbserver: self.p.timeout = TIMEOUT_MS self.p.logfile_read = self.logstream - if expect_reset: - loop_num = 2 + if self.config.no_prompt_wait: + # Send an empty command to set up the 'expect' logic. This has + # the side effect of ensuring that there was no partial command + # line entered + self.run_command(' ') else: - loop_num = 1 - self.wait_for_boot_prompt(loop_num = loop_num) + if expect_reset: + loop_num = 2 + else: + loop_num = 1 + self.wait_for_boot_prompt(loop_num = loop_num) self.at_prompt = True self.at_prompt_logevt = self.logstream.logfile.cur_evt except Exception as ex: diff --git a/test/py/u_boot_console_exec_attach.py b/test/py/u_boot_console_exec_attach.py index 5f4916b7da2..42fc15197b9 100644 --- a/test/py/u_boot_console_exec_attach.py +++ b/test/py/u_boot_console_exec_attach.py @@ -59,15 +59,18 @@ class ConsoleExecAttach(ConsoleBase): args = [self.config.board_type, self.config.board_identity] s = Spawn(['u-boot-test-console'] + args)
- try: - self.log.action('Resetting board') - cmd = ['u-boot-test-reset'] + args - runner = self.log.get_runner(cmd[0], sys.stdout) - runner.run(cmd) - runner.close() - except: - s.close() - raise + if self.config.no_prompt_wait: + self.log.action('Connecting to board without reset') + else: + try: + self.log.action('Resetting board') + cmd = ['u-boot-test-reset'] + args + runner = self.log.get_runner(cmd[0], sys.stdout) + runner.run(cmd) + runner.close() + except: + s.close() + raise
return s

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 ---
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 3fa6f6e32ec..cda1a186390 100644 --- a/test/test-main.c +++ b/test/test-main.c @@ -448,7 +448,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 & UT_TESTF_OTHER_FDT) && !IS_ENABLED(CONFIG_SANDBOX)) return skip_test(uts); @@ -458,8 +458,11 @@ static int ut_run_test_live_flat(struct unit_test_state *uts, if (CONFIG_IS_ENABLED(OF_LIVE)) { if (!(test->flags & UT_TESTF_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++; + } } }
@@ -483,8 +486,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 ca66b9d9e61..6547c6922c6 100644 --- a/test/py/conftest.py +++ b/test/py/conftest.py @@ -117,14 +117,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) @@ -163,17 +185,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')

In Labgrid there is the concept of a 'role', which is similar to the U-Boot board ID in U-Boot's pytest subsystem.
The role indicates both the target and information about the U-Boot build to use. It can also provide any amount of other configuration. The information is obtained using the 'labgrid-client query' operation.
Make use of this in tests, so that only the role is required in gitlab and other situations. The board type and other things can be queried as needed.
Use a new 'u-boot-test-getrole' script to obtain the requested information.
With this it is possible to run lab tests in gitlab with just a single 'ROLE' variable for each board.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
test/py/conftest.py | 31 +++++++++++++++++++++++++++---- 1 file changed, 27 insertions(+), 4 deletions(-)
diff --git a/test/py/conftest.py b/test/py/conftest.py index 6547c6922c6..5de8d7b0e23 100644 --- a/test/py/conftest.py +++ b/test/py/conftest.py @@ -23,6 +23,7 @@ from pathlib import Path import pytest import re from _pytest.runner import runtestprotocol +import subprocess import sys
# Globals: The HTML log file, and the connection to the U-Boot console. @@ -79,6 +80,7 @@ def pytest_addoption(parser): parser.addoption('--gdbserver', default=None, help='Run sandbox under gdbserver. The argument is the channel '+ 'over which gdbserver should communicate, e.g. localhost:1234') + parser.addoption('--role', help='U-Boot board role (for Labgrid)') parser.addoption('--no-prompt-wait', default=False, action='store_true', help="Assume that U-Boot is ready and don't wait for a prompt")
@@ -130,12 +132,33 @@ def get_details(config): str: Build directory str: Source directory """ - board_type = config.getoption('board_type') - board_identity = config.getoption('board_identity') + role = config.getoption('role') build_dir = config.getoption('build_dir') + if role: + board_identity = role + cmd = ['u-boot-test-getrole', role, '--configure'] + env = os.environ.copy() + if build_dir: + env['U_BOOT_BUILD_DIR'] = build_dir + proc = subprocess.run(cmd, capture_output=True, encoding='utf-8', + env=env) + if proc.returncode: + raise ValueError(proc.stderr) + print('conftest: lab:', proc.stdout) + vals = {} + for line in proc.stdout.splitlines(): + item, value = line.split(' ', maxsplit=1) + k = item.split(':')[-1] + vals[k] = value + print('conftest: lab info:', vals) + board_type, default_build_dir, source_dir = (vals['board'], + vals['build_dir'], vals['source_dir']) + else: + board_type = config.getoption('board_type') + board_identity = config.getoption('board_identity')
- source_dir = os.path.dirname(os.path.dirname(TEST_PY_DIR)) - default_build_dir = source_dir + '/build-' + board_type + 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 = default_build_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 81a09a9d639..cb0d8f702ba 100644 --- a/test/py/u_boot_spawn.py +++ b/test/py/u_boot_spawn.py @@ -139,6 +139,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.
@@ -195,18 +221,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 board 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 ---
(no changes since v1)
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 8a9c4a576dc..fa87952694d 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]*\))') @@ -190,13 +191,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: @@ -207,13 +208,9 @@ class ConsoleBase(object): if m == 2: self.p.send(' ') continue - raise Exception('Bad pattern found on console: ' + + raise BootFail('Bad pattern found on console: ' + self.bad_pattern_ids[m - 3])
- except Exception as ex: - self.log.error(str(ex)) - self.cleanup_spawn() - raise finally: self.log.timestamp()
@@ -279,7 +276,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 @@ -289,14 +286,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 @@ -355,8 +356,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 cb0d8f702ba..57ea845ad4c 100644 --- a/test/py/u_boot_spawn.py +++ b/test/py/u_boot_spawn.py @@ -17,6 +17,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 5de8d7b0e23..42af1abd72e 100644 --- a/test/py/conftest.py +++ b/test/py/conftest.py @@ -25,6 +25,7 @@ import re from _pytest.runner import runtestprotocol import subprocess 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 @@ -280,6 +281,7 @@ def pytest_configure(config): ubconfig.gdbserver = gdbserver ubconfig.no_prompt_wait = config.getoption('no_prompt_wait') ubconfig.dtb = build_dir + '/arch/sandbox/dts/test.dtb' + ubconfig.connection_ok = True
env_vars = ( 'board_type', @@ -446,8 +448,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 57ea845ad4c..62eb4118731 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 sys @@ -28,6 +29,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 fa87952694d..9474fa87ec9 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]*\))') @@ -294,12 +294,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()

There is quite a bit of code in pytest to try to start up U-Boot on a board, with timeouts, expects, etc.
This is tedious to maintain and is peripheral to the test system's purpose. It seems better to put this logic in the lab itself, where is can provide such support.
With Labgrid we can use the UbootStrategy class to get the board into a useful state, however it needs to do it. Then it can report to pytest by writing a suitable string along with the U-Boot version it detected.
Add support for detecting 'lab mode' and simply assume that all is well in that case. Collect the version string when Labgrid says it is ready.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
test/py/u_boot_console_base.py | 68 ++++++++++++++++++++++++++-------- 1 file changed, 53 insertions(+), 15 deletions(-)
diff --git a/test/py/u_boot_console_base.py b/test/py/u_boot_console_base.py index 9474fa87ec9..bcba68f0aac 100644 --- a/test/py/u_boot_console_base.py +++ b/test/py/u_boot_console_base.py @@ -23,13 +23,21 @@ pattern_stop_autoboot_prompt = re.compile('Hit any key to stop autoboot: ') pattern_unknown_command = re.compile('Unknown command '.*' - try 'help'') pattern_error_notification = re.compile('## Error: ') pattern_error_please_reset = re.compile('### ERROR ### Please RESET the board ###') -pattern_ready_prompt = re.compile('U-Boot is ready') +pattern_ready_prompt = re.compile('{lab ready in (.*)s: (.*)}') +pattern_lab_mode = re.compile('{lab mode.*}')
PAT_ID = 0 PAT_RE = 1
# Timeout before expecting the console to be ready (in milliseconds) -TIMEOUT_MS = 30000 +TIMEOUT_MS = 30000 # Standard timeout + +# Timeout for board preparation in lab mode. This needs to be enough to build +# U-Boot, write it to the board and then boot the board. Since this process is +# under the control of another program (e.g. Labgrid), it will failure sooner +# if something goes way. So use a very long timeout here to cover all possible +# situations. +TIMEOUT_PREPARE_MS = 3 * 60 * 1000
bad_pattern_defs = ( ('spl_signon', pattern_u_boot_spl_signon), @@ -143,6 +151,7 @@ class ConsoleBase(object):
self.at_prompt = False self.at_prompt_logevt = None + self.lab_mode = False
def get_spawn(self): # This is not called, ssubclass must define this. @@ -176,40 +185,69 @@ class ConsoleBase(object): self.p.close() self.logstream.close()
+ def set_lab_mode(self): + """Select lab mode + + This tells us that we will get a 'lab ready' message when the board is + ready for use. We don't need to look for signon messages. + """ + self.log.info(f'test.py: Lab mode is active') + self.p.timeout = TIMEOUT_PREPARE_MS + self.lab_mode = True + def wait_for_boot_prompt(self, loop_num = 1): """Wait for the boot up until command prompt. This is for internal use only. """ try: + self.log.info('Waiting for U-Boot to be ready') bcfg = self.config.buildconfig config_spl_serial = bcfg.get('config_spl_serial', 'n') == 'y' env_spl_skipped = self.config.env.get('env__spl_skipped', False) env_spl_banner_times = self.config.env.get('env__spl_banner_times', 1)
- while loop_num > 0: + while not self.lab_mode and loop_num > 0: loop_num -= 1 while config_spl_serial and not env_spl_skipped and env_spl_banner_times > 0: - m = self.p.expect([pattern_u_boot_spl_signon] + - self.bad_patterns) - if m != 0: + m = self.p.expect([pattern_u_boot_spl_signon, + pattern_lab_mode] + self.bad_patterns) + if m == 1: + self.set_lab_mode() + break + elif m != 0: raise BootFail('Bad pattern found on SPL console: ' + - self.bad_pattern_ids[m - 1]) + 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 BootFail('Bad pattern found on console: ' + - self.bad_pattern_ids[m - 1]) - self.u_boot_version_string = self.p.after + if not self.lab_mode: + m = self.p.expect([pattern_u_boot_main_signon, + pattern_lab_mode] + self.bad_patterns) + if m == 1: + self.set_lab_mode() + elif m != 0: + raise BootFail('Bad pattern found on console: ' + + self.bad_pattern_ids[m - 1]) + if not self.lab_mode: + self.u_boot_version_string = self.p.after while True: m = self.p.expect([self.prompt_compiled, pattern_ready_prompt, pattern_stop_autoboot_prompt] + self.bad_patterns) - if m == 0 or m == 1: + if m == 0: + self.log.info(f'Found ready prompt {m}') + break + elif m == 1: + m = pattern_ready_prompt.search(self.p.after) + self.u_boot_version_string = m.group(2) + self.log.info(f'Lab: Board is ready') + self.p.timeout = TIMEOUT_MS break if m == 2: + self.log.info(f'Found autoboot prompt {m}') self.p.send(' ') continue - raise BootFail('Bad pattern found on console: ' + - self.bad_pattern_ids[m - 3]) + if not self.lab_mode: + raise BootFail('Missing prompt / ready message on console: ' + + self.bad_pattern_ids[m - 3]) + self.log.info(f'U-Boot is ready')
finally: self.log.timestamp()

We expect commands to be echoed and this should happen quite quickly, since U-Boot is sitting at the prompt waiting for a command.
Reduce the timeout for this situation. Try to produce a more useful error message when something goes wrong. Also handle the case where the connection has gone away since the last command was issued.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
test/py/u_boot_console_base.py | 35 ++++++++++++++++++++-------------- 1 file changed, 21 insertions(+), 14 deletions(-)
diff --git a/test/py/u_boot_console_base.py b/test/py/u_boot_console_base.py index bcba68f0aac..e2e78179555 100644 --- a/test/py/u_boot_console_base.py +++ b/test/py/u_boot_console_base.py @@ -31,6 +31,7 @@ PAT_RE = 1
# Timeout before expecting the console to be ready (in milliseconds) TIMEOUT_MS = 30000 # Standard timeout +TIMEOUT_CMD_MS = 10000 # Command-echo timeout
# Timeout for board preparation in lab mode. This needs to be enough to build # U-Boot, write it to the board and then boot the board. Since this process is @@ -300,22 +301,28 @@ class ConsoleBase(object):
try: self.at_prompt = False + if not self.p: + raise BootFail( + f"Lab failure: Connection lost when sending command '{cmd}'") + if send_nl: cmd += '\n' - while cmd: - # Limit max outstanding data, so UART FIFOs don't overflow - chunk = cmd[:self.max_fifo_fill] - cmd = cmd[self.max_fifo_fill:] - self.p.send(chunk) - if not wait_for_echo: - continue - chunk = re.escape(chunk) - chunk = chunk.replace('\\n', '[\r\n]') - m = self.p.expect([chunk] + self.bad_patterns) - if m != 0: - self.at_prompt = False - raise BootFail('Bad pattern found on console: ' + - self.bad_pattern_ids[m - 1]) + rem = cmd # Remaining to be sent + with self.temporary_timeout(TIMEOUT_CMD_MS): + while rem: + # Limit max outstanding data, so UART FIFOs don't overflow + chunk = rem[:self.max_fifo_fill] + rem = rem[self.max_fifo_fill:] + self.p.send(chunk) + if not wait_for_echo: + continue + chunk = re.escape(chunk) + chunk = chunk.replace('\\n', '[\r\n]') + m = self.p.expect([chunk] + self.bad_patterns) + if m != 0: + self.at_prompt = False + raise BootFail(f"Failed to get echo on console (cmd '{cmd}':rem '{rem}'): " + + self.bad_pattern_ids[m - 1]) if not wait_for_prompt: return if wait_for_reboot:

Fix a typo in a comment.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
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 e2e78179555..f610fa9a6f8 100644 --- a/test/py/u_boot_console_base.py +++ b/test/py/u_boot_console_base.py @@ -123,7 +123,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

There is a very annoying bug at present where the terminal echos part of the first command sent to the board. This happens because the terminal is still set to echo for a period until Labgrid starts up and can change this.
Fix this by disabling echo (and other terminal features) as soon as the spawn happens.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v2)
Changes in v2: - Only disable echo if a terminal is detected
test/py/u_boot_spawn.py | 13 +++++++++++++ 1 file changed, 13 insertions(+)
diff --git a/test/py/u_boot_spawn.py b/test/py/u_boot_spawn.py index 62eb4118731..c0ff0813554 100644 --- a/test/py/u_boot_spawn.py +++ b/test/py/u_boot_spawn.py @@ -12,6 +12,7 @@ import pytest import signal import select import sys +import termios import time import traceback
@@ -117,11 +118,23 @@ class Spawn: finally: os._exit(255)
+ old = None try: + if os.isatty(sys.stdout.fileno()): + new = termios.tcgetattr(self.fd) + old = new + new[3] = new[3] & ~(termios.ICANON | termios.ISIG) + new[3] = new[3] & ~termios.ECHO + new[6][termios.VMIN] = 0 + new[6][termios.VTIME] = 0 + termios.tcsetattr(self.fd, termios.TCSANOW, new) + self.poll = select.poll() self.poll.register(self.fd, select.POLLIN | select.POLLPRI | select.POLLERR | select.POLLHUP | select.POLLNVAL) except: + if old: + termios.tcsetattr(self.fd, termios.TCSANOW, old) self.close() raise

Send the Labgrid quit characters to ask it to exit gracefully. This typically allows it to power off the board being used.
If that doesn't work, try the less graceful approach.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
test/py/u_boot_spawn.py | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/test/py/u_boot_spawn.py b/test/py/u_boot_spawn.py index c0ff0813554..ec1fa465047 100644 --- a/test/py/u_boot_spawn.py +++ b/test/py/u_boot_spawn.py @@ -16,6 +16,9 @@ import termios import time import traceback
+# Character to send (twice) to exit the terminal +EXIT_CHAR = 0x1d # FS (Ctrl + ]) + class Timeout(Exception): """An exception sub-class that indicates that a timeout occurred."""
@@ -304,15 +307,25 @@ class Spawn: None.
Returns: - Nothing. + str: Type of closure completed """ + self.send(chr(EXIT_CHAR) * 2)
+ # Wait about 10 seconds for Labgrid to close and power off the board + for _ in range(100): + if not self.isalive(): + return 'normal' + time.sleep(0.1) + + # That didn't work, so try closing the PTY os.close(self.fd) for _ in range(100): if not self.isalive(): - break + return 'break' time.sleep(0.1)
+ return 'timeout' + def get_expect_output(self): """Return the output read by expect()

This can take a while and involve multiple steps (e.g. turning the board back off). Add a section for it and show the output.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
test/py/u_boot_console_base.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/test/py/u_boot_console_base.py b/test/py/u_boot_console_base.py index f610fa9a6f8..b279d95dea0 100644 --- a/test/py/u_boot_console_base.py +++ b/test/py/u_boot_console_base.py @@ -183,7 +183,10 @@ class ConsoleBase(object): """
if self.p: - self.p.close() + self.log.start_section('Stopping U-Boot') + close_type = self.p.close() + self.log.info(f'Close type: {close_type}') + self.log.end_section('Stopping U-Boot') self.logstream.close()
def set_lab_mode(self):

Add a way to run tests on a real hardware lab. This is in the very early experimental stages. There are only 23 boards and 3 of those are broken! (bob, ff3399, samus). A fourth fails due to problems with the TPM tests.
To try this, assuming you have gitlab access, set SJG_LAB=1, e.g.:
git push -o ci.variable="SJG_LAB=1" dm HEAD:try
This relies on the two previous series targeted at -next as well as the bugfix series for -master
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Andrejs Cainikovs andrejs.cainikovs@toradex.com ---
(no changes since v3)
Changes in v3: - Split out most patches into two new series and update cover letter
Changes in v2: - Avoid running a docker image for skipped lab tests
.gitlab-ci.yml | 153 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 153 insertions(+)
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 0a15b7352cd..8586a472be1 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -17,6 +17,7 @@ stages: - testsuites - test.py - world build + - sjg-lab
.buildman_and_testpy_template: &buildman_and_testpy_dfn stage: test.py @@ -491,3 +492,155 @@ coreboot test.py: TEST_PY_TEST_SPEC: "not sleep" TEST_PY_ID: "--id qemu" <<: *buildman_and_testpy_dfn + +.lab_template: &lab_dfn + stage: sjg-lab + rules: + - if: $SJG_LAB == "1" + when: always + - when: manual + tags: [ 'lab' ] + script: + - if [[ -z "${SJG_LAB}" ]]; then + exit 0; + fi + # Environment: + # SRC - source tree + # OUT - output directory for builds + - export SRC="$(pwd)" + - export OUT="${SRC}/build/${BOARD}" + - export PATH=$PATH:~/bin + - export PATH=$PATH:/vid/software/devel/ubtest/u-boot-test-hooks/bin + + # Load it on the device + - ret=0 + - echo "role ${ROLE}" + - export strategy="-s uboot -e off" + # export verbose="-v" + - ${SRC}/test/py/test.py --role ${ROLE} --build-dir "${OUT}" + --capture=tee-sys -k "not bootstd"|| ret=$? + - U_BOOT_BOARD_IDENTITY="${ROLE}" u-boot-test-release || true + - if [[ $ret -ne 0 ]]; then + exit $ret; + fi + artifacts: + when: always + paths: + - "build/${BOARD}/test-log.html" + - "build/${BOARD}/multiplexed_log.css" + expire_in: 1 week + +rpi3: + variables: + ROLE: rpi3 + <<: *lab_dfn + +opi_pc: + variables: + ROLE: opi_pc + <<: *lab_dfn + +pcduino3_nano: + variables: + ROLE: pcduino3_nano + <<: *lab_dfn + +samus: + variables: + ROLE: samus + <<: *lab_dfn + +link: + variables: + ROLE: link + <<: *lab_dfn + +jerry: + variables: + ROLE: jerry + <<: *lab_dfn + +minnowmax: + variables: + ROLE: minnowmax + <<: *lab_dfn + +opi_pc2: + variables: + ROLE: opi_pc2 + <<: *lab_dfn + +bpi: + variables: + ROLE: bpi + <<: *lab_dfn + +rpi2: + variables: + ROLE: rpi2 + <<: *lab_dfn + +bob: + variables: + ROLE: bob + <<: *lab_dfn + +ff3399: + variables: + ROLE: ff3399 + <<: *lab_dfn + +coral: + variables: + ROLE: coral + <<: *lab_dfn + +rpi3z: + variables: + ROLE: rpi3z + <<: *lab_dfn + +bbb: + variables: + ROLE: bbb + <<: *lab_dfn + +kevin: + variables: + ROLE: kevin + <<: *lab_dfn + +pine64: + variables: + ROLE: pine64 + <<: *lab_dfn + +c4: + variables: + ROLE: c4 + <<: *lab_dfn + +rpi4: + variables: + ROLE: rpi4 + <<: *lab_dfn + +rpi0: + variables: + ROLE: rpi0 + <<: *lab_dfn + +snow: + variables: + ROLE: snow + <<: *lab_dfn + +pcduino3: + variables: + ROLE: pcduino3 + <<: *lab_dfn + +nyan-big: + variables: + ROLE: nyan-big + <<: *lab_dfn
participants (1)
-
Simon Glass