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

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')

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

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 03dfd8ab562..9eea65b5929 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 @@ -287,6 +288,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', @@ -453,8 +455,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.

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()

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

Send the Labgrid quit characters to ask it to exit gracefully. This typically allows it to power off the board being used. Only do this when labgrid is being used (detected with an env var).

If that doesn't work, try the less graceful approach.

Signed-off-by: Simon Glass sjg@chromium.org ---

Changes in v6: - Avoid doing the special shutdown unless USE_LABGRID is enabled

test/py/u_boot_spawn.py | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/test/py/u_boot_spawn.py b/test/py/u_boot_spawn.py index f2398098a00..72d3d5e77b1 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."""

@@ -303,15 +306,28 @@ class Spawn: None.

Returns: - Nothing. + str: Type of closure completed """ - + # For Labgrid, ask it is exit gracefully, so it can transition the board + # to the final state (like 'off') before exiting. + if os.environ.get('USE_LABGRID'): + 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()

The Beagleplay board uses two entirely separate builds to produce an image, rather than using an SPL build for this purpose.

Handle this in test.py by adding more parameters.

Signed-off-by: Simon Glass sjg@chromium.org ---

(no changes since v5)

Changes in v5: - Add a patch to support testing with two board-builds

test/py/conftest.py | 36 +++++++++++++++++++++++++++++++----- 1 file changed, 31 insertions(+), 5 deletions(-)

diff --git a/test/py/conftest.py b/test/py/conftest.py index 9eea65b5929..542c0d53e46 100644 --- a/test/py/conftest.py +++ b/test/py/conftest.py @@ -66,12 +66,16 @@ def pytest_addoption(parser):

parser.addoption('--build-dir', default=None, help='U-Boot build directory (O=)') + parser.addoption('--build-dir-extra', default=None, + help='U-Boot build directory for extra build (O=)') parser.addoption('--result-dir', default=None, help='U-Boot test result/tmp directory') parser.addoption('--persistent-data-dir', default=None, help='U-Boot test persistent generated data directory') parser.addoption('--board-type', '--bd', '-B', default='sandbox', help='U-Boot board type') + parser.addoption('--board-type-extra', '--bde', default='sandbox', + help='U-Boot extra board type') parser.addoption('--board-identity', '--id', default='na', help='U-Boot board identity/instance') parser.addoption('--build', default=False, action='store_true', @@ -129,14 +133,17 @@ def get_details(config): Returns: tuple: str: Board type (U-Boot build name) + str: Extra board type (where two U-Boot builds are needed) str: Identity for the lab board str: Build directory + str: Extra build directory (where two U-Boot builds are needed) str: Source directory """ role = config.getoption('role')

# Get a few provided parameters build_dir = config.getoption('build_dir') + build_dir_extra = config.getoption('build_dir_extra') if role: # When using a role, build_dir and build_dir_extra are normally not set, # since they are picked up from Labgrid via the u-boot-test-getrole @@ -146,6 +153,8 @@ def get_details(config): env = os.environ.copy() if build_dir: env['U_BOOT_BUILD_DIR'] = build_dir + if build_dir_extra: + env['U_BOOT_BUILD_DIR_EXTRA'] = build_dir_extra proc = subprocess.run(cmd, capture_output=True, encoding='utf-8', env=env) if proc.returncode: @@ -159,24 +168,36 @@ def get_details(config): vals[k] = value # For debugging # print('conftest: lab info:', vals) - board_type, default_build_dir, source_dir = (vals['board'], - vals['build_dir'], vals['source_dir']) + + # Read the build directories here, in case none were provided in the + # command-line arguments + (board_type, board_type_extra, default_build_dir, + default_build_dir_extra, source_dir) = (vals['board'], + vals['board_extra'], vals['build_dir'], vals['build_dir_extra'], + vals['source_dir']) else: board_type = config.getoption('board_type') + board_type_extra = config.getoption('board_type_extra') 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 + default_build_dir_extra = source_dir + '/build-' + board_type_extra + + # Use the provided command-line arguments if present, else fall back to if not build_dir: build_dir = default_build_dir + if not build_dir_extra: + build_dir_extra = default_build_dir_extra

- return board_type, board_identity, build_dir, source_dir + return (board_type, board_type_extra, board_identity, build_dir, + build_dir_extra, 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_dir = get_details(config)[3]

build_done_file = Path(build_dir) / 'build.done' if build_done_file.exists(): @@ -216,7 +237,8 @@ def pytest_configure(config): global console global ubconfig

- board_type, board_identity, build_dir, source_dir = get_details(config) + (board_type, board_type_extra, board_identity, build_dir, build_dir_extra, + source_dir) = get_details(config)

board_type_filename = board_type.replace('-', '_') board_identity_filename = board_identity.replace('-', '_') @@ -281,9 +303,11 @@ def pytest_configure(config): ubconfig.test_py_dir = TEST_PY_DIR ubconfig.source_dir = source_dir ubconfig.build_dir = build_dir + ubconfig.build_dir_extra = build_dir_extra ubconfig.result_dir = result_dir ubconfig.persistent_data_dir = persistent_data_dir ubconfig.board_type = board_type + ubconfig.board_type_extra = board_type_extra ubconfig.board_identity = board_identity ubconfig.gdbserver = gdbserver ubconfig.no_prompt_wait = config.getoption('no_prompt_wait') @@ -292,10 +316,12 @@ def pytest_configure(config):

env_vars = ( 'board_type', + 'board_type_extra', 'board_identity', 'source_dir', 'test_py_dir', 'build_dir', + 'build_dir_extra', 'result_dir', 'persistent_data_dir', )