* [PATCH v3 01/19] test: Allow signaling that U-Boot is ready
2024-06-23 20:31 [PATCH v3 00/19] labgrid: Provide an integration with Labgrid Simon Glass
@ 2024-06-23 20:31 ` Simon Glass
2024-06-23 20:31 ` [PATCH v3 02/19] test: Use a constant for the test timeout Simon Glass
` (17 subsequent siblings)
18 siblings, 0 replies; 47+ messages in thread
From: Simon Glass @ 2024-06-23 20:31 UTC (permalink / raw)
To: U-Boot Mailing List; +Cc: Tom Rini, Simon Glass, Love Kumar
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))
--
2.34.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH v3 02/19] test: Use a constant for the test timeout
2024-06-23 20:31 [PATCH v3 00/19] labgrid: Provide an integration with Labgrid Simon Glass
2024-06-23 20:31 ` [PATCH v3 01/19] test: Allow signaling that U-Boot is ready Simon Glass
@ 2024-06-23 20:31 ` Simon Glass
2024-06-23 20:31 ` [PATCH v3 03/19] test: Pass stderr to stdout Simon Glass
` (16 subsequent siblings)
18 siblings, 0 replies; 47+ messages in thread
From: Simon Glass @ 2024-06-23 20:31 UTC (permalink / raw)
To: U-Boot Mailing List; +Cc: Tom Rini, Simon Glass, Love Kumar
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
--
2.34.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH v3 03/19] test: Pass stderr to stdout
2024-06-23 20:31 [PATCH v3 00/19] labgrid: Provide an integration with Labgrid Simon Glass
2024-06-23 20:31 ` [PATCH v3 01/19] test: Allow signaling that U-Boot is ready Simon Glass
2024-06-23 20:31 ` [PATCH v3 02/19] test: Use a constant for the test timeout Simon Glass
@ 2024-06-23 20:31 ` Simon Glass
2024-06-23 20:31 ` [PATCH v3 04/19] test: Release board after tests complete Simon Glass
` (15 subsequent siblings)
18 siblings, 0 replies; 47+ messages in thread
From: Simon Glass @ 2024-06-23 20:31 UTC (permalink / raw)
To: U-Boot Mailing List; +Cc: Tom Rini, Simon Glass
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:')
--
2.34.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH v3 04/19] test: Release board after tests complete
2024-06-23 20:31 [PATCH v3 00/19] labgrid: Provide an integration with Labgrid Simon Glass
` (2 preceding siblings ...)
2024-06-23 20:31 ` [PATCH v3 03/19] test: Pass stderr to stdout Simon Glass
@ 2024-06-23 20:31 ` Simon Glass
2024-06-23 20:31 ` [PATCH v3 05/19] test: Allow connecting to a running board Simon Glass
` (14 subsequent siblings)
18 siblings, 0 replies; 47+ messages in thread
From: Simon Glass @ 2024-06-23 20:31 UTC (permalink / raw)
To: U-Boot Mailing List; +Cc: Tom Rini, Simon Glass
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()
--
2.34.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH v3 05/19] test: Allow connecting to a running board
2024-06-23 20:31 [PATCH v3 00/19] labgrid: Provide an integration with Labgrid Simon Glass
` (3 preceding siblings ...)
2024-06-23 20:31 ` [PATCH v3 04/19] test: Release board after tests complete Simon Glass
@ 2024-06-23 20:31 ` Simon Glass
2024-06-23 20:32 ` [PATCH v3 06/19] test: Avoid failing skipped tests Simon Glass
` (13 subsequent siblings)
18 siblings, 0 replies; 47+ messages in thread
From: Simon Glass @ 2024-06-23 20:31 UTC (permalink / raw)
To: U-Boot Mailing List; +Cc: Tom Rini, Simon Glass, Love Kumar
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
--
2.34.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH v3 06/19] test: Avoid failing skipped tests
2024-06-23 20:31 [PATCH v3 00/19] labgrid: Provide an integration with Labgrid Simon Glass
` (4 preceding siblings ...)
2024-06-23 20:31 ` [PATCH v3 05/19] test: Allow connecting to a running board Simon Glass
@ 2024-06-23 20:32 ` Simon Glass
2024-06-24 18:05 ` Tom Rini
2024-06-23 20:32 ` [PATCH v3 07/19] test: Create a common function to get the config Simon Glass
` (12 subsequent siblings)
18 siblings, 1 reply; 47+ messages in thread
From: Simon Glass @ 2024-06-23 20:32 UTC (permalink / raw)
To: U-Boot Mailing List; +Cc: Tom Rini, Simon Glass, Bin Meng, Sean Anderson
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.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
(no changes since v1)
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;
--
2.34.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [PATCH v3 06/19] test: Avoid failing skipped tests
2024-06-23 20:32 ` [PATCH v3 06/19] test: Avoid failing skipped tests Simon Glass
@ 2024-06-24 18:05 ` Tom Rini
2024-06-25 12:38 ` Simon Glass
0 siblings, 1 reply; 47+ messages in thread
From: Tom Rini @ 2024-06-24 18:05 UTC (permalink / raw)
To: Simon Glass; +Cc: U-Boot Mailing List, Bin Meng, Sean Anderson
[-- Attachment #1: Type: text/plain, Size: 1844 bytes --]
On Sun, Jun 23, 2024 at 02:32:00PM -0600, Simon Glass wrote:
> 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.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
> (no changes since v1)
>
> 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;
How did you trigger this case exactly?
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH v3 06/19] test: Avoid failing skipped tests
2024-06-24 18:05 ` Tom Rini
@ 2024-06-25 12:38 ` Simon Glass
2024-06-25 14:14 ` Tom Rini
0 siblings, 1 reply; 47+ messages in thread
From: Simon Glass @ 2024-06-25 12:38 UTC (permalink / raw)
To: Tom Rini; +Cc: U-Boot Mailing List, Bin Meng, Sean Anderson
Hi Tom,
On Mon, 24 Jun 2024 at 19:06, Tom Rini <trini@konsulko.com> wrote:
>
> On Sun, Jun 23, 2024 at 02:32:00PM -0600, Simon Glass wrote:
>
> > 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.
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> >
> > (no changes since v1)
> >
> > 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;
>
> How did you trigger this case exactly?
I noticed this in CI, where some skipped tests were shown as failed in
the log, even though they were not counted as failures in the final
results.
>
> --
> Tom
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH v3 06/19] test: Avoid failing skipped tests
2024-06-25 12:38 ` Simon Glass
@ 2024-06-25 14:14 ` Tom Rini
2024-06-26 8:00 ` Simon Glass
0 siblings, 1 reply; 47+ messages in thread
From: Tom Rini @ 2024-06-25 14:14 UTC (permalink / raw)
To: Simon Glass; +Cc: U-Boot Mailing List, Bin Meng, Sean Anderson
[-- Attachment #1: Type: text/plain, Size: 2815 bytes --]
On Tue, Jun 25, 2024 at 01:38:00PM +0100, Simon Glass wrote:
> Hi Tom,
>
> On Mon, 24 Jun 2024 at 19:06, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Sun, Jun 23, 2024 at 02:32:00PM -0600, Simon Glass wrote:
> >
> > > 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.
> > >
> > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > ---
> > >
> > > (no changes since v1)
> > >
> > > 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;
> >
> > How did you trigger this case exactly?
>
> I noticed this in CI, where some skipped tests were shown as failed in
> the log, even though they were not counted as failures in the final
> results.
That's really really strange, do you have an example log or something
around still?
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH v3 06/19] test: Avoid failing skipped tests
2024-06-25 14:14 ` Tom Rini
@ 2024-06-26 8:00 ` Simon Glass
2024-06-26 13:56 ` Tom Rini
2024-08-22 3:00 ` Simon Glass
0 siblings, 2 replies; 47+ messages in thread
From: Simon Glass @ 2024-06-26 8:00 UTC (permalink / raw)
To: Tom Rini; +Cc: U-Boot Mailing List, Bin Meng, Sean Anderson
Hi Tom,
On Tue, 25 Jun 2024 at 15:14, Tom Rini <trini@konsulko.com> wrote:
>
> On Tue, Jun 25, 2024 at 01:38:00PM +0100, Simon Glass wrote:
> > Hi Tom,
> >
> > On Mon, 24 Jun 2024 at 19:06, Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Sun, Jun 23, 2024 at 02:32:00PM -0600, Simon Glass wrote:
> > >
> > > > 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.
> > > >
> > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > > ---
> > > >
> > > > (no changes since v1)
> > > >
> > > > 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;
> > >
> > > How did you trigger this case exactly?
> >
> > I noticed this in CI, where some skipped tests were shown as failed in
> > the log, even though they were not counted as failures in the final
> > results.
>
> That's really really strange, do you have an example log or something
> around still?
This happens on snow, which is (maybe) the only real board that
defines CONFIG_UNIT_TEST
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
For this particular mechanism (-EAGAIN returned by test_pre_run()) , I
think a better fix would be to squash the error in ut_run_test(), as
is done when -EAGAIN is returned in the body of the test. I'll update
that. I cannot see any other way this could happen, but we can always
deal with it later if it does.
Regards,
Simon
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH v3 06/19] test: Avoid failing skipped tests
2024-06-26 8:00 ` Simon Glass
@ 2024-06-26 13:56 ` Tom Rini
2024-06-26 13:59 ` Tom Rini
2024-08-22 3:00 ` Simon Glass
1 sibling, 1 reply; 47+ messages in thread
From: Tom Rini @ 2024-06-26 13:56 UTC (permalink / raw)
To: Simon Glass; +Cc: U-Boot Mailing List, Bin Meng, Sean Anderson
[-- Attachment #1: Type: text/plain, Size: 4345 bytes --]
On Wed, Jun 26, 2024 at 09:00:42AM +0100, Simon Glass wrote:
> Hi Tom,
>
> On Tue, 25 Jun 2024 at 15:14, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Tue, Jun 25, 2024 at 01:38:00PM +0100, Simon Glass wrote:
> > > Hi Tom,
> > >
> > > On Mon, 24 Jun 2024 at 19:06, Tom Rini <trini@konsulko.com> wrote:
> > > >
> > > > On Sun, Jun 23, 2024 at 02:32:00PM -0600, Simon Glass wrote:
> > > >
> > > > > 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.
> > > > >
> > > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > > > ---
> > > > >
> > > > > (no changes since v1)
> > > > >
> > > > > 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;
> > > >
> > > > How did you trigger this case exactly?
> > >
> > > I noticed this in CI, where some skipped tests were shown as failed in
> > > the log, even though they were not counted as failures in the final
> > > results.
> >
> > That's really really strange, do you have an example log or something
> > around still?
>
> This happens on snow, which is (maybe) the only real board that
> defines CONFIG_UNIT_TEST
I think it is too, but that's also perhaps a reminder that I should be
enabling it as part of my build before testing scripts. I'll go do that
now and see if this problem shows up a tiny bit more widely.
> 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
>
> For this particular mechanism (-EAGAIN returned by test_pre_run()) , I
> think a better fix would be to squash the error in ut_run_test(), as
> is done when -EAGAIN is returned in the body of the test. I'll update
> that. I cannot see any other way this could happen, but we can always
> deal with it later if it does.
Thanks for explaining, please also include the example in the commit
message in v2.
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH v3 06/19] test: Avoid failing skipped tests
2024-06-26 13:56 ` Tom Rini
@ 2024-06-26 13:59 ` Tom Rini
0 siblings, 0 replies; 47+ messages in thread
From: Tom Rini @ 2024-06-26 13:59 UTC (permalink / raw)
To: Simon Glass; +Cc: U-Boot Mailing List, Bin Meng, Sean Anderson
[-- Attachment #1: Type: text/plain, Size: 3886 bytes --]
On Wed, Jun 26, 2024 at 07:56:24AM -0600, Tom Rini wrote:
> On Wed, Jun 26, 2024 at 09:00:42AM +0100, Simon Glass wrote:
> > Hi Tom,
> >
> > On Tue, 25 Jun 2024 at 15:14, Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Tue, Jun 25, 2024 at 01:38:00PM +0100, Simon Glass wrote:
> > > > Hi Tom,
> > > >
> > > > On Mon, 24 Jun 2024 at 19:06, Tom Rini <trini@konsulko.com> wrote:
> > > > >
> > > > > On Sun, Jun 23, 2024 at 02:32:00PM -0600, Simon Glass wrote:
> > > > >
> > > > > > 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.
> > > > > >
> > > > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > > > > ---
> > > > > >
> > > > > > (no changes since v1)
> > > > > >
> > > > > > 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;
> > > > >
> > > > > How did you trigger this case exactly?
> > > >
> > > > I noticed this in CI, where some skipped tests were shown as failed in
> > > > the log, even though they were not counted as failures in the final
> > > > results.
> > >
> > > That's really really strange, do you have an example log or something
> > > around still?
> >
> > This happens on snow, which is (maybe) the only real board that
> > defines CONFIG_UNIT_TEST
>
> I think it is too, but that's also perhaps a reminder that I should be
> enabling it as part of my build before testing scripts. I'll go do that
> now and see if this problem shows up a tiny bit more widely.
OK, not right now then, there's missing dependencies within the test.
I'll selectively enable once v2 is in tho.
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v3 06/19] test: Avoid failing skipped tests
2024-06-26 8:00 ` Simon Glass
2024-06-26 13:56 ` Tom Rini
@ 2024-08-22 3:00 ` Simon Glass
2024-08-22 14:11 ` Tom Rini
1 sibling, 1 reply; 47+ messages in thread
From: Simon Glass @ 2024-08-22 3:00 UTC (permalink / raw)
To: Tom Rini; +Cc: U-Boot Mailing List, Bin Meng, Sean Anderson
Hi Tom,
On Wed, 26 Jun 2024 at 02:00, Simon Glass <sjg@chromium.org> wrote:
>
> Hi Tom,
>
> On Tue, 25 Jun 2024 at 15:14, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Tue, Jun 25, 2024 at 01:38:00PM +0100, Simon Glass wrote:
> > > Hi Tom,
> > >
> > > On Mon, 24 Jun 2024 at 19:06, Tom Rini <trini@konsulko.com> wrote:
> > > >
> > > > On Sun, Jun 23, 2024 at 02:32:00PM -0600, Simon Glass wrote:
> > > >
> > > > > 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.
> > > > >
> > > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > > > ---
> > > > >
> > > > > (no changes since v1)
> > > > >
> > > > > 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;
> > > >
> > > > How did you trigger this case exactly?
> > >
> > > I noticed this in CI, where some skipped tests were shown as failed in
> > > the log, even though they were not counted as failures in the final
> > > results.
> >
> > That's really really strange, do you have an example log or something
> > around still?
>
> This happens on snow, which is (maybe) the only real board that
> defines CONFIG_UNIT_TEST
>
> 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
>
> For this particular mechanism (-EAGAIN returned by test_pre_run()) , I
> think a better fix would be to squash the error in ut_run_test(), as
> is done when -EAGAIN is returned in the body of the test. I'll update
> that. I cannot see any other way this could happen, but we can always
> deal with it later if it does.
No, that doesn't work, since the failure counting happens in the
caller. This patch seems to be the right fix, so I'll send it again
with a better commit message.
Regards,
Simon
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH v3 06/19] test: Avoid failing skipped tests
2024-08-22 3:00 ` Simon Glass
@ 2024-08-22 14:11 ` Tom Rini
2024-08-22 14:22 ` Simon Glass
0 siblings, 1 reply; 47+ messages in thread
From: Tom Rini @ 2024-08-22 14:11 UTC (permalink / raw)
To: Simon Glass; +Cc: U-Boot Mailing List, Bin Meng, Sean Anderson
[-- Attachment #1: Type: text/plain, Size: 4634 bytes --]
On Wed, Aug 21, 2024 at 09:00:33PM -0600, Simon Glass wrote:
> Hi Tom,
>
> On Wed, 26 Jun 2024 at 02:00, Simon Glass <sjg@chromium.org> wrote:
> >
> > Hi Tom,
> >
> > On Tue, 25 Jun 2024 at 15:14, Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Tue, Jun 25, 2024 at 01:38:00PM +0100, Simon Glass wrote:
> > > > Hi Tom,
> > > >
> > > > On Mon, 24 Jun 2024 at 19:06, Tom Rini <trini@konsulko.com> wrote:
> > > > >
> > > > > On Sun, Jun 23, 2024 at 02:32:00PM -0600, Simon Glass wrote:
> > > > >
> > > > > > 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.
> > > > > >
> > > > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > > > > ---
> > > > > >
> > > > > > (no changes since v1)
> > > > > >
> > > > > > 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;
> > > > >
> > > > > How did you trigger this case exactly?
> > > >
> > > > I noticed this in CI, where some skipped tests were shown as failed in
> > > > the log, even though they were not counted as failures in the final
> > > > results.
> > >
> > > That's really really strange, do you have an example log or something
> > > around still?
> >
> > This happens on snow, which is (maybe) the only real board that
> > defines CONFIG_UNIT_TEST
> >
> > 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
> >
> > For this particular mechanism (-EAGAIN returned by test_pre_run()) , I
> > think a better fix would be to squash the error in ut_run_test(), as
> > is done when -EAGAIN is returned in the body of the test. I'll update
> > that. I cannot see any other way this could happen, but we can always
> > deal with it later if it does.
>
> No, that doesn't work, since the failure counting happens in the
> caller. This patch seems to be the right fix, so I'll send it again
> with a better commit message.
And I'll try and test it on Pi as with the series I posted and so
UNIT_TEST compiles on there, I got what looks like the same failure.
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH v3 06/19] test: Avoid failing skipped tests
2024-08-22 14:11 ` Tom Rini
@ 2024-08-22 14:22 ` Simon Glass
2024-08-22 14:29 ` Tom Rini
0 siblings, 1 reply; 47+ messages in thread
From: Simon Glass @ 2024-08-22 14:22 UTC (permalink / raw)
To: Tom Rini; +Cc: U-Boot Mailing List, Bin Meng, Sean Anderson
Hi Tom,
On Thu, 22 Aug 2024 at 08:11, Tom Rini <trini@konsulko.com> wrote:
>
> On Wed, Aug 21, 2024 at 09:00:33PM -0600, Simon Glass wrote:
> > Hi Tom,
> >
> > On Wed, 26 Jun 2024 at 02:00, Simon Glass <sjg@chromium.org> wrote:
> > >
> > > Hi Tom,
> > >
> > > On Tue, 25 Jun 2024 at 15:14, Tom Rini <trini@konsulko.com> wrote:
> > > >
> > > > On Tue, Jun 25, 2024 at 01:38:00PM +0100, Simon Glass wrote:
> > > > > Hi Tom,
> > > > >
> > > > > On Mon, 24 Jun 2024 at 19:06, Tom Rini <trini@konsulko.com> wrote:
> > > > > >
> > > > > > On Sun, Jun 23, 2024 at 02:32:00PM -0600, Simon Glass wrote:
> > > > > >
> > > > > > > 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.
> > > > > > >
> > > > > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > > > > > ---
> > > > > > >
> > > > > > > (no changes since v1)
> > > > > > >
> > > > > > > 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;
> > > > > >
> > > > > > How did you trigger this case exactly?
> > > > >
> > > > > I noticed this in CI, where some skipped tests were shown as failed in
> > > > > the log, even though they were not counted as failures in the final
> > > > > results.
> > > >
> > > > That's really really strange, do you have an example log or something
> > > > around still?
> > >
> > > This happens on snow, which is (maybe) the only real board that
> > > defines CONFIG_UNIT_TEST
> > >
> > > 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
> > >
> > > For this particular mechanism (-EAGAIN returned by test_pre_run()) , I
> > > think a better fix would be to squash the error in ut_run_test(), as
> > > is done when -EAGAIN is returned in the body of the test. I'll update
> > > that. I cannot see any other way this could happen, but we can always
> > > deal with it later if it does.
> >
> > No, that doesn't work, since the failure counting happens in the
> > caller. This patch seems to be the right fix, so I'll send it again
> > with a better commit message.
>
> And I'll try and test it on Pi as with the series I posted and so
> UNIT_TEST compiles on there, I got what looks like the same failure.
OK ta.
It would be good to get the Labgrid series into -next as it fixes
quite a few problems on snow for me. It will provide a better base for
solving other problems.
Regards,
SImon
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH v3 06/19] test: Avoid failing skipped tests
2024-08-22 14:22 ` Simon Glass
@ 2024-08-22 14:29 ` Tom Rini
0 siblings, 0 replies; 47+ messages in thread
From: Tom Rini @ 2024-08-22 14:29 UTC (permalink / raw)
To: Simon Glass; +Cc: U-Boot Mailing List, Bin Meng, Sean Anderson
[-- Attachment #1: Type: text/plain, Size: 5598 bytes --]
On Thu, Aug 22, 2024 at 08:22:59AM -0600, Simon Glass wrote:
> Hi Tom,
>
> On Thu, 22 Aug 2024 at 08:11, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Wed, Aug 21, 2024 at 09:00:33PM -0600, Simon Glass wrote:
> > > Hi Tom,
> > >
> > > On Wed, 26 Jun 2024 at 02:00, Simon Glass <sjg@chromium.org> wrote:
> > > >
> > > > Hi Tom,
> > > >
> > > > On Tue, 25 Jun 2024 at 15:14, Tom Rini <trini@konsulko.com> wrote:
> > > > >
> > > > > On Tue, Jun 25, 2024 at 01:38:00PM +0100, Simon Glass wrote:
> > > > > > Hi Tom,
> > > > > >
> > > > > > On Mon, 24 Jun 2024 at 19:06, Tom Rini <trini@konsulko.com> wrote:
> > > > > > >
> > > > > > > On Sun, Jun 23, 2024 at 02:32:00PM -0600, Simon Glass wrote:
> > > > > > >
> > > > > > > > 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.
> > > > > > > >
> > > > > > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > > > > > > ---
> > > > > > > >
> > > > > > > > (no changes since v1)
> > > > > > > >
> > > > > > > > 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;
> > > > > > >
> > > > > > > How did you trigger this case exactly?
> > > > > >
> > > > > > I noticed this in CI, where some skipped tests were shown as failed in
> > > > > > the log, even though they were not counted as failures in the final
> > > > > > results.
> > > > >
> > > > > That's really really strange, do you have an example log or something
> > > > > around still?
> > > >
> > > > This happens on snow, which is (maybe) the only real board that
> > > > defines CONFIG_UNIT_TEST
> > > >
> > > > 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
> > > >
> > > > For this particular mechanism (-EAGAIN returned by test_pre_run()) , I
> > > > think a better fix would be to squash the error in ut_run_test(), as
> > > > is done when -EAGAIN is returned in the body of the test. I'll update
> > > > that. I cannot see any other way this could happen, but we can always
> > > > deal with it later if it does.
> > >
> > > No, that doesn't work, since the failure counting happens in the
> > > caller. This patch seems to be the right fix, so I'll send it again
> > > with a better commit message.
> >
> > And I'll try and test it on Pi as with the series I posted and so
> > UNIT_TEST compiles on there, I got what looks like the same failure.
>
> OK ta.
>
> It would be good to get the Labgrid series into -next as it fixes
> quite a few problems on snow for me. It will provide a better base for
> solving other problems.
I want to pick some of it for sure. But it also broke my lab support
(before an office power cycle left it in an odd state and I need to go
and confirm what's needed / not needed again to bring the boards up).
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH v3 07/19] test: Create a common function to get the config
2024-06-23 20:31 [PATCH v3 00/19] labgrid: Provide an integration with Labgrid Simon Glass
` (5 preceding siblings ...)
2024-06-23 20:32 ` [PATCH v3 06/19] test: Avoid failing skipped tests Simon Glass
@ 2024-06-23 20:32 ` Simon Glass
2024-06-23 20:32 ` [PATCH v3 08/19] test: Introduce the concept of a role Simon Glass
` (11 subsequent siblings)
18 siblings, 0 replies; 47+ messages in thread
From: Simon Glass @ 2024-06-23 20:32 UTC (permalink / raw)
To: U-Boot Mailing List; +Cc: Tom Rini, Simon Glass
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')
--
2.34.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH v3 08/19] test: Introduce the concept of a role
2024-06-23 20:31 [PATCH v3 00/19] labgrid: Provide an integration with Labgrid Simon Glass
` (6 preceding siblings ...)
2024-06-23 20:32 ` [PATCH v3 07/19] test: Create a common function to get the config Simon Glass
@ 2024-06-23 20:32 ` Simon Glass
2024-06-24 18:13 ` Tom Rini
2024-06-23 20:32 ` [PATCH v3 09/19] test: Move the receive code into a function Simon Glass
` (10 subsequent siblings)
18 siblings, 1 reply; 47+ messages in thread
From: Simon Glass @ 2024-06-23 20:32 UTC (permalink / raw)
To: U-Boot Mailing List; +Cc: Tom Rini, Simon Glass
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
--
2.34.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [PATCH v3 08/19] test: Introduce the concept of a role
2024-06-23 20:32 ` [PATCH v3 08/19] test: Introduce the concept of a role Simon Glass
@ 2024-06-24 18:13 ` Tom Rini
2024-06-25 12:38 ` Simon Glass
0 siblings, 1 reply; 47+ messages in thread
From: Tom Rini @ 2024-06-24 18:13 UTC (permalink / raw)
To: Simon Glass; +Cc: U-Boot Mailing List
[-- Attachment #1: Type: text/plain, Size: 3766 bytes --]
On Sun, Jun 23, 2024 at 02:32:02PM -0600, Simon Glass wrote:
> 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
I'm a little confused here. Why can't we construct "role" from
board_type+board_identity and then we have the board
conf.${board_type}_${board_identity} file set whatever needs setting to
be "labgrid" ?
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH v3 08/19] test: Introduce the concept of a role
2024-06-24 18:13 ` Tom Rini
@ 2024-06-25 12:38 ` Simon Glass
2024-06-25 14:27 ` Tom Rini
0 siblings, 1 reply; 47+ messages in thread
From: Simon Glass @ 2024-06-25 12:38 UTC (permalink / raw)
To: Tom Rini; +Cc: U-Boot Mailing List
Hi Tom,
On Mon, 24 Jun 2024 at 19:13, Tom Rini <trini@konsulko.com> wrote:
>
> On Sun, Jun 23, 2024 at 02:32:02PM -0600, Simon Glass wrote:
>
> > 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
>
> I'm a little confused here. Why can't we construct "role" from
> board_type+board_identity and then we have the board
> conf.${board_type}_${board_identity} file set whatever needs setting to
> be "labgrid" ?
The role is equivalent to the board identity, not the combination of
the U-Boot board type and the board identity. I went this way to avoid
having to type long and complicated roles when connecting to boards.
It is similar to how things work today, except that the board type is
implied by the 'role'.
For boards which have multiple identities (e.g. can support two
different board types), Labgrid handles acquiring and releasing the
shares resources, to avoid any problems.
Regards,
Simon
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH v3 08/19] test: Introduce the concept of a role
2024-06-25 12:38 ` Simon Glass
@ 2024-06-25 14:27 ` Tom Rini
2024-06-26 8:00 ` Simon Glass
0 siblings, 1 reply; 47+ messages in thread
From: Tom Rini @ 2024-06-25 14:27 UTC (permalink / raw)
To: Simon Glass; +Cc: U-Boot Mailing List
[-- Attachment #1: Type: text/plain, Size: 6470 bytes --]
On Tue, Jun 25, 2024 at 01:38:08PM +0100, Simon Glass wrote:
> Hi Tom,
>
> On Mon, 24 Jun 2024 at 19:13, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Sun, Jun 23, 2024 at 02:32:02PM -0600, Simon Glass wrote:
> >
> > > 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
> >
> > I'm a little confused here. Why can't we construct "role" from
> > board_type+board_identity and then we have the board
> > conf.${board_type}_${board_identity} file set whatever needs setting to
> > be "labgrid" ?
>
> The role is equivalent to the board identity, not the combination of
> the U-Boot board type and the board identity. I went this way to avoid
> having to type long and complicated roles when connecting to boards.
> It is similar to how things work today, except that the board type is
> implied by the 'role'.
>
> For boards which have multiple identities (e.g. can support two
> different board types), Labgrid handles acquiring and releasing the
> shares resources, to avoid any problems.
I guess I have two sets of questions. First, if it's basically the
board identity why can't we just use that as the role name, in your
setup?
But second, I'm not sure why we need this. The labgrid support we worked
up here (but, sigh, it's not really clean enough to post) has:
$ cat bin/lootbox/conf.rpi_4_na
console_impl=labgrid
reset_impl=labgrid
flash_impl=labgrid.rpi_4
flash_writer=labgrid.rpi_4
For example and:
$ cat bin/writer.labgrid.rpi_4
set -e
build=${U_BOOT_BUILD_DIR}
$LG_CLIENT write-files -T ${build}/u-boot.bin kernel8.img
So I don't know what the "role" part you have is for. And yes, there end
up being multiple writer.labgrid.FOO scripts because for TI K3 platforms
(such as beagleplay) we have:
$ cat bin/writer.labgrid.ti-k3
set -e
build=${U_BOOT_BUILD_DIR}
if [ -z "${tispl}" -o -z "${uboot}" -o -z "${tiboot3}" ]; then
echo "Must configure tispl, uboot, tiboot3 and optionally sysfw"
echo "per the board documentation."
exit 1
fi
echo "Writing build at ${build}"
$LG_CLIENT write-files -T ${build}/${tispl} tispl.bin
$LG_CLIENT write-files -T ${build}/${uboot} u-boot.img
$LG_CLIENT write-files -T ${build/_a??/_r5}/${tiboot3} tiboot3.bin
echo "Done writing build"
In all cases we first build U-Boot and then pass the build directory to
test.py, in something like:
export LG_PLACE=rpi4
./test/py/test.py -ra --bd rpi_4 --build-dir .../build-rpi4 \
--results-dir .../results-rpi4 --persistent-data-dir .../pd-rpi4 \
--exitfirst
The only U-Boot side changes I needed to make were for counting the SPL
banner instances, and that was regardless of framework (a particularly
fun platform will show it 3 times).
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH v3 08/19] test: Introduce the concept of a role
2024-06-25 14:27 ` Tom Rini
@ 2024-06-26 8:00 ` Simon Glass
2024-06-26 14:29 ` Tom Rini
0 siblings, 1 reply; 47+ messages in thread
From: Simon Glass @ 2024-06-26 8:00 UTC (permalink / raw)
To: Tom Rini; +Cc: U-Boot Mailing List
Hi Tom,
On Tue, 25 Jun 2024 at 15:27, Tom Rini <trini@konsulko.com> wrote:
>
> On Tue, Jun 25, 2024 at 01:38:08PM +0100, Simon Glass wrote:
> > Hi Tom,
> >
> > On Mon, 24 Jun 2024 at 19:13, Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Sun, Jun 23, 2024 at 02:32:02PM -0600, Simon Glass wrote:
> > >
> > > > 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
> > >
> > > I'm a little confused here. Why can't we construct "role" from
> > > board_type+board_identity and then we have the board
> > > conf.${board_type}_${board_identity} file set whatever needs setting to
> > > be "labgrid" ?
> >
> > The role is equivalent to the board identity, not the combination of
> > the U-Boot board type and the board identity. I went this way to avoid
> > having to type long and complicated roles when connecting to boards.
> > It is similar to how things work today, except that the board type is
> > implied by the 'role'.
> >
> > For boards which have multiple identities (e.g. can support two
> > different board types), Labgrid handles acquiring and releasing the
> > shares resources, to avoid any problems.
>
> I guess I have two sets of questions. First, if it's basically the
> board identity why can't we just use that as the role name, in your
> setup?
Yes, that's what I am doing. If you look in console.labgrid you can
see that it is passing U_BOOT_BOARD_IDENTITY as the -r argument.
>
> But second, I'm not sure why we need this. The labgrid support we worked
> up here (but, sigh, it's not really clean enough to post) has:
> $ cat bin/lootbox/conf.rpi_4_na
> console_impl=labgrid
> reset_impl=labgrid
> flash_impl=labgrid.rpi_4
> flash_writer=labgrid.rpi_4
>
> For example and:
> $ cat bin/writer.labgrid.rpi_4
> set -e
>
> build=${U_BOOT_BUILD_DIR}
>
> $LG_CLIENT write-files -T ${build}/u-boot.bin kernel8.img
>
> So I don't know what the "role" part you have is for. And yes, there end
> up being multiple writer.labgrid.FOO scripts because for TI K3 platforms
> (such as beagleplay) we have:
> $ cat bin/writer.labgrid.ti-k3
> set -e
> build=${U_BOOT_BUILD_DIR}
>
> if [ -z "${tispl}" -o -z "${uboot}" -o -z "${tiboot3}" ]; then
> echo "Must configure tispl, uboot, tiboot3 and optionally sysfw"
> echo "per the board documentation."
> exit 1
> fi
> echo "Writing build at ${build}"
> $LG_CLIENT write-files -T ${build}/${tispl} tispl.bin
> $LG_CLIENT write-files -T ${build}/${uboot} u-boot.img
> $LG_CLIENT write-files -T ${build/_a??/_r5}/${tiboot3} tiboot3.bin
> echo "Done writing build"
>
> In all cases we first build U-Boot and then pass the build directory to
> test.py, in something like:
> export LG_PLACE=rpi4
> ./test/py/test.py -ra --bd rpi_4 --build-dir .../build-rpi4 \
> --results-dir .../results-rpi4 --persistent-data-dir .../pd-rpi4 \
> --exitfirst
>
> The only U-Boot side changes I needed to make were for counting the SPL
> banner instances, and that was regardless of framework (a particularly
> fun platform will show it 3 times).
Yes it is possible to build U-Boot separately. Of course that involved
various blobs and so on, so every board is different. The approach I
have taken here is to have Labgrid call buildman to build what is
needed, with the blobs defined in the environment.
You can use the -B flag to use a pre-built U-Boot, with the scripts
I've included.
Regards,
Simon
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH v3 08/19] test: Introduce the concept of a role
2024-06-26 8:00 ` Simon Glass
@ 2024-06-26 14:29 ` Tom Rini
2024-06-27 8:37 ` Simon Glass
0 siblings, 1 reply; 47+ messages in thread
From: Tom Rini @ 2024-06-26 14:29 UTC (permalink / raw)
To: Simon Glass; +Cc: U-Boot Mailing List
[-- Attachment #1: Type: text/plain, Size: 8301 bytes --]
On Wed, Jun 26, 2024 at 09:00:33AM +0100, Simon Glass wrote:
> Hi Tom,
>
> On Tue, 25 Jun 2024 at 15:27, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Tue, Jun 25, 2024 at 01:38:08PM +0100, Simon Glass wrote:
> > > Hi Tom,
> > >
> > > On Mon, 24 Jun 2024 at 19:13, Tom Rini <trini@konsulko.com> wrote:
> > > >
> > > > On Sun, Jun 23, 2024 at 02:32:02PM -0600, Simon Glass wrote:
> > > >
> > > > > 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
> > > >
> > > > I'm a little confused here. Why can't we construct "role" from
> > > > board_type+board_identity and then we have the board
> > > > conf.${board_type}_${board_identity} file set whatever needs setting to
> > > > be "labgrid" ?
> > >
> > > The role is equivalent to the board identity, not the combination of
> > > the U-Boot board type and the board identity. I went this way to avoid
> > > having to type long and complicated roles when connecting to boards.
> > > It is similar to how things work today, except that the board type is
> > > implied by the 'role'.
> > >
> > > For boards which have multiple identities (e.g. can support two
> > > different board types), Labgrid handles acquiring and releasing the
> > > shares resources, to avoid any problems.
> >
> > I guess I have two sets of questions. First, if it's basically the
> > board identity why can't we just use that as the role name, in your
> > setup?
>
> Yes, that's what I am doing. If you look in console.labgrid you can
> see that it is passing U_BOOT_BOARD_IDENTITY as the -r argument.
Then why do we need this?
> > But second, I'm not sure why we need this. The labgrid support we worked
> > up here (but, sigh, it's not really clean enough to post) has:
> > $ cat bin/lootbox/conf.rpi_4_na
> > console_impl=labgrid
> > reset_impl=labgrid
> > flash_impl=labgrid.rpi_4
> > flash_writer=labgrid.rpi_4
> >
> > For example and:
> > $ cat bin/writer.labgrid.rpi_4
> > set -e
> >
> > build=${U_BOOT_BUILD_DIR}
> >
> > $LG_CLIENT write-files -T ${build}/u-boot.bin kernel8.img
> >
> > So I don't know what the "role" part you have is for. And yes, there end
> > up being multiple writer.labgrid.FOO scripts because for TI K3 platforms
> > (such as beagleplay) we have:
> > $ cat bin/writer.labgrid.ti-k3
> > set -e
> > build=${U_BOOT_BUILD_DIR}
> >
> > if [ -z "${tispl}" -o -z "${uboot}" -o -z "${tiboot3}" ]; then
> > echo "Must configure tispl, uboot, tiboot3 and optionally sysfw"
> > echo "per the board documentation."
> > exit 1
> > fi
> > echo "Writing build at ${build}"
> > $LG_CLIENT write-files -T ${build}/${tispl} tispl.bin
> > $LG_CLIENT write-files -T ${build}/${uboot} u-boot.img
> > $LG_CLIENT write-files -T ${build/_a??/_r5}/${tiboot3} tiboot3.bin
> > echo "Done writing build"
> >
> > In all cases we first build U-Boot and then pass the build directory to
> > test.py, in something like:
> > export LG_PLACE=rpi4
> > ./test/py/test.py -ra --bd rpi_4 --build-dir .../build-rpi4 \
> > --results-dir .../results-rpi4 --persistent-data-dir .../pd-rpi4 \
> > --exitfirst
> >
> > The only U-Boot side changes I needed to make were for counting the SPL
> > banner instances, and that was regardless of framework (a particularly
> > fun platform will show it 3 times).
>
> Yes it is possible to build U-Boot separately. Of course that involved
> various blobs and so on, so every board is different. The approach I
> have taken here is to have Labgrid call buildman to build what is
> needed, with the blobs defined in the environment.
>
> You can use the -B flag to use a pre-built U-Boot, with the scripts
> I've included.
OK. I personally think pre-built (or outside of buildman built) U-Boot
will be an important case, since everything needs more options enabled
to test more features, but on real hardware. For example,
CONFIG_UNIT_TEST should be on for everyone, in testing, once the build
issues are resolved. And I need to add CONFIG_FIT to some platforms so
that I can then use the kernel test. And some platforms I end up
enabling CONFIG_CMD_BOOTEFI_HELLO on (but others disabling
CONFIG_CMD_BOOTEFI_SELFTEST as that fails and that's just A Thing).
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH v3 08/19] test: Introduce the concept of a role
2024-06-26 14:29 ` Tom Rini
@ 2024-06-27 8:37 ` Simon Glass
2024-07-02 23:12 ` Tom Rini
0 siblings, 1 reply; 47+ messages in thread
From: Simon Glass @ 2024-06-27 8:37 UTC (permalink / raw)
To: Tom Rini; +Cc: U-Boot Mailing List
Hi Tom,
On Wed, 26 Jun 2024 at 15:29, Tom Rini <trini@konsulko.com> wrote:
>
> On Wed, Jun 26, 2024 at 09:00:33AM +0100, Simon Glass wrote:
> > Hi Tom,
> >
> > On Tue, 25 Jun 2024 at 15:27, Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Tue, Jun 25, 2024 at 01:38:08PM +0100, Simon Glass wrote:
> > > > Hi Tom,
> > > >
> > > > On Mon, 24 Jun 2024 at 19:13, Tom Rini <trini@konsulko.com> wrote:
> > > > >
> > > > > On Sun, Jun 23, 2024 at 02:32:02PM -0600, Simon Glass wrote:
> > > > >
> > > > > > 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
> > > > >
> > > > > I'm a little confused here. Why can't we construct "role" from
> > > > > board_type+board_identity and then we have the board
> > > > > conf.${board_type}_${board_identity} file set whatever needs setting to
> > > > > be "labgrid" ?
> > > >
> > > > The role is equivalent to the board identity, not the combination of
> > > > the U-Boot board type and the board identity. I went this way to avoid
> > > > having to type long and complicated roles when connecting to boards.
> > > > It is similar to how things work today, except that the board type is
> > > > implied by the 'role'.
> > > >
> > > > For boards which have multiple identities (e.g. can support two
> > > > different board types), Labgrid handles acquiring and releasing the
> > > > shares resources, to avoid any problems.
> > >
> > > I guess I have two sets of questions. First, if it's basically the
> > > board identity why can't we just use that as the role name, in your
> > > setup?
> >
> > Yes, that's what I am doing. If you look in console.labgrid you can
> > see that it is passing U_BOOT_BOARD_IDENTITY as the -r argument.
>
> Then why do we need this?
We need to pass a role to Labgrid, since it determines the board
identity to use. It also (indirectly) determines the U-Boot build to
use, since each board identity / role is a particular board with a
particular build.
For example:
role / identify = samus - uses 'samus' board with build chromebook_samus
role / identify = samus_tpl - uses 'samus' board with build
chromebook_samus_tpl
Basically, as I understand it, the 'role' is the thing we want.
Labgrid environment:
samus:
resources:
RemotePlace:
name: samus
...
UBootProviderDriver:
board: chromebook_samus
binman_indir: /vid/software/devel/samus/bin
samus_tpl:
resources:
RemotePlace:
name: samus
UBootProviderDriver:
board: chromebook_samus_tpl
binman_indir: /vid/software/devel/samus/bin
>
> > > But second, I'm not sure why we need this. The labgrid support we worked
> > > up here (but, sigh, it's not really clean enough to post) has:
> > > $ cat bin/lootbox/conf.rpi_4_na
> > > console_impl=labgrid
> > > reset_impl=labgrid
> > > flash_impl=labgrid.rpi_4
> > > flash_writer=labgrid.rpi_4
> > >
> > > For example and:
> > > $ cat bin/writer.labgrid.rpi_4
> > > set -e
> > >
> > > build=${U_BOOT_BUILD_DIR}
> > >
> > > $LG_CLIENT write-files -T ${build}/u-boot.bin kernel8.img
> > >
> > > So I don't know what the "role" part you have is for. And yes, there end
> > > up being multiple writer.labgrid.FOO scripts because for TI K3 platforms
> > > (such as beagleplay) we have:
> > > $ cat bin/writer.labgrid.ti-k3
> > > set -e
> > > build=${U_BOOT_BUILD_DIR}
> > >
> > > if [ -z "${tispl}" -o -z "${uboot}" -o -z "${tiboot3}" ]; then
> > > echo "Must configure tispl, uboot, tiboot3 and optionally sysfw"
> > > echo "per the board documentation."
> > > exit 1
> > > fi
> > > echo "Writing build at ${build}"
> > > $LG_CLIENT write-files -T ${build}/${tispl} tispl.bin
> > > $LG_CLIENT write-files -T ${build}/${uboot} u-boot.img
> > > $LG_CLIENT write-files -T ${build/_a??/_r5}/${tiboot3} tiboot3.bin
> > > echo "Done writing build"
> > >
> > > In all cases we first build U-Boot and then pass the build directory to
> > > test.py, in something like:
> > > export LG_PLACE=rpi4
> > > ./test/py/test.py -ra --bd rpi_4 --build-dir .../build-rpi4 \
> > > --results-dir .../results-rpi4 --persistent-data-dir .../pd-rpi4 \
> > > --exitfirst
> > >
> > > The only U-Boot side changes I needed to make were for counting the SPL
> > > banner instances, and that was regardless of framework (a particularly
> > > fun platform will show it 3 times).
> >
> > Yes it is possible to build U-Boot separately. Of course that involved
> > various blobs and so on, so every board is different. The approach I
> > have taken here is to have Labgrid call buildman to build what is
> > needed, with the blobs defined in the environment.
> >
> > You can use the -B flag to use a pre-built U-Boot, with the scripts
> > I've included.
>
> OK. I personally think pre-built (or outside of buildman built) U-Boot
> will be an important case, since everything needs more options enabled
> to test more features, but on real hardware. For example,
> CONFIG_UNIT_TEST should be on for everyone, in testing, once the build
> issues are resolved. And I need to add CONFIG_FIT to some platforms so
> that I can then use the kernel test. And some platforms I end up
> enabling CONFIG_CMD_BOOTEFI_HELLO on (but others disabling
> CONFIG_CMD_BOOTEFI_SELFTEST as that fails and that's just A Thing).
Yes that all sounds good. I have figured out how to add QEMU into this
Labgrid integration, but I cannot Debian to boot all the way to a
prompt with -nographic unless I pass something special on the Linux
commandline. So for now I parked that.
Regards,
Simon
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH v3 08/19] test: Introduce the concept of a role
2024-06-27 8:37 ` Simon Glass
@ 2024-07-02 23:12 ` Tom Rini
2024-07-13 15:13 ` Simon Glass
0 siblings, 1 reply; 47+ messages in thread
From: Tom Rini @ 2024-07-02 23:12 UTC (permalink / raw)
To: Simon Glass; +Cc: U-Boot Mailing List
[-- Attachment #1: Type: text/plain, Size: 11439 bytes --]
On Thu, Jun 27, 2024 at 09:37:18AM +0100, Simon Glass wrote:
> Hi Tom,
>
> On Wed, 26 Jun 2024 at 15:29, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Wed, Jun 26, 2024 at 09:00:33AM +0100, Simon Glass wrote:
> > > Hi Tom,
> > >
> > > On Tue, 25 Jun 2024 at 15:27, Tom Rini <trini@konsulko.com> wrote:
> > > >
> > > > On Tue, Jun 25, 2024 at 01:38:08PM +0100, Simon Glass wrote:
> > > > > Hi Tom,
> > > > >
> > > > > On Mon, 24 Jun 2024 at 19:13, Tom Rini <trini@konsulko.com> wrote:
> > > > > >
> > > > > > On Sun, Jun 23, 2024 at 02:32:02PM -0600, Simon Glass wrote:
> > > > > >
> > > > > > > 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
> > > > > >
> > > > > > I'm a little confused here. Why can't we construct "role" from
> > > > > > board_type+board_identity and then we have the board
> > > > > > conf.${board_type}_${board_identity} file set whatever needs setting to
> > > > > > be "labgrid" ?
> > > > >
> > > > > The role is equivalent to the board identity, not the combination of
> > > > > the U-Boot board type and the board identity. I went this way to avoid
> > > > > having to type long and complicated roles when connecting to boards.
> > > > > It is similar to how things work today, except that the board type is
> > > > > implied by the 'role'.
> > > > >
> > > > > For boards which have multiple identities (e.g. can support two
> > > > > different board types), Labgrid handles acquiring and releasing the
> > > > > shares resources, to avoid any problems.
> > > >
> > > > I guess I have two sets of questions. First, if it's basically the
> > > > board identity why can't we just use that as the role name, in your
> > > > setup?
> > >
> > > Yes, that's what I am doing. If you look in console.labgrid you can
> > > see that it is passing U_BOOT_BOARD_IDENTITY as the -r argument.
> >
> > Then why do we need this?
>
> We need to pass a role to Labgrid, since it determines the board
> identity to use. It also (indirectly) determines the U-Boot build to
> use, since each board identity / role is a particular board with a
> particular build.
Oh, I get where you're coming from now at least. But this still sounds
like a detail to put in to the conf.${board}_${board_type} file and not
a thing to set here?
> For example:
> role / identify = samus - uses 'samus' board with build chromebook_samus
> role / identify = samus_tpl - uses 'samus' board with build
> chromebook_samus_tpl
Yes, or using one physical Pi 4 to test rpi_4_defconfig and
rpi_arm64_defconfig (and if labgrid supported a way to remove files from
the FAT partition, rpi_4_32b_defconfig).
> Basically, as I understand it, the 'role' is the thing we want.
>
> Labgrid environment:
>
> samus:
> resources:
> RemotePlace:
> name: samus
> ...
> UBootProviderDriver:
> board: chromebook_samus
> binman_indir: /vid/software/devel/samus/bin
>
> samus_tpl:
> resources:
> RemotePlace:
> name: samus
> UBootProviderDriver:
> board: chromebook_samus_tpl
> binman_indir: /vid/software/devel/samus/bin
I guess the problem here is that from my point of view, this can live in
the u-boot-test-hooks/bin/<host>/conf.<machine> file since we're never
going to worry about building U-Boot (even if blobs aren't a problem, we
want to enable more features to test more things on HW) but from your
point of view, buildman must provide test.py with the correct build so
we need to know things prior.
> > > > But second, I'm not sure why we need this. The labgrid support we worked
> > > > up here (but, sigh, it's not really clean enough to post) has:
> > > > $ cat bin/lootbox/conf.rpi_4_na
> > > > console_impl=labgrid
> > > > reset_impl=labgrid
> > > > flash_impl=labgrid.rpi_4
> > > > flash_writer=labgrid.rpi_4
> > > >
> > > > For example and:
> > > > $ cat bin/writer.labgrid.rpi_4
> > > > set -e
> > > >
> > > > build=${U_BOOT_BUILD_DIR}
> > > >
> > > > $LG_CLIENT write-files -T ${build}/u-boot.bin kernel8.img
> > > >
> > > > So I don't know what the "role" part you have is for. And yes, there end
> > > > up being multiple writer.labgrid.FOO scripts because for TI K3 platforms
> > > > (such as beagleplay) we have:
> > > > $ cat bin/writer.labgrid.ti-k3
> > > > set -e
> > > > build=${U_BOOT_BUILD_DIR}
> > > >
> > > > if [ -z "${tispl}" -o -z "${uboot}" -o -z "${tiboot3}" ]; then
> > > > echo "Must configure tispl, uboot, tiboot3 and optionally sysfw"
> > > > echo "per the board documentation."
> > > > exit 1
> > > > fi
> > > > echo "Writing build at ${build}"
> > > > $LG_CLIENT write-files -T ${build}/${tispl} tispl.bin
> > > > $LG_CLIENT write-files -T ${build}/${uboot} u-boot.img
> > > > $LG_CLIENT write-files -T ${build/_a??/_r5}/${tiboot3} tiboot3.bin
> > > > echo "Done writing build"
> > > >
> > > > In all cases we first build U-Boot and then pass the build directory to
> > > > test.py, in something like:
> > > > export LG_PLACE=rpi4
> > > > ./test/py/test.py -ra --bd rpi_4 --build-dir .../build-rpi4 \
> > > > --results-dir .../results-rpi4 --persistent-data-dir .../pd-rpi4 \
> > > > --exitfirst
> > > >
> > > > The only U-Boot side changes I needed to make were for counting the SPL
> > > > banner instances, and that was regardless of framework (a particularly
> > > > fun platform will show it 3 times).
> > >
> > > Yes it is possible to build U-Boot separately. Of course that involved
> > > various blobs and so on, so every board is different. The approach I
> > > have taken here is to have Labgrid call buildman to build what is
> > > needed, with the blobs defined in the environment.
> > >
> > > You can use the -B flag to use a pre-built U-Boot, with the scripts
> > > I've included.
> >
> > OK. I personally think pre-built (or outside of buildman built) U-Boot
> > will be an important case, since everything needs more options enabled
> > to test more features, but on real hardware. For example,
> > CONFIG_UNIT_TEST should be on for everyone, in testing, once the build
> > issues are resolved. And I need to add CONFIG_FIT to some platforms so
> > that I can then use the kernel test. And some platforms I end up
> > enabling CONFIG_CMD_BOOTEFI_HELLO on (but others disabling
> > CONFIG_CMD_BOOTEFI_SELFTEST as that fails and that's just A Thing).
>
> Yes that all sounds good. I have figured out how to add QEMU into this
> Labgrid integration, but I cannot Debian to boot all the way to a
> prompt with -nographic unless I pass something special on the Linux
> commandline. So for now I parked that.
Putting QEMU in to labgrid too could be interesting, yes. But I lost how
it's related? To be clear, today we can test boot a Linux kernel on
hardware. Somewhere on my TODO list is cycling over what kernel images
to grab and shove in to the docker container so that our existing QEMU
tests can do that too, for some platforms at least.
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH v3 08/19] test: Introduce the concept of a role
2024-07-02 23:12 ` Tom Rini
@ 2024-07-13 15:13 ` Simon Glass
2024-07-13 16:57 ` Tom Rini
0 siblings, 1 reply; 47+ messages in thread
From: Simon Glass @ 2024-07-13 15:13 UTC (permalink / raw)
To: Tom Rini; +Cc: U-Boot Mailing List
Hi Tom,
On Wed, 3 Jul 2024 at 00:12, Tom Rini <trini@konsulko.com> wrote:
>
> On Thu, Jun 27, 2024 at 09:37:18AM +0100, Simon Glass wrote:
> > Hi Tom,
> >
> > On Wed, 26 Jun 2024 at 15:29, Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Wed, Jun 26, 2024 at 09:00:33AM +0100, Simon Glass wrote:
> > > > Hi Tom,
> > > >
> > > > On Tue, 25 Jun 2024 at 15:27, Tom Rini <trini@konsulko.com> wrote:
> > > > >
> > > > > On Tue, Jun 25, 2024 at 01:38:08PM +0100, Simon Glass wrote:
> > > > > > Hi Tom,
> > > > > >
> > > > > > On Mon, 24 Jun 2024 at 19:13, Tom Rini <trini@konsulko.com> wrote:
> > > > > > >
> > > > > > > On Sun, Jun 23, 2024 at 02:32:02PM -0600, Simon Glass wrote:
> > > > > > >
> > > > > > > > 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
> > > > > > >
> > > > > > > I'm a little confused here. Why can't we construct "role" from
> > > > > > > board_type+board_identity and then we have the board
> > > > > > > conf.${board_type}_${board_identity} file set whatever needs setting to
> > > > > > > be "labgrid" ?
> > > > > >
> > > > > > The role is equivalent to the board identity, not the combination of
> > > > > > the U-Boot board type and the board identity. I went this way to avoid
> > > > > > having to type long and complicated roles when connecting to boards.
> > > > > > It is similar to how things work today, except that the board type is
> > > > > > implied by the 'role'.
> > > > > >
> > > > > > For boards which have multiple identities (e.g. can support two
> > > > > > different board types), Labgrid handles acquiring and releasing the
> > > > > > shares resources, to avoid any problems.
> > > > >
> > > > > I guess I have two sets of questions. First, if it's basically the
> > > > > board identity why can't we just use that as the role name, in your
> > > > > setup?
> > > >
> > > > Yes, that's what I am doing. If you look in console.labgrid you can
> > > > see that it is passing U_BOOT_BOARD_IDENTITY as the -r argument.
> > >
> > > Then why do we need this?
> >
> > We need to pass a role to Labgrid, since it determines the board
> > identity to use. It also (indirectly) determines the U-Boot build to
> > use, since each board identity / role is a particular board with a
> > particular build.
>
> Oh, I get where you're coming from now at least. But this still sounds
> like a detail to put in to the conf.${board}_${board_type} file and not
> a thing to set here?
There are no such files with the Labgrid integration so far. They are
not needed.
>
> > For example:
> > role / identify = samus - uses 'samus' board with build chromebook_samus
> > role / identify = samus_tpl - uses 'samus' board with build
> > chromebook_samus_tpl
>
> Yes, or using one physical Pi 4 to test rpi_4_defconfig and
> rpi_arm64_defconfig (and if labgrid supported a way to remove files from
> the FAT partition, rpi_4_32b_defconfig).
Yes, that would be a nice addition.
>
> > Basically, as I understand it, the 'role' is the thing we want.
> >
> > Labgrid environment:
> >
> > samus:
> > resources:
> > RemotePlace:
> > name: samus
> > ...
> > UBootProviderDriver:
> > board: chromebook_samus
> > binman_indir: /vid/software/devel/samus/bin
> >
> > samus_tpl:
> > resources:
> > RemotePlace:
> > name: samus
> > UBootProviderDriver:
> > board: chromebook_samus_tpl
> > binman_indir: /vid/software/devel/samus/bin
>
> I guess the problem here is that from my point of view, this can live in
> the u-boot-test-hooks/bin/<host>/conf.<machine> file since we're never
> going to worry about building U-Boot (even if blobs aren't a problem, we
> want to enable more features to test more things on HW) but from your
> point of view, buildman must provide test.py with the correct build so
> we need to know things prior.
Well, either you already have a build to test, iwc it is fine, or if
you don't you can pass --build to force a build, or rely on Labgrid to
initiate the build.
But in your case, the build must be done before running test.py since
it needs the .config file.
>
> > > > > But second, I'm not sure why we need this. The labgrid support we worked
> > > > > up here (but, sigh, it's not really clean enough to post) has:
> > > > > $ cat bin/lootbox/conf.rpi_4_na
> > > > > console_impl=labgrid
> > > > > reset_impl=labgrid
> > > > > flash_impl=labgrid.rpi_4
> > > > > flash_writer=labgrid.rpi_4
> > > > >
> > > > > For example and:
> > > > > $ cat bin/writer.labgrid.rpi_4
> > > > > set -e
> > > > >
> > > > > build=${U_BOOT_BUILD_DIR}
> > > > >
> > > > > $LG_CLIENT write-files -T ${build}/u-boot.bin kernel8.img
> > > > >
> > > > > So I don't know what the "role" part you have is for. And yes, there end
> > > > > up being multiple writer.labgrid.FOO scripts because for TI K3 platforms
> > > > > (such as beagleplay) we have:
> > > > > $ cat bin/writer.labgrid.ti-k3
> > > > > set -e
> > > > > build=${U_BOOT_BUILD_DIR}
> > > > >
> > > > > if [ -z "${tispl}" -o -z "${uboot}" -o -z "${tiboot3}" ]; then
> > > > > echo "Must configure tispl, uboot, tiboot3 and optionally sysfw"
> > > > > echo "per the board documentation."
> > > > > exit 1
> > > > > fi
> > > > > echo "Writing build at ${build}"
> > > > > $LG_CLIENT write-files -T ${build}/${tispl} tispl.bin
> > > > > $LG_CLIENT write-files -T ${build}/${uboot} u-boot.img
> > > > > $LG_CLIENT write-files -T ${build/_a??/_r5}/${tiboot3} tiboot3.bin
> > > > > echo "Done writing build"
> > > > >
> > > > > In all cases we first build U-Boot and then pass the build directory to
> > > > > test.py, in something like:
> > > > > export LG_PLACE=rpi4
> > > > > ./test/py/test.py -ra --bd rpi_4 --build-dir .../build-rpi4 \
> > > > > --results-dir .../results-rpi4 --persistent-data-dir .../pd-rpi4 \
> > > > > --exitfirst
> > > > >
> > > > > The only U-Boot side changes I needed to make were for counting the SPL
> > > > > banner instances, and that was regardless of framework (a particularly
> > > > > fun platform will show it 3 times).
> > > >
> > > > Yes it is possible to build U-Boot separately. Of course that involved
> > > > various blobs and so on, so every board is different. The approach I
> > > > have taken here is to have Labgrid call buildman to build what is
> > > > needed, with the blobs defined in the environment.
> > > >
> > > > You can use the -B flag to use a pre-built U-Boot, with the scripts
> > > > I've included.
> > >
> > > OK. I personally think pre-built (or outside of buildman built) U-Boot
> > > will be an important case, since everything needs more options enabled
> > > to test more features, but on real hardware. For example,
> > > CONFIG_UNIT_TEST should be on for everyone, in testing, once the build
> > > issues are resolved. And I need to add CONFIG_FIT to some platforms so
> > > that I can then use the kernel test. And some platforms I end up
> > > enabling CONFIG_CMD_BOOTEFI_HELLO on (but others disabling
> > > CONFIG_CMD_BOOTEFI_SELFTEST as that fails and that's just A Thing).
> >
> > Yes that all sounds good. I have figured out how to add QEMU into this
> > Labgrid integration, but I cannot Debian to boot all the way to a
> > prompt with -nographic unless I pass something special on the Linux
> > commandline. So for now I parked that.
>
> Putting QEMU in to labgrid too could be interesting, yes. But I lost how
> it's related? To be clear, today we can test boot a Linux kernel on
> hardware. Somewhere on my TODO list is cycling over what kernel images
> to grab and shove in to the docker container so that our existing QEMU
> tests can do that too, for some platforms at least.
It's just a nice way of allowing 'ub-int qemu-x86' and getting to a
U-Boot prompt. Yes there are other ways to do it, and in fact it works
today if you set up your conf files for the machine you are on.
Regards,
Simon
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH v3 08/19] test: Introduce the concept of a role
2024-07-13 15:13 ` Simon Glass
@ 2024-07-13 16:57 ` Tom Rini
2024-07-15 7:11 ` Simon Glass
0 siblings, 1 reply; 47+ messages in thread
From: Tom Rini @ 2024-07-13 16:57 UTC (permalink / raw)
To: Simon Glass; +Cc: U-Boot Mailing List
[-- Attachment #1: Type: text/plain, Size: 13402 bytes --]
On Sat, Jul 13, 2024 at 04:13:55PM +0100, Simon Glass wrote:
> Hi Tom,
>
> On Wed, 3 Jul 2024 at 00:12, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Thu, Jun 27, 2024 at 09:37:18AM +0100, Simon Glass wrote:
> > > Hi Tom,
> > >
> > > On Wed, 26 Jun 2024 at 15:29, Tom Rini <trini@konsulko.com> wrote:
> > > >
> > > > On Wed, Jun 26, 2024 at 09:00:33AM +0100, Simon Glass wrote:
> > > > > Hi Tom,
> > > > >
> > > > > On Tue, 25 Jun 2024 at 15:27, Tom Rini <trini@konsulko.com> wrote:
> > > > > >
> > > > > > On Tue, Jun 25, 2024 at 01:38:08PM +0100, Simon Glass wrote:
> > > > > > > Hi Tom,
> > > > > > >
> > > > > > > On Mon, 24 Jun 2024 at 19:13, Tom Rini <trini@konsulko.com> wrote:
> > > > > > > >
> > > > > > > > On Sun, Jun 23, 2024 at 02:32:02PM -0600, Simon Glass wrote:
> > > > > > > >
> > > > > > > > > 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
> > > > > > > >
> > > > > > > > I'm a little confused here. Why can't we construct "role" from
> > > > > > > > board_type+board_identity and then we have the board
> > > > > > > > conf.${board_type}_${board_identity} file set whatever needs setting to
> > > > > > > > be "labgrid" ?
> > > > > > >
> > > > > > > The role is equivalent to the board identity, not the combination of
> > > > > > > the U-Boot board type and the board identity. I went this way to avoid
> > > > > > > having to type long and complicated roles when connecting to boards.
> > > > > > > It is similar to how things work today, except that the board type is
> > > > > > > implied by the 'role'.
> > > > > > >
> > > > > > > For boards which have multiple identities (e.g. can support two
> > > > > > > different board types), Labgrid handles acquiring and releasing the
> > > > > > > shares resources, to avoid any problems.
> > > > > >
> > > > > > I guess I have two sets of questions. First, if it's basically the
> > > > > > board identity why can't we just use that as the role name, in your
> > > > > > setup?
> > > > >
> > > > > Yes, that's what I am doing. If you look in console.labgrid you can
> > > > > see that it is passing U_BOOT_BOARD_IDENTITY as the -r argument.
> > > >
> > > > Then why do we need this?
> > >
> > > We need to pass a role to Labgrid, since it determines the board
> > > identity to use. It also (indirectly) determines the U-Boot build to
> > > use, since each board identity / role is a particular board with a
> > > particular build.
> >
> > Oh, I get where you're coming from now at least. But this still sounds
> > like a detail to put in to the conf.${board}_${board_type} file and not
> > a thing to set here?
>
> There are no such files with the Labgrid integration so far. They are
> not needed.
They're needed in my case since I do not (cannot) use buildman to then
kick off the tests.
[snip]
> > > Basically, as I understand it, the 'role' is the thing we want.
> > >
> > > Labgrid environment:
> > >
> > > samus:
> > > resources:
> > > RemotePlace:
> > > name: samus
> > > ...
> > > UBootProviderDriver:
> > > board: chromebook_samus
> > > binman_indir: /vid/software/devel/samus/bin
> > >
> > > samus_tpl:
> > > resources:
> > > RemotePlace:
> > > name: samus
> > > UBootProviderDriver:
> > > board: chromebook_samus_tpl
> > > binman_indir: /vid/software/devel/samus/bin
> >
> > I guess the problem here is that from my point of view, this can live in
> > the u-boot-test-hooks/bin/<host>/conf.<machine> file since we're never
> > going to worry about building U-Boot (even if blobs aren't a problem, we
> > want to enable more features to test more things on HW) but from your
> > point of view, buildman must provide test.py with the correct build so
> > we need to know things prior.
>
> Well, either you already have a build to test, iwc it is fine, or if
> you don't you can pass --build to force a build, or rely on Labgrid to
> initiate the build.
No, neither buildman nor labgrid can initiate a functional build. Have
you integrated the beagleplay in to your lab? That I believe
demonstrates one of the problems (you need to build both
am62x_beagleplay_a53 and am62x_beagleplay_r5 and write files from both,
to test a given rev on the platform).
> But in your case, the build must be done before running test.py since
> it needs the .config file.
Yes, I build first and pass test.py the build directory.
> > > > > > But second, I'm not sure why we need this. The labgrid support we worked
> > > > > > up here (but, sigh, it's not really clean enough to post) has:
> > > > > > $ cat bin/lootbox/conf.rpi_4_na
> > > > > > console_impl=labgrid
> > > > > > reset_impl=labgrid
> > > > > > flash_impl=labgrid.rpi_4
> > > > > > flash_writer=labgrid.rpi_4
> > > > > >
> > > > > > For example and:
> > > > > > $ cat bin/writer.labgrid.rpi_4
> > > > > > set -e
> > > > > >
> > > > > > build=${U_BOOT_BUILD_DIR}
> > > > > >
> > > > > > $LG_CLIENT write-files -T ${build}/u-boot.bin kernel8.img
> > > > > >
> > > > > > So I don't know what the "role" part you have is for. And yes, there end
> > > > > > up being multiple writer.labgrid.FOO scripts because for TI K3 platforms
> > > > > > (such as beagleplay) we have:
> > > > > > $ cat bin/writer.labgrid.ti-k3
> > > > > > set -e
> > > > > > build=${U_BOOT_BUILD_DIR}
> > > > > >
> > > > > > if [ -z "${tispl}" -o -z "${uboot}" -o -z "${tiboot3}" ]; then
> > > > > > echo "Must configure tispl, uboot, tiboot3 and optionally sysfw"
> > > > > > echo "per the board documentation."
> > > > > > exit 1
> > > > > > fi
> > > > > > echo "Writing build at ${build}"
> > > > > > $LG_CLIENT write-files -T ${build}/${tispl} tispl.bin
> > > > > > $LG_CLIENT write-files -T ${build}/${uboot} u-boot.img
> > > > > > $LG_CLIENT write-files -T ${build/_a??/_r5}/${tiboot3} tiboot3.bin
> > > > > > echo "Done writing build"
> > > > > >
> > > > > > In all cases we first build U-Boot and then pass the build directory to
> > > > > > test.py, in something like:
> > > > > > export LG_PLACE=rpi4
> > > > > > ./test/py/test.py -ra --bd rpi_4 --build-dir .../build-rpi4 \
> > > > > > --results-dir .../results-rpi4 --persistent-data-dir .../pd-rpi4 \
> > > > > > --exitfirst
> > > > > >
> > > > > > The only U-Boot side changes I needed to make were for counting the SPL
> > > > > > banner instances, and that was regardless of framework (a particularly
> > > > > > fun platform will show it 3 times).
> > > > >
> > > > > Yes it is possible to build U-Boot separately. Of course that involved
> > > > > various blobs and so on, so every board is different. The approach I
> > > > > have taken here is to have Labgrid call buildman to build what is
> > > > > needed, with the blobs defined in the environment.
> > > > >
> > > > > You can use the -B flag to use a pre-built U-Boot, with the scripts
> > > > > I've included.
> > > >
> > > > OK. I personally think pre-built (or outside of buildman built) U-Boot
> > > > will be an important case, since everything needs more options enabled
> > > > to test more features, but on real hardware. For example,
> > > > CONFIG_UNIT_TEST should be on for everyone, in testing, once the build
> > > > issues are resolved. And I need to add CONFIG_FIT to some platforms so
> > > > that I can then use the kernel test. And some platforms I end up
> > > > enabling CONFIG_CMD_BOOTEFI_HELLO on (but others disabling
> > > > CONFIG_CMD_BOOTEFI_SELFTEST as that fails and that's just A Thing).
> > >
> > > Yes that all sounds good. I have figured out how to add QEMU into this
> > > Labgrid integration, but I cannot Debian to boot all the way to a
> > > prompt with -nographic unless I pass something special on the Linux
> > > commandline. So for now I parked that.
> >
> > Putting QEMU in to labgrid too could be interesting, yes. But I lost how
> > it's related? To be clear, today we can test boot a Linux kernel on
> > hardware. Somewhere on my TODO list is cycling over what kernel images
> > to grab and shove in to the docker container so that our existing QEMU
> > tests can do that too, for some platforms at least.
>
> It's just a nice way of allowing 'ub-int qemu-x86' and getting to a
> U-Boot prompt. Yes there are other ways to do it, and in fact it works
> today if you set up your conf files for the machine you are on.
Yes, I've locally included qemu hosts as needed. I guess this was just
as an aside? Because yes, it would be good to run the net_boot tests on
more platforms, automatically, including/especially QEMU.
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH v3 08/19] test: Introduce the concept of a role
2024-07-13 16:57 ` Tom Rini
@ 2024-07-15 7:11 ` Simon Glass
2024-07-15 21:03 ` Tom Rini
0 siblings, 1 reply; 47+ messages in thread
From: Simon Glass @ 2024-07-15 7:11 UTC (permalink / raw)
To: Tom Rini; +Cc: U-Boot Mailing List
Hi Tom,
On Sat, 13 Jul 2024 at 17:57, Tom Rini <trini@konsulko.com> wrote:
>
> On Sat, Jul 13, 2024 at 04:13:55PM +0100, Simon Glass wrote:
> > Hi Tom,
> >
> > On Wed, 3 Jul 2024 at 00:12, Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Thu, Jun 27, 2024 at 09:37:18AM +0100, Simon Glass wrote:
> > > > Hi Tom,
> > > >
> > > > On Wed, 26 Jun 2024 at 15:29, Tom Rini <trini@konsulko.com> wrote:
> > > > >
> > > > > On Wed, Jun 26, 2024 at 09:00:33AM +0100, Simon Glass wrote:
> > > > > > Hi Tom,
> > > > > >
> > > > > > On Tue, 25 Jun 2024 at 15:27, Tom Rini <trini@konsulko.com> wrote:
> > > > > > >
> > > > > > > On Tue, Jun 25, 2024 at 01:38:08PM +0100, Simon Glass wrote:
> > > > > > > > Hi Tom,
> > > > > > > >
> > > > > > > > On Mon, 24 Jun 2024 at 19:13, Tom Rini <trini@konsulko.com> wrote:
> > > > > > > > >
> > > > > > > > > On Sun, Jun 23, 2024 at 02:32:02PM -0600, Simon Glass wrote:
> > > > > > > > >
> > > > > > > > > > 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
> > > > > > > > >
> > > > > > > > > I'm a little confused here. Why can't we construct "role" from
> > > > > > > > > board_type+board_identity and then we have the board
> > > > > > > > > conf.${board_type}_${board_identity} file set whatever needs setting to
> > > > > > > > > be "labgrid" ?
> > > > > > > >
> > > > > > > > The role is equivalent to the board identity, not the combination of
> > > > > > > > the U-Boot board type and the board identity. I went this way to avoid
> > > > > > > > having to type long and complicated roles when connecting to boards.
> > > > > > > > It is similar to how things work today, except that the board type is
> > > > > > > > implied by the 'role'.
> > > > > > > >
> > > > > > > > For boards which have multiple identities (e.g. can support two
> > > > > > > > different board types), Labgrid handles acquiring and releasing the
> > > > > > > > shares resources, to avoid any problems.
> > > > > > >
> > > > > > > I guess I have two sets of questions. First, if it's basically the
> > > > > > > board identity why can't we just use that as the role name, in your
> > > > > > > setup?
> > > > > >
> > > > > > Yes, that's what I am doing. If you look in console.labgrid you can
> > > > > > see that it is passing U_BOOT_BOARD_IDENTITY as the -r argument.
> > > > >
> > > > > Then why do we need this?
> > > >
> > > > We need to pass a role to Labgrid, since it determines the board
> > > > identity to use. It also (indirectly) determines the U-Boot build to
> > > > use, since each board identity / role is a particular board with a
> > > > particular build.
> > >
> > > Oh, I get where you're coming from now at least. But this still sounds
> > > like a detail to put in to the conf.${board}_${board_type} file and not
> > > a thing to set here?
> >
> > There are no such files with the Labgrid integration so far. They are
> > not needed.
>
> They're needed in my case since I do not (cannot) use buildman to then
> kick off the tests.
OK...is your environment upstream so I can compare with mine?
>
> [snip]
> > > > Basically, as I understand it, the 'role' is the thing we want.
> > > >
> > > > Labgrid environment:
> > > >
> > > > samus:
> > > > resources:
> > > > RemotePlace:
> > > > name: samus
> > > > ...
> > > > UBootProviderDriver:
> > > > board: chromebook_samus
> > > > binman_indir: /vid/software/devel/samus/bin
> > > >
> > > > samus_tpl:
> > > > resources:
> > > > RemotePlace:
> > > > name: samus
> > > > UBootProviderDriver:
> > > > board: chromebook_samus_tpl
> > > > binman_indir: /vid/software/devel/samus/bin
> > >
> > > I guess the problem here is that from my point of view, this can live in
> > > the u-boot-test-hooks/bin/<host>/conf.<machine> file since we're never
> > > going to worry about building U-Boot (even if blobs aren't a problem, we
> > > want to enable more features to test more things on HW) but from your
> > > point of view, buildman must provide test.py with the correct build so
> > > we need to know things prior.
> >
> > Well, either you already have a build to test, iwc it is fine, or if
> > you don't you can pass --build to force a build, or rely on Labgrid to
> > initiate the build.
>
> No, neither buildman nor labgrid can initiate a functional build. Have
> you integrated the beagleplay in to your lab? That I believe
> demonstrates one of the problems (you need to build both
> am62x_beagleplay_a53 and am62x_beagleplay_r5 and write files from both,
> to test a given rev on the platform).
Actually I was about to do that. Will get back to it in a few weeks.
Labgrid can initiate two builds and copy files from both.
>
> > But in your case, the build must be done before running test.py since
> > it needs the .config file.
>
> Yes, I build first and pass test.py the build directory.
>
> > > > > > > But second, I'm not sure why we need this. The labgrid support we worked
> > > > > > > up here (but, sigh, it's not really clean enough to post) has:
> > > > > > > $ cat bin/lootbox/conf.rpi_4_na
> > > > > > > console_impl=labgrid
> > > > > > > reset_impl=labgrid
> > > > > > > flash_impl=labgrid.rpi_4
> > > > > > > flash_writer=labgrid.rpi_4
> > > > > > >
> > > > > > > For example and:
> > > > > > > $ cat bin/writer.labgrid.rpi_4
> > > > > > > set -e
> > > > > > >
> > > > > > > build=${U_BOOT_BUILD_DIR}
> > > > > > >
> > > > > > > $LG_CLIENT write-files -T ${build}/u-boot.bin kernel8.img
> > > > > > >
> > > > > > > So I don't know what the "role" part you have is for. And yes, there end
> > > > > > > up being multiple writer.labgrid.FOO scripts because for TI K3 platforms
> > > > > > > (such as beagleplay) we have:
> > > > > > > $ cat bin/writer.labgrid.ti-k3
> > > > > > > set -e
> > > > > > > build=${U_BOOT_BUILD_DIR}
> > > > > > >
> > > > > > > if [ -z "${tispl}" -o -z "${uboot}" -o -z "${tiboot3}" ]; then
> > > > > > > echo "Must configure tispl, uboot, tiboot3 and optionally sysfw"
> > > > > > > echo "per the board documentation."
> > > > > > > exit 1
> > > > > > > fi
> > > > > > > echo "Writing build at ${build}"
> > > > > > > $LG_CLIENT write-files -T ${build}/${tispl} tispl.bin
> > > > > > > $LG_CLIENT write-files -T ${build}/${uboot} u-boot.img
> > > > > > > $LG_CLIENT write-files -T ${build/_a??/_r5}/${tiboot3} tiboot3.bin
> > > > > > > echo "Done writing build"
> > > > > > >
> > > > > > > In all cases we first build U-Boot and then pass the build directory to
> > > > > > > test.py, in something like:
> > > > > > > export LG_PLACE=rpi4
> > > > > > > ./test/py/test.py -ra --bd rpi_4 --build-dir .../build-rpi4 \
> > > > > > > --results-dir .../results-rpi4 --persistent-data-dir .../pd-rpi4 \
> > > > > > > --exitfirst
> > > > > > >
> > > > > > > The only U-Boot side changes I needed to make were for counting the SPL
> > > > > > > banner instances, and that was regardless of framework (a particularly
> > > > > > > fun platform will show it 3 times).
> > > > > >
> > > > > > Yes it is possible to build U-Boot separately. Of course that involved
> > > > > > various blobs and so on, so every board is different. The approach I
> > > > > > have taken here is to have Labgrid call buildman to build what is
> > > > > > needed, with the blobs defined in the environment.
> > > > > >
> > > > > > You can use the -B flag to use a pre-built U-Boot, with the scripts
> > > > > > I've included.
> > > > >
> > > > > OK. I personally think pre-built (or outside of buildman built) U-Boot
> > > > > will be an important case, since everything needs more options enabled
> > > > > to test more features, but on real hardware. For example,
> > > > > CONFIG_UNIT_TEST should be on for everyone, in testing, once the build
> > > > > issues are resolved. And I need to add CONFIG_FIT to some platforms so
> > > > > that I can then use the kernel test. And some platforms I end up
> > > > > enabling CONFIG_CMD_BOOTEFI_HELLO on (but others disabling
> > > > > CONFIG_CMD_BOOTEFI_SELFTEST as that fails and that's just A Thing).
> > > >
> > > > Yes that all sounds good. I have figured out how to add QEMU into this
> > > > Labgrid integration, but I cannot Debian to boot all the way to a
> > > > prompt with -nographic unless I pass something special on the Linux
> > > > commandline. So for now I parked that.
> > >
> > > Putting QEMU in to labgrid too could be interesting, yes. But I lost how
> > > it's related? To be clear, today we can test boot a Linux kernel on
> > > hardware. Somewhere on my TODO list is cycling over what kernel images
> > > to grab and shove in to the docker container so that our existing QEMU
> > > tests can do that too, for some platforms at least.
> >
> > It's just a nice way of allowing 'ub-int qemu-x86' and getting to a
> > U-Boot prompt. Yes there are other ways to do it, and in fact it works
> > today if you set up your conf files for the machine you are on.
>
> Yes, I've locally included qemu hosts as needed. I guess this was just
> as an aside? Because yes, it would be good to run the net_boot tests on
> more platforms, automatically, including/especially QEMU.
Indeed.
Regards,
SImon
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH v3 08/19] test: Introduce the concept of a role
2024-07-15 7:11 ` Simon Glass
@ 2024-07-15 21:03 ` Tom Rini
2024-07-31 14:39 ` Simon Glass
0 siblings, 1 reply; 47+ messages in thread
From: Tom Rini @ 2024-07-15 21:03 UTC (permalink / raw)
To: Simon Glass; +Cc: U-Boot Mailing List
[-- Attachment #1: Type: text/plain, Size: 11310 bytes --]
On Mon, Jul 15, 2024 at 08:11:28AM +0100, Simon Glass wrote:
> Hi Tom,
>
> On Sat, 13 Jul 2024 at 17:57, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Sat, Jul 13, 2024 at 04:13:55PM +0100, Simon Glass wrote:
> > > Hi Tom,
> > >
> > > On Wed, 3 Jul 2024 at 00:12, Tom Rini <trini@konsulko.com> wrote:
> > > >
> > > > On Thu, Jun 27, 2024 at 09:37:18AM +0100, Simon Glass wrote:
> > > > > Hi Tom,
> > > > >
> > > > > On Wed, 26 Jun 2024 at 15:29, Tom Rini <trini@konsulko.com> wrote:
> > > > > >
> > > > > > On Wed, Jun 26, 2024 at 09:00:33AM +0100, Simon Glass wrote:
> > > > > > > Hi Tom,
> > > > > > >
> > > > > > > On Tue, 25 Jun 2024 at 15:27, Tom Rini <trini@konsulko.com> wrote:
> > > > > > > >
> > > > > > > > On Tue, Jun 25, 2024 at 01:38:08PM +0100, Simon Glass wrote:
> > > > > > > > > Hi Tom,
> > > > > > > > >
> > > > > > > > > On Mon, 24 Jun 2024 at 19:13, Tom Rini <trini@konsulko.com> wrote:
> > > > > > > > > >
> > > > > > > > > > On Sun, Jun 23, 2024 at 02:32:02PM -0600, Simon Glass wrote:
> > > > > > > > > >
> > > > > > > > > > > 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
> > > > > > > > > >
> > > > > > > > > > I'm a little confused here. Why can't we construct "role" from
> > > > > > > > > > board_type+board_identity and then we have the board
> > > > > > > > > > conf.${board_type}_${board_identity} file set whatever needs setting to
> > > > > > > > > > be "labgrid" ?
> > > > > > > > >
> > > > > > > > > The role is equivalent to the board identity, not the combination of
> > > > > > > > > the U-Boot board type and the board identity. I went this way to avoid
> > > > > > > > > having to type long and complicated roles when connecting to boards.
> > > > > > > > > It is similar to how things work today, except that the board type is
> > > > > > > > > implied by the 'role'.
> > > > > > > > >
> > > > > > > > > For boards which have multiple identities (e.g. can support two
> > > > > > > > > different board types), Labgrid handles acquiring and releasing the
> > > > > > > > > shares resources, to avoid any problems.
> > > > > > > >
> > > > > > > > I guess I have two sets of questions. First, if it's basically the
> > > > > > > > board identity why can't we just use that as the role name, in your
> > > > > > > > setup?
> > > > > > >
> > > > > > > Yes, that's what I am doing. If you look in console.labgrid you can
> > > > > > > see that it is passing U_BOOT_BOARD_IDENTITY as the -r argument.
> > > > > >
> > > > > > Then why do we need this?
> > > > >
> > > > > We need to pass a role to Labgrid, since it determines the board
> > > > > identity to use. It also (indirectly) determines the U-Boot build to
> > > > > use, since each board identity / role is a particular board with a
> > > > > particular build.
> > > >
> > > > Oh, I get where you're coming from now at least. But this still sounds
> > > > like a detail to put in to the conf.${board}_${board_type} file and not
> > > > a thing to set here?
> > >
> > > There are no such files with the Labgrid integration so far. They are
> > > not needed.
> >
> > They're needed in my case since I do not (cannot) use buildman to then
> > kick off the tests.
>
> OK...is your environment upstream so I can compare with mine?
The engineer here that was working on it is unfortunately leaving
shortly and I forget if he got everything labgrid related posted. The
other half of the environment is that none of my tests treat the hooks
repository any different than before. And note that when I say cannot
above I mean that because:
1) All of the TI platforms that require an Cortex-R and Cortex-A builds.
You can (nominally) stick with only upgrading one part at a time, and so
just test an even smaller subset on the R core, and once that passes,
test the subset of tests that run on HW on the A core.
2) Enable further options. I enable CMD_BOOTMENU, CMD_LOG globally,
CMD_TFTPPUT and FIT+FIT_SIGNATURE globally and BOOTSTAGE (+ stash) on
Pi, and I'm going to cover TI K3 platforms next (since they too can
easily share a stash addr).
The latter could be solved if buildman had some native config-fragment
support and we didn't have the #include games we have today. No, I don't
have a good idea on solving that either, only noting that today I use
scripts/kconfig/merge_config.sh to combine defconfig + the above.
> > [snip]
> > > > > Basically, as I understand it, the 'role' is the thing we want.
> > > > >
> > > > > Labgrid environment:
> > > > >
> > > > > samus:
> > > > > resources:
> > > > > RemotePlace:
> > > > > name: samus
> > > > > ...
> > > > > UBootProviderDriver:
> > > > > board: chromebook_samus
> > > > > binman_indir: /vid/software/devel/samus/bin
> > > > >
> > > > > samus_tpl:
> > > > > resources:
> > > > > RemotePlace:
> > > > > name: samus
> > > > > UBootProviderDriver:
> > > > > board: chromebook_samus_tpl
> > > > > binman_indir: /vid/software/devel/samus/bin
> > > >
> > > > I guess the problem here is that from my point of view, this can live in
> > > > the u-boot-test-hooks/bin/<host>/conf.<machine> file since we're never
> > > > going to worry about building U-Boot (even if blobs aren't a problem, we
> > > > want to enable more features to test more things on HW) but from your
> > > > point of view, buildman must provide test.py with the correct build so
> > > > we need to know things prior.
> > >
> > > Well, either you already have a build to test, iwc it is fine, or if
> > > you don't you can pass --build to force a build, or rely on Labgrid to
> > > initiate the build.
> >
> > No, neither buildman nor labgrid can initiate a functional build. Have
> > you integrated the beagleplay in to your lab? That I believe
> > demonstrates one of the problems (you need to build both
> > am62x_beagleplay_a53 and am62x_beagleplay_r5 and write files from both,
> > to test a given rev on the platform).
>
> Actually I was about to do that. Will get back to it in a few weeks.
> Labgrid can initiate two builds and copy files from both.
I'm very interested in what this all looks like once that works too.
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH v3 08/19] test: Introduce the concept of a role
2024-07-15 21:03 ` Tom Rini
@ 2024-07-31 14:39 ` Simon Glass
0 siblings, 0 replies; 47+ messages in thread
From: Simon Glass @ 2024-07-31 14:39 UTC (permalink / raw)
To: Tom Rini; +Cc: U-Boot Mailing List
Hi Tom,
On Mon, 15 Jul 2024 at 15:03, Tom Rini <trini@konsulko.com> wrote:
>
> On Mon, Jul 15, 2024 at 08:11:28AM +0100, Simon Glass wrote:
> > Hi Tom,
> >
> > On Sat, 13 Jul 2024 at 17:57, Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Sat, Jul 13, 2024 at 04:13:55PM +0100, Simon Glass wrote:
> > > > Hi Tom,
> > > >
> > > > On Wed, 3 Jul 2024 at 00:12, Tom Rini <trini@konsulko.com> wrote:
> > > > >
> > > > > On Thu, Jun 27, 2024 at 09:37:18AM +0100, Simon Glass wrote:
> > > > > > Hi Tom,
> > > > > >
> > > > > > On Wed, 26 Jun 2024 at 15:29, Tom Rini <trini@konsulko.com> wrote:
> > > > > > >
> > > > > > > On Wed, Jun 26, 2024 at 09:00:33AM +0100, Simon Glass wrote:
> > > > > > > > Hi Tom,
> > > > > > > >
> > > > > > > > On Tue, 25 Jun 2024 at 15:27, Tom Rini <trini@konsulko.com> wrote:
> > > > > > > > >
> > > > > > > > > On Tue, Jun 25, 2024 at 01:38:08PM +0100, Simon Glass wrote:
> > > > > > > > > > Hi Tom,
> > > > > > > > > >
> > > > > > > > > > On Mon, 24 Jun 2024 at 19:13, Tom Rini <trini@konsulko.com> wrote:
> > > > > > > > > > >
> > > > > > > > > > > On Sun, Jun 23, 2024 at 02:32:02PM -0600, Simon Glass wrote:
> > > > > > > > > > >
> > > > > > > > > > > > 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
> > > > > > > > > > >
> > > > > > > > > > > I'm a little confused here. Why can't we construct "role" from
> > > > > > > > > > > board_type+board_identity and then we have the board
> > > > > > > > > > > conf.${board_type}_${board_identity} file set whatever needs setting to
> > > > > > > > > > > be "labgrid" ?
> > > > > > > > > >
> > > > > > > > > > The role is equivalent to the board identity, not the combination of
> > > > > > > > > > the U-Boot board type and the board identity. I went this way to avoid
> > > > > > > > > > having to type long and complicated roles when connecting to boards.
> > > > > > > > > > It is similar to how things work today, except that the board type is
> > > > > > > > > > implied by the 'role'.
> > > > > > > > > >
> > > > > > > > > > For boards which have multiple identities (e.g. can support two
> > > > > > > > > > different board types), Labgrid handles acquiring and releasing the
> > > > > > > > > > shares resources, to avoid any problems.
> > > > > > > > >
> > > > > > > > > I guess I have two sets of questions. First, if it's basically the
> > > > > > > > > board identity why can't we just use that as the role name, in your
> > > > > > > > > setup?
> > > > > > > >
> > > > > > > > Yes, that's what I am doing. If you look in console.labgrid you can
> > > > > > > > see that it is passing U_BOOT_BOARD_IDENTITY as the -r argument.
> > > > > > >
> > > > > > > Then why do we need this?
> > > > > >
> > > > > > We need to pass a role to Labgrid, since it determines the board
> > > > > > identity to use. It also (indirectly) determines the U-Boot build to
> > > > > > use, since each board identity / role is a particular board with a
> > > > > > particular build.
> > > > >
> > > > > Oh, I get where you're coming from now at least. But this still sounds
> > > > > like a detail to put in to the conf.${board}_${board_type} file and not
> > > > > a thing to set here?
> > > >
> > > > There are no such files with the Labgrid integration so far. They are
> > > > not needed.
> > >
> > > They're needed in my case since I do not (cannot) use buildman to then
> > > kick off the tests.
> >
> > OK...is your environment upstream so I can compare with mine?
>
> The engineer here that was working on it is unfortunately leaving
> shortly and I forget if he got everything labgrid related posted. The
> other half of the environment is that none of my tests treat the hooks
> repository any different than before. And note that when I say cannot
> above I mean that because:
> 1) All of the TI platforms that require an Cortex-R and Cortex-A builds.
> You can (nominally) stick with only upgrading one part at a time, and so
> just test an even smaller subset on the R core, and once that passes,
> test the subset of tests that run on HW on the A core.
> 2) Enable further options. I enable CMD_BOOTMENU, CMD_LOG globally,
> CMD_TFTPPUT and FIT+FIT_SIGNATURE globally and BOOTSTAGE (+ stash) on
> Pi, and I'm going to cover TI K3 platforms next (since they too can
> easily share a stash addr).
OK I see. To support that with my integration I would need to provide
a way to enable config options in the Labgrid environment. I suppose
that is pretty easy.
>
> The latter could be solved if buildman had some native config-fragment
> support and we didn't have the #include games we have today. No, I don't
> have a good idea on solving that either, only noting that today I use
> scripts/kconfig/merge_config.sh to combine defconfig + the above.
We have an issue for config fragments [1] but I haven't looked at it,
other than proposing a possible option.
>
>
> > > [snip]
> > > > > > Basically, as I understand it, the 'role' is the thing we want.
> > > > > >
> > > > > > Labgrid environment:
> > > > > >
> > > > > > samus:
> > > > > > resources:
> > > > > > RemotePlace:
> > > > > > name: samus
> > > > > > ...
> > > > > > UBootProviderDriver:
> > > > > > board: chromebook_samus
> > > > > > binman_indir: /vid/software/devel/samus/bin
> > > > > >
> > > > > > samus_tpl:
> > > > > > resources:
> > > > > > RemotePlace:
> > > > > > name: samus
> > > > > > UBootProviderDriver:
> > > > > > board: chromebook_samus_tpl
> > > > > > binman_indir: /vid/software/devel/samus/bin
> > > > >
> > > > > I guess the problem here is that from my point of view, this can live in
> > > > > the u-boot-test-hooks/bin/<host>/conf.<machine> file since we're never
> > > > > going to worry about building U-Boot (even if blobs aren't a problem, we
> > > > > want to enable more features to test more things on HW) but from your
> > > > > point of view, buildman must provide test.py with the correct build so
> > > > > we need to know things prior.
> > > >
> > > > Well, either you already have a build to test, iwc it is fine, or if
> > > > you don't you can pass --build to force a build, or rely on Labgrid to
> > > > initiate the build.
> > >
> > > No, neither buildman nor labgrid can initiate a functional build. Have
> > > you integrated the beagleplay in to your lab? That I believe
> > > demonstrates one of the problems (you need to build both
> > > am62x_beagleplay_a53 and am62x_beagleplay_r5 and write files from both,
> > > to test a given rev on the platform).
> >
> > Actually I was about to do that. Will get back to it in a few weeks.
> > Labgrid can initiate two builds and copy files from both.
>
> I'm very interested in what this all looks like once that works too.
OK, I pushed it to [2]. There are no changes to the U-Boot side though.
>
> --
> Tom
Regards,
Simon
[1] https://source.denx.de/u-boot/custodians/u-boot-dm/-/issues/9
[2] https://github.com/labgrid-project/labgrid/pull/1411
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH v3 09/19] test: Move the receive code into a function
2024-06-23 20:31 [PATCH v3 00/19] labgrid: Provide an integration with Labgrid Simon Glass
` (7 preceding siblings ...)
2024-06-23 20:32 ` [PATCH v3 08/19] test: Introduce the concept of a role Simon Glass
@ 2024-06-23 20:32 ` Simon Glass
2024-06-23 20:32 ` [PATCH v3 10/19] test: Separate out the exception handling Simon Glass
` (9 subsequent siblings)
18 siblings, 0 replies; 47+ messages in thread
From: Simon Glass @ 2024-06-23 20:32 UTC (permalink / raw)
To: U-Boot Mailing List; +Cc: Tom Rini, Simon Glass
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
--
2.34.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH v3 10/19] test: Separate out the exception handling
2024-06-23 20:31 [PATCH v3 00/19] labgrid: Provide an integration with Labgrid Simon Glass
` (8 preceding siblings ...)
2024-06-23 20:32 ` [PATCH v3 09/19] test: Move the receive code into a function Simon Glass
@ 2024-06-23 20:32 ` Simon Glass
2024-06-23 20:32 ` [PATCH v3 11/19] test: Detect dead connections Simon Glass
` (8 subsequent siblings)
18 siblings, 0 replies; 47+ messages in thread
From: Simon Glass @ 2024-06-23 20:32 UTC (permalink / raw)
To: U-Boot Mailing List; +Cc: Tom Rini, Simon Glass, Love Kumar
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.
--
2.34.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH v3 11/19] test: Detect dead connections
2024-06-23 20:31 [PATCH v3 00/19] labgrid: Provide an integration with Labgrid Simon Glass
` (9 preceding siblings ...)
2024-06-23 20:32 ` [PATCH v3 10/19] test: Separate out the exception handling Simon Glass
@ 2024-06-23 20:32 ` Simon Glass
2024-06-23 20:32 ` [PATCH v3 12/19] test: Tidy up remaining exceptions Simon Glass
` (7 subsequent siblings)
18 siblings, 0 replies; 47+ messages in thread
From: Simon Glass @ 2024-06-23 20:32 UTC (permalink / raw)
To: U-Boot Mailing List; +Cc: Tom Rini, Simon Glass
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.
--
2.34.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH v3 12/19] test: Tidy up remaining exceptions
2024-06-23 20:31 [PATCH v3 00/19] labgrid: Provide an integration with Labgrid Simon Glass
` (10 preceding siblings ...)
2024-06-23 20:32 ` [PATCH v3 11/19] test: Detect dead connections Simon Glass
@ 2024-06-23 20:32 ` Simon Glass
2024-06-23 20:32 ` [PATCH v3 13/19] test: Introduce lab mode Simon Glass
` (6 subsequent siblings)
18 siblings, 0 replies; 47+ messages in thread
From: Simon Glass @ 2024-06-23 20:32 UTC (permalink / raw)
To: U-Boot Mailing List; +Cc: Tom Rini, Simon Glass, Love Kumar
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()
--
2.34.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH v3 13/19] test: Introduce lab mode
2024-06-23 20:31 [PATCH v3 00/19] labgrid: Provide an integration with Labgrid Simon Glass
` (11 preceding siblings ...)
2024-06-23 20:32 ` [PATCH v3 12/19] test: Tidy up remaining exceptions Simon Glass
@ 2024-06-23 20:32 ` Simon Glass
2024-06-23 20:32 ` [PATCH v3 14/19] test: Improve handling of sending commands Simon Glass
` (5 subsequent siblings)
18 siblings, 0 replies; 47+ messages in thread
From: Simon Glass @ 2024-06-23 20:32 UTC (permalink / raw)
To: U-Boot Mailing List; +Cc: Tom Rini, Simon Glass, Love Kumar
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()
--
2.34.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH v3 14/19] test: Improve handling of sending commands
2024-06-23 20:31 [PATCH v3 00/19] labgrid: Provide an integration with Labgrid Simon Glass
` (12 preceding siblings ...)
2024-06-23 20:32 ` [PATCH v3 13/19] test: Introduce lab mode Simon Glass
@ 2024-06-23 20:32 ` Simon Glass
2024-06-23 20:32 ` [PATCH v3 15/19] test: Fix mulptiplex_log typo Simon Glass
` (4 subsequent siblings)
18 siblings, 0 replies; 47+ messages in thread
From: Simon Glass @ 2024-06-23 20:32 UTC (permalink / raw)
To: U-Boot Mailing List; +Cc: Tom Rini, Simon Glass, Love Kumar
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:
--
2.34.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH v3 15/19] test: Fix mulptiplex_log typo
2024-06-23 20:31 [PATCH v3 00/19] labgrid: Provide an integration with Labgrid Simon Glass
` (13 preceding siblings ...)
2024-06-23 20:32 ` [PATCH v3 14/19] test: Improve handling of sending commands Simon Glass
@ 2024-06-23 20:32 ` Simon Glass
2024-06-23 20:32 ` [PATCH v3 16/19] test: Avoid double echo when starting up Simon Glass
` (3 subsequent siblings)
18 siblings, 0 replies; 47+ messages in thread
From: Simon Glass @ 2024-06-23 20:32 UTC (permalink / raw)
To: U-Boot Mailing List; +Cc: Tom Rini, Simon Glass, Love Kumar
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
--
2.34.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH v3 16/19] test: Avoid double echo when starting up
2024-06-23 20:31 [PATCH v3 00/19] labgrid: Provide an integration with Labgrid Simon Glass
` (14 preceding siblings ...)
2024-06-23 20:32 ` [PATCH v3 15/19] test: Fix mulptiplex_log typo Simon Glass
@ 2024-06-23 20:32 ` Simon Glass
2024-06-23 20:32 ` [PATCH v3 17/19] test: Try to shut down the lab console gracefully Simon Glass
` (2 subsequent siblings)
18 siblings, 0 replies; 47+ messages in thread
From: Simon Glass @ 2024-06-23 20:32 UTC (permalink / raw)
To: U-Boot Mailing List; +Cc: Tom Rini, Simon Glass
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
--
2.34.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH v3 17/19] test: Try to shut down the lab console gracefully
2024-06-23 20:31 [PATCH v3 00/19] labgrid: Provide an integration with Labgrid Simon Glass
` (15 preceding siblings ...)
2024-06-23 20:32 ` [PATCH v3 16/19] test: Avoid double echo when starting up Simon Glass
@ 2024-06-23 20:32 ` Simon Glass
2024-06-23 20:32 ` [PATCH v3 18/19] test: Add a section for closing the connection Simon Glass
2024-06-23 20:32 ` [PATCH v3 19/19] CI: Allow running tests on sjg lab Simon Glass
18 siblings, 0 replies; 47+ messages in thread
From: Simon Glass @ 2024-06-23 20:32 UTC (permalink / raw)
To: U-Boot Mailing List; +Cc: Tom Rini, Simon Glass
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()
--
2.34.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH v3 18/19] test: Add a section for closing the connection
2024-06-23 20:31 [PATCH v3 00/19] labgrid: Provide an integration with Labgrid Simon Glass
` (16 preceding siblings ...)
2024-06-23 20:32 ` [PATCH v3 17/19] test: Try to shut down the lab console gracefully Simon Glass
@ 2024-06-23 20:32 ` Simon Glass
2024-06-23 20:32 ` [PATCH v3 19/19] CI: Allow running tests on sjg lab Simon Glass
18 siblings, 0 replies; 47+ messages in thread
From: Simon Glass @ 2024-06-23 20:32 UTC (permalink / raw)
To: U-Boot Mailing List; +Cc: Tom Rini, Simon Glass, Love Kumar
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):
--
2.34.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH v3 19/19] CI: Allow running tests on sjg lab
2024-06-23 20:31 [PATCH v3 00/19] labgrid: Provide an integration with Labgrid Simon Glass
` (17 preceding siblings ...)
2024-06-23 20:32 ` [PATCH v3 18/19] test: Add a section for closing the connection Simon Glass
@ 2024-06-23 20:32 ` Simon Glass
2024-06-24 7:13 ` Andrejs Cainikovs
18 siblings, 1 reply; 47+ messages in thread
From: Simon Glass @ 2024-06-23 20:32 UTC (permalink / raw)
To: U-Boot Mailing List
Cc: Tom Rini, Simon Glass, Andrejs Cainikovs, Leo Yu-Chi Liang,
Marek Vasut
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>
---
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 165f765a833..75c18a0f2f7 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
@@ -482,3 +483,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
--
2.34.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [PATCH v3 19/19] CI: Allow running tests on sjg lab
2024-06-23 20:32 ` [PATCH v3 19/19] CI: Allow running tests on sjg lab Simon Glass
@ 2024-06-24 7:13 ` Andrejs Cainikovs
2024-06-24 14:56 ` Michael Nazzareno Trimarchi
0 siblings, 1 reply; 47+ messages in thread
From: Andrejs Cainikovs @ 2024-06-24 7:13 UTC (permalink / raw)
To: Simon Glass; +Cc: U-Boot Mailing List, Tom Rini, Leo Yu-Chi Liang, Marek Vasut
On Sun, Jun 23, 2024 at 02:32:13PM -0600, Simon Glass wrote:
> 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>
> ---
>
> 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 165f765a833..75c18a0f2f7 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
> @@ -482,3 +483,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
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH v3 19/19] CI: Allow running tests on sjg lab
2024-06-24 7:13 ` Andrejs Cainikovs
@ 2024-06-24 14:56 ` Michael Nazzareno Trimarchi
2024-06-24 18:01 ` Tom Rini
0 siblings, 1 reply; 47+ messages in thread
From: Michael Nazzareno Trimarchi @ 2024-06-24 14:56 UTC (permalink / raw)
To: Andrejs Cainikovs
Cc: Simon Glass, U-Boot Mailing List, Tom Rini, Leo Yu-Chi Liang,
Marek Vasut
Hi Simon
On Mon, Jun 24, 2024 at 2:52 PM Andrejs Cainikovs
<andrejs.cainikovs@toradex.com> wrote:
>
> On Sun, Jun 23, 2024 at 02:32:13PM -0600, Simon Glass wrote:
> > 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>
>
Do you have documentation on how to set it? We would like to do in our
company too
Michael
> > ---
> >
> > 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 165f765a833..75c18a0f2f7 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
> > @@ -482,3 +483,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
> > --
> > 2.34.1
> >
>
--
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
michael@amarulasolutions.com
__________________________________
Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
info@amarulasolutions.com
www.amarulasolutions.com
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH v3 19/19] CI: Allow running tests on sjg lab
2024-06-24 14:56 ` Michael Nazzareno Trimarchi
@ 2024-06-24 18:01 ` Tom Rini
2024-06-25 12:30 ` Simon Glass
0 siblings, 1 reply; 47+ messages in thread
From: Tom Rini @ 2024-06-24 18:01 UTC (permalink / raw)
To: Michael Nazzareno Trimarchi
Cc: Andrejs Cainikovs, Simon Glass, U-Boot Mailing List,
Leo Yu-Chi Liang, Marek Vasut
[-- Attachment #1: Type: text/plain, Size: 1332 bytes --]
On Mon, Jun 24, 2024 at 04:56:02PM +0200, Michael Nazzareno Trimarchi wrote:
> Hi Simon
>
> On Mon, Jun 24, 2024 at 2:52 PM Andrejs Cainikovs
> <andrejs.cainikovs@toradex.com> wrote:
> >
> > On Sun, Jun 23, 2024 at 02:32:13PM -0600, Simon Glass wrote:
> > > 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>
> >
>
> Do you have documentation on how to set it? We would like to do in our
> company too
I _think_ from some talking with Simon before the biggest sticking point
might be changes needed on the labgrid side of things. However, that
might also be most easily remedied if there's a few people showing up in
the GitHub issue(s) showing interest in getting changes made/merged and
to use the overall feature.
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v3 19/19] CI: Allow running tests on sjg lab
2024-06-24 18:01 ` Tom Rini
@ 2024-06-25 12:30 ` Simon Glass
2024-08-09 16:10 ` Simon Glass
0 siblings, 1 reply; 47+ messages in thread
From: Simon Glass @ 2024-06-25 12:30 UTC (permalink / raw)
To: Tom Rini
Cc: Michael Nazzareno Trimarchi, Andrejs Cainikovs,
U-Boot Mailing List, Leo Yu-Chi Liang, Marek Vasut
Hi,
On Mon, 24 Jun 2024 at 19:01, Tom Rini <trini@konsulko.com> wrote:
>
> On Mon, Jun 24, 2024 at 04:56:02PM +0200, Michael Nazzareno Trimarchi wrote:
> > Hi Simon
> >
> > On Mon, Jun 24, 2024 at 2:52 PM Andrejs Cainikovs
> > <andrejs.cainikovs@toradex.com> wrote:
> > >
> > > On Sun, Jun 23, 2024 at 02:32:13PM -0600, Simon Glass wrote:
> > > > 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>
> > >
> >
> > Do you have documentation on how to set it? We would like to do in our
> > company too
>
> I _think_ from some talking with Simon before the biggest sticking point
> might be changes needed on the labgrid side of things. However, that
> might also be most easily remedied if there's a few people showing up in
> the GitHub issue(s) showing interest in getting changes made/merged and
> to use the overall feature.
The documentation is in the PR [1] mostly in the last commit [2].
Yes it would really help for you to try it out and comment on the PR.
I may end up splitting it into a few separate PRs, but code review on
the project is very limited, from what I have seen so far. You will
see an example of my lab (devices and environment file).
I also have a few minor updates to the PR which I just uploaded, to
work on top of the grpc branch and to support QEMU.
[1] https://github.com/labgrid-project/labgrid/pull/1411
[2] https://github.com/labgrid-project/labgrid/pull/1411/commits/c4b13af0e6169228c9adef03d4b66401201edd23
Regards,
Simon
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v3 19/19] CI: Allow running tests on sjg lab
2024-06-25 12:30 ` Simon Glass
@ 2024-08-09 16:10 ` Simon Glass
0 siblings, 0 replies; 47+ messages in thread
From: Simon Glass @ 2024-08-09 16:10 UTC (permalink / raw)
To: Tom Rini
Cc: Michael Nazzareno Trimarchi, Andrejs Cainikovs,
U-Boot Mailing List, Leo Yu-Chi Liang, Marek Vasut
Hi Tom,
On Tue, 25 Jun 2024 at 06:30, Simon Glass <sjg@chromium.org> wrote:
>
> Hi,
>
> On Mon, 24 Jun 2024 at 19:01, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Mon, Jun 24, 2024 at 04:56:02PM +0200, Michael Nazzareno Trimarchi wrote:
> > > Hi Simon
> > >
> > > On Mon, Jun 24, 2024 at 2:52 PM Andrejs Cainikovs
> > > <andrejs.cainikovs@toradex.com> wrote:
> > > >
> > > > On Sun, Jun 23, 2024 at 02:32:13PM -0600, Simon Glass wrote:
> > > > > 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>
> > > >
> > >
> > > Do you have documentation on how to set it? We would like to do in our
> > > company too
> >
> > I _think_ from some talking with Simon before the biggest sticking point
> > might be changes needed on the labgrid side of things. However, that
> > might also be most easily remedied if there's a few people showing up in
> > the GitHub issue(s) showing interest in getting changes made/merged and
> > to use the overall feature.
>
> The documentation is in the PR [1] mostly in the last commit [2].
>
> Yes it would really help for you to try it out and comment on the PR.
> I may end up splitting it into a few separate PRs, but code review on
> the project is very limited, from what I have seen so far. You will
> see an example of my lab (devices and environment file).
>
> I also have a few minor updates to the PR which I just uploaded, to
> work on top of the grpc branch and to support QEMU.
Just to mention that I updated the Labgrid integration to support
beagleplay (which as you know combines the U-Boot builds for two
boards). It resulted in no changes at all to this series.
So perhaps this series can be reviewed and some of it applied?
Documentation is below.
>
> [1] https://github.com/labgrid-project/labgrid/pull/1411
> [2] https://github.com/labgrid-project/labgrid/pull/1411/commits/c4b13af0e6169228c9adef03d4b66401201edd23
Regards,
Simon
^ permalink raw reply [flat|nested] 47+ messages in thread