* [PATCH v6 01/19] test: Allow signaling that U-Boot is ready
2024-09-20 6:01 [PATCH v6 00/19] labgrid: Provide an integration with Labgrid Simon Glass
@ 2024-09-20 6:01 ` Simon Glass
2024-09-23 20:35 ` Tom Rini
2024-09-20 6:01 ` [PATCH v6 02/19] test: Use a constant for the test timeout Simon Glass
` (18 subsequent siblings)
19 siblings, 1 reply; 37+ messages in thread
From: Simon Glass @ 2024-09-20 6:01 UTC (permalink / raw)
To: u-boot; +Cc: Tom Rini, Simon Glass
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.43.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [PATCH v6 01/19] test: Allow signaling that U-Boot is ready
2024-09-20 6:01 ` [PATCH v6 01/19] test: Allow signaling that U-Boot is ready Simon Glass
@ 2024-09-23 20:35 ` Tom Rini
2024-09-25 12:49 ` Simon Glass
0 siblings, 1 reply; 37+ messages in thread
From: Tom Rini @ 2024-09-23 20:35 UTC (permalink / raw)
To: Simon Glass; +Cc: u-boot
[-- Attachment #1: Type: text/plain, Size: 769 bytes --]
On Fri, Sep 20, 2024 at 08:01:36AM +0200, Simon Glass wrote:
> 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(-)
What happens is that labgrid can also be told to look for and then
interrupt autoboot, just like our pytests can do, and the system is at
the prompt. But this is also what it's like for a system with autoboot
disabled. Do we actually need this patch to achieve the functionality
you want? Doesn't that already just happen?
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v6 01/19] test: Allow signaling that U-Boot is ready
2024-09-23 20:35 ` Tom Rini
@ 2024-09-25 12:49 ` Simon Glass
2024-09-25 17:26 ` Tom Rini
0 siblings, 1 reply; 37+ messages in thread
From: Simon Glass @ 2024-09-25 12:49 UTC (permalink / raw)
To: Tom Rini; +Cc: u-boot
Hi Tom,
On Mon, 23 Sept 2024 at 22:35, Tom Rini <trini@konsulko.com> wrote:
>
> On Fri, Sep 20, 2024 at 08:01:36AM +0200, Simon Glass wrote:
>
>
> > 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(-)
>
> What happens is that labgrid can also be told to look for and then
> interrupt autoboot, just like our pytests can do, and the system is at
> the prompt. But this is also what it's like for a system with autoboot
> disabled. Do we actually need this patch to achieve the functionality
> you want? Doesn't that already just happen?
The point of this patch is actually to remove code in pytest, by
allowing it to skip all the banner-detection stuff. It does not affect
things in Labgrid, since it still needs to watch for banners, etc.
Without this patch, we have to tell Labgrid's U-Boot driver to do
nothing, so that pytest does it. But that is not a good idea, since
Labgrid has a lot more info about the board than pytest has. For
example, look at all the SPL-banner-count stuff.
Basically, without this patch we cannot use '-s uboot' to tell the
Labgrid strategy to take us to a U-Boot prompt. We must just use a raw
console with no strategy, relying on pytest to do all the work.
I hope that helps explain the problem?
Regards,
Simon
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v6 01/19] test: Allow signaling that U-Boot is ready
2024-09-25 12:49 ` Simon Glass
@ 2024-09-25 17:26 ` Tom Rini
2024-09-26 21:36 ` Simon Glass
0 siblings, 1 reply; 37+ messages in thread
From: Tom Rini @ 2024-09-25 17:26 UTC (permalink / raw)
To: Simon Glass; +Cc: u-boot
[-- Attachment #1: Type: text/plain, Size: 2217 bytes --]
On Wed, Sep 25, 2024 at 02:49:56PM +0200, Simon Glass wrote:
> Hi Tom,
>
> On Mon, 23 Sept 2024 at 22:35, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Fri, Sep 20, 2024 at 08:01:36AM +0200, Simon Glass wrote:
> >
> >
> > > 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(-)
> >
> > What happens is that labgrid can also be told to look for and then
> > interrupt autoboot, just like our pytests can do, and the system is at
> > the prompt. But this is also what it's like for a system with autoboot
> > disabled. Do we actually need this patch to achieve the functionality
> > you want? Doesn't that already just happen?
>
> The point of this patch is actually to remove code in pytest, by
> allowing it to skip all the banner-detection stuff. It does not affect
> things in Labgrid, since it still needs to watch for banners, etc.
But you can't remove code from pytest, people can and will run the suite
outside of labgrid.
> Without this patch, we have to tell Labgrid's U-Boot driver to do
> nothing, so that pytest does it. But that is not a good idea, since
> Labgrid has a lot more info about the board than pytest has. For
> example, look at all the SPL-banner-count stuff.
Why do you have to tell it to do nothing? The pytest suite works fine,
today, if the board stops at the prompt automatically. To be clear, the
labgrid yaml file I'm using with my scripts has the information to stop
autoboot in it and it's not causing a problem.
> Basically, without this patch we cannot use '-s uboot' to tell the
> Labgrid strategy to take us to a U-Boot prompt. We must just use a raw
> console with no strategy, relying on pytest to do all the work.
>
> I hope that helps explain the problem?
I think I see what you're saying, and it's based on the assumption that
we'll make everyone either use labgrid?
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v6 01/19] test: Allow signaling that U-Boot is ready
2024-09-25 17:26 ` Tom Rini
@ 2024-09-26 21:36 ` Simon Glass
2024-09-27 2:51 ` Tom Rini
0 siblings, 1 reply; 37+ messages in thread
From: Simon Glass @ 2024-09-26 21:36 UTC (permalink / raw)
To: Tom Rini; +Cc: u-boot
Hi Tom,
On Wed, 25 Sept 2024 at 19:26, Tom Rini <trini@konsulko.com> wrote:
>
> On Wed, Sep 25, 2024 at 02:49:56PM +0200, Simon Glass wrote:
> > Hi Tom,
> >
> > On Mon, 23 Sept 2024 at 22:35, Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Fri, Sep 20, 2024 at 08:01:36AM +0200, Simon Glass wrote:
> > >
> > >
> > > > 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(-)
> > >
> > > What happens is that labgrid can also be told to look for and then
> > > interrupt autoboot, just like our pytests can do, and the system is at
> > > the prompt. But this is also what it's like for a system with autoboot
> > > disabled. Do we actually need this patch to achieve the functionality
> > > you want? Doesn't that already just happen?
> >
> > The point of this patch is actually to remove code in pytest, by
> > allowing it to skip all the banner-detection stuff. It does not affect
> > things in Labgrid, since it still needs to watch for banners, etc.
>
> But you can't remove code from pytest, people can and will run the suite
> outside of labgrid.
Yes, that's right. I meant that with Labgrid the code is not used.
>
> > Without this patch, we have to tell Labgrid's U-Boot driver to do
> > nothing, so that pytest does it. But that is not a good idea, since
> > Labgrid has a lot more info about the board than pytest has. For
> > example, look at all the SPL-banner-count stuff.
>
> Why do you have to tell it to do nothing? The pytest suite works fine,
> today, if the board stops at the prompt automatically. To be clear, the
> labgrid yaml file I'm using with my scripts has the information to stop
> autoboot in it and it's not causing a problem.
But if it wasn't causing a problem I would not have invented this
annoying scheme. For several boards, by the time pytest gets to see
the output it is too late to press a key and stop. Also, some boards
have a different prompt which is not detected by pytest.
For example:
configs/am62x_beagleplay_a53_defconfig:CONFIG_AUTOBOOT_PROMPT="Press
SPACE to abort autoboot in %d seconds\n"
>
> > Basically, without this patch we cannot use '-s uboot' to tell the
> > Labgrid strategy to take us to a U-Boot prompt. We must just use a raw
> > console with no strategy, relying on pytest to do all the work.
> >
> > I hope that helps explain the problem?
>
> I think I see what you're saying, and it's based on the assumption that
> we'll make everyone either use labgrid?
Not at all. I tested this version of the series again with my
pre-Labgrid lab and it works fine. But I do believe that the hooks
that pytest has are not ideal for use with Labgrid.
Regards,
Simon
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v6 01/19] test: Allow signaling that U-Boot is ready
2024-09-26 21:36 ` Simon Glass
@ 2024-09-27 2:51 ` Tom Rini
2024-10-31 18:03 ` Simon Glass
0 siblings, 1 reply; 37+ messages in thread
From: Tom Rini @ 2024-09-27 2:51 UTC (permalink / raw)
To: Simon Glass; +Cc: u-boot
[-- Attachment #1: Type: text/plain, Size: 4278 bytes --]
On Thu, Sep 26, 2024 at 11:36:00PM +0200, Simon Glass wrote:
> Hi Tom,
>
> On Wed, 25 Sept 2024 at 19:26, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Wed, Sep 25, 2024 at 02:49:56PM +0200, Simon Glass wrote:
> > > Hi Tom,
> > >
> > > On Mon, 23 Sept 2024 at 22:35, Tom Rini <trini@konsulko.com> wrote:
> > > >
> > > > On Fri, Sep 20, 2024 at 08:01:36AM +0200, Simon Glass wrote:
> > > >
> > > >
> > > > > 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(-)
> > > >
> > > > What happens is that labgrid can also be told to look for and then
> > > > interrupt autoboot, just like our pytests can do, and the system is at
> > > > the prompt. But this is also what it's like for a system with autoboot
> > > > disabled. Do we actually need this patch to achieve the functionality
> > > > you want? Doesn't that already just happen?
> > >
> > > The point of this patch is actually to remove code in pytest, by
> > > allowing it to skip all the banner-detection stuff. It does not affect
> > > things in Labgrid, since it still needs to watch for banners, etc.
> >
> > But you can't remove code from pytest, people can and will run the suite
> > outside of labgrid.
>
> Yes, that's right. I meant that with Labgrid the code is not used.
OK. I still think this (and I assume the related flag to tell whichever
helper that was that the platform is ready to go) are too implementation
specific.
> > > Without this patch, we have to tell Labgrid's U-Boot driver to do
> > > nothing, so that pytest does it. But that is not a good idea, since
> > > Labgrid has a lot more info about the board than pytest has. For
> > > example, look at all the SPL-banner-count stuff.
> >
> > Why do you have to tell it to do nothing? The pytest suite works fine,
> > today, if the board stops at the prompt automatically. To be clear, the
> > labgrid yaml file I'm using with my scripts has the information to stop
> > autoboot in it and it's not causing a problem.
>
> But if it wasn't causing a problem I would not have invented this
> annoying scheme.
Well, I honestly thought it might be a remnant from development that
wasn't needed once everything else was done. I do that from time to time
myself.
> For several boards, by the time pytest gets to see
> the output it is too late to press a key and stop.
Why / how? Do we perhaps need to adjust the timeout upwards, slightly?
Especially in light of the changes you also did to detect a "dead" board
and so we don't wait 30 minutes for a test run to fail.
> Also, some boards
> have a different prompt which is not detected by pytest.
>
> For example:
> configs/am62x_beagleplay_a53_defconfig:CONFIG_AUTOBOOT_PROMPT="Press
> SPACE to abort autoboot in %d seconds\n"
Yeah, I had forgotten that as I turn that off along with turning on
other tests via config fragment, on that platform. That really is a
deficiency in our pytest version of this.
> > > Basically, without this patch we cannot use '-s uboot' to tell the
> > > Labgrid strategy to take us to a U-Boot prompt. We must just use a raw
> > > console with no strategy, relying on pytest to do all the work.
> > >
> > > I hope that helps explain the problem?
> >
> > I think I see what you're saying, and it's based on the assumption that
> > we'll make everyone either use labgrid?
>
> Not at all. I tested this version of the series again with my
> pre-Labgrid lab and it works fine. But I do believe that the hooks
> that pytest has are not ideal for use with Labgrid.
OK. But, why can't you make use of the labgrid functionality to pause
the board? It's the state flag for labgrid-client yes? Dropping that in
with my labgrid support would be just a tweak to the console script to
check if the board set labgrid_strategy or something, and I think you
could adapt yours to that too?
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v6 01/19] test: Allow signaling that U-Boot is ready
2024-09-27 2:51 ` Tom Rini
@ 2024-10-31 18:03 ` Simon Glass
2024-10-31 18:28 ` Tom Rini
0 siblings, 1 reply; 37+ messages in thread
From: Simon Glass @ 2024-10-31 18:03 UTC (permalink / raw)
To: Tom Rini; +Cc: u-boot
Hi Tom,
On Fri, 27 Sept 2024 at 04:52, Tom Rini <trini@konsulko.com> wrote:
>
> On Thu, Sep 26, 2024 at 11:36:00PM +0200, Simon Glass wrote:
> > Hi Tom,
> >
> > On Wed, 25 Sept 2024 at 19:26, Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Wed, Sep 25, 2024 at 02:49:56PM +0200, Simon Glass wrote:
> > > > Hi Tom,
> > > >
> > > > On Mon, 23 Sept 2024 at 22:35, Tom Rini <trini@konsulko.com> wrote:
> > > > >
> > > > > On Fri, Sep 20, 2024 at 08:01:36AM +0200, Simon Glass wrote:
> > > > >
> > > > >
> > > > > > 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(-)
> > > > >
> > > > > What happens is that labgrid can also be told to look for and then
> > > > > interrupt autoboot, just like our pytests can do, and the system is at
> > > > > the prompt. But this is also what it's like for a system with autoboot
> > > > > disabled. Do we actually need this patch to achieve the functionality
> > > > > you want? Doesn't that already just happen?
> > > >
> > > > The point of this patch is actually to remove code in pytest, by
> > > > allowing it to skip all the banner-detection stuff. It does not affect
> > > > things in Labgrid, since it still needs to watch for banners, etc.
> > >
> > > But you can't remove code from pytest, people can and will run the suite
> > > outside of labgrid.
> >
> > Yes, that's right. I meant that with Labgrid the code is not used.
>
> OK. I still think this (and I assume the related flag to tell whichever
> helper that was that the platform is ready to go) are too implementation
> specific.
OK, but it does have the virtue of working properly on all the boards
I have, rather than just a subset, without this patch.
>
> > > > Without this patch, we have to tell Labgrid's U-Boot driver to do
> > > > nothing, so that pytest does it. But that is not a good idea, since
> > > > Labgrid has a lot more info about the board than pytest has. For
> > > > example, look at all the SPL-banner-count stuff.
> > >
> > > Why do you have to tell it to do nothing? The pytest suite works fine,
> > > today, if the board stops at the prompt automatically. To be clear, the
> > > labgrid yaml file I'm using with my scripts has the information to stop
> > > autoboot in it and it's not causing a problem.
> >
> > But if it wasn't causing a problem I would not have invented this
> > annoying scheme.
>
> Well, I honestly thought it might be a remnant from development that
> wasn't needed once everything else was done. I do that from time to time
> myself.
OK.
>
> > For several boards, by the time pytest gets to see
> > the output it is too late to press a key and stop.
>
> Why / how? Do we perhaps need to adjust the timeout upwards, slightly?
> Especially in light of the changes you also did to detect a "dead" board
> and so we don't wait 30 minutes for a test run to fail.
Why / how - because the board prints its SPL banner as soon as reset
is released, then waits for U-Boot proper to be sent, at which case
the U-Boot banner is sent .Then labgrid disconnects from the console
and runs microcom, which connects to the console over ser2net and
misses some of what has already been sent.
This is actually a fundamental problem, not something we can adjust
with timeouts.
>
> > Also, some boards
> > have a different prompt which is not detected by pytest.
> >
> > For example:
> > configs/am62x_beagleplay_a53_defconfig:CONFIG_AUTOBOOT_PROMPT="Press
> > SPACE to abort autoboot in %d seconds\n"
>
> Yeah, I had forgotten that as I turn that off along with turning on
> other tests via config fragment, on that platform. That really is a
> deficiency in our pytest version of this.
OK
>
> > > > Basically, without this patch we cannot use '-s uboot' to tell the
> > > > Labgrid strategy to take us to a U-Boot prompt. We must just use a raw
> > > > console with no strategy, relying on pytest to do all the work.
> > > >
> > > > I hope that helps explain the problem?
> > >
> > > I think I see what you're saying, and it's based on the assumption that
> > > we'll make everyone either use labgrid?
> >
> > Not at all. I tested this version of the series again with my
> > pre-Labgrid lab and it works fine. But I do believe that the hooks
> > that pytest has are not ideal for use with Labgrid.
>
> OK. But, why can't you make use of the labgrid functionality to pause
> the board? It's the state flag for labgrid-client yes? Dropping that in
> with my labgrid support would be just a tweak to the console script to
> check if the board set labgrid_strategy or something, and I think you
> could adapt yours to that too?
I am really not sure what to say here. I have explained the reality of
the situation. I have spent hours debugging it and figuring out the
root cause. There is no other tweak I can think of that will solve
this problem.
Regards,
Simon
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v6 01/19] test: Allow signaling that U-Boot is ready
2024-10-31 18:03 ` Simon Glass
@ 2024-10-31 18:28 ` Tom Rini
2024-11-01 15:33 ` Simon Glass
0 siblings, 1 reply; 37+ messages in thread
From: Tom Rini @ 2024-10-31 18:28 UTC (permalink / raw)
To: Simon Glass; +Cc: u-boot
[-- Attachment #1: Type: text/plain, Size: 6806 bytes --]
On Thu, Oct 31, 2024 at 07:03:19PM +0100, Simon Glass wrote:
> Hi Tom,
>
> On Fri, 27 Sept 2024 at 04:52, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Thu, Sep 26, 2024 at 11:36:00PM +0200, Simon Glass wrote:
> > > Hi Tom,
> > >
> > > On Wed, 25 Sept 2024 at 19:26, Tom Rini <trini@konsulko.com> wrote:
> > > >
> > > > On Wed, Sep 25, 2024 at 02:49:56PM +0200, Simon Glass wrote:
> > > > > Hi Tom,
> > > > >
> > > > > On Mon, 23 Sept 2024 at 22:35, Tom Rini <trini@konsulko.com> wrote:
> > > > > >
> > > > > > On Fri, Sep 20, 2024 at 08:01:36AM +0200, Simon Glass wrote:
> > > > > >
> > > > > >
> > > > > > > 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(-)
> > > > > >
> > > > > > What happens is that labgrid can also be told to look for and then
> > > > > > interrupt autoboot, just like our pytests can do, and the system is at
> > > > > > the prompt. But this is also what it's like for a system with autoboot
> > > > > > disabled. Do we actually need this patch to achieve the functionality
> > > > > > you want? Doesn't that already just happen?
> > > > >
> > > > > The point of this patch is actually to remove code in pytest, by
> > > > > allowing it to skip all the banner-detection stuff. It does not affect
> > > > > things in Labgrid, since it still needs to watch for banners, etc.
> > > >
> > > > But you can't remove code from pytest, people can and will run the suite
> > > > outside of labgrid.
> > >
> > > Yes, that's right. I meant that with Labgrid the code is not used.
> >
> > OK. I still think this (and I assume the related flag to tell whichever
> > helper that was that the platform is ready to go) are too implementation
> > specific.
>
> OK, but it does have the virtue of working properly on all the boards
> I have, rather than just a subset, without this patch.
>
> >
> > > > > Without this patch, we have to tell Labgrid's U-Boot driver to do
> > > > > nothing, so that pytest does it. But that is not a good idea, since
> > > > > Labgrid has a lot more info about the board than pytest has. For
> > > > > example, look at all the SPL-banner-count stuff.
> > > >
> > > > Why do you have to tell it to do nothing? The pytest suite works fine,
> > > > today, if the board stops at the prompt automatically. To be clear, the
> > > > labgrid yaml file I'm using with my scripts has the information to stop
> > > > autoboot in it and it's not causing a problem.
> > >
> > > But if it wasn't causing a problem I would not have invented this
> > > annoying scheme.
> >
> > Well, I honestly thought it might be a remnant from development that
> > wasn't needed once everything else was done. I do that from time to time
> > myself.
>
> OK.
>
> >
> > > For several boards, by the time pytest gets to see
> > > the output it is too late to press a key and stop.
> >
> > Why / how? Do we perhaps need to adjust the timeout upwards, slightly?
> > Especially in light of the changes you also did to detect a "dead" board
> > and so we don't wait 30 minutes for a test run to fail.
>
> Why / how - because the board prints its SPL banner as soon as reset
> is released, then waits for U-Boot proper to be sent, at which case
> the U-Boot banner is sent .Then labgrid disconnects from the console
> and runs microcom, which connects to the console over ser2net and
> misses some of what has already been sent.
>
> This is actually a fundamental problem, not something we can adjust
> with timeouts.
Wait, what? *Labgrid* is the thing that's the problem here with dropping
the connection to the board and re-establishing it at some point?
That is the kind of detail which (a) needs to be in the commit message
(b) perhaps an issue filed (or if already exists Link:'d in the commit
message) and (c) would have made it clear why we need this work-around
and then I'd have been a lot happier to accept it as a work-around back
on v1 or so.
> > > Also, some boards
> > > have a different prompt which is not detected by pytest.
> > >
> > > For example:
> > > configs/am62x_beagleplay_a53_defconfig:CONFIG_AUTOBOOT_PROMPT="Press
> > > SPACE to abort autoboot in %d seconds\n"
> >
> > Yeah, I had forgotten that as I turn that off along with turning on
> > other tests via config fragment, on that platform. That really is a
> > deficiency in our pytest version of this.
>
> OK
>
> >
> > > > > Basically, without this patch we cannot use '-s uboot' to tell the
> > > > > Labgrid strategy to take us to a U-Boot prompt. We must just use a raw
> > > > > console with no strategy, relying on pytest to do all the work.
> > > > >
> > > > > I hope that helps explain the problem?
> > > >
> > > > I think I see what you're saying, and it's based on the assumption that
> > > > we'll make everyone either use labgrid?
> > >
> > > Not at all. I tested this version of the series again with my
> > > pre-Labgrid lab and it works fine. But I do believe that the hooks
> > > that pytest has are not ideal for use with Labgrid.
> >
> > OK. But, why can't you make use of the labgrid functionality to pause
> > the board? It's the state flag for labgrid-client yes? Dropping that in
> > with my labgrid support would be just a tweak to the console script to
> > check if the board set labgrid_strategy or something, and I think you
> > could adapt yours to that too?
>
> I am really not sure what to say here. I have explained the reality of
> the situation. I have spent hours debugging it and figuring out the
> root cause. There is no other tweak I can think of that will solve
> this problem.
Well, I hate to get on a tangent here, but I think a common thread is
that after you've spent hours debugging a problem and working out a
solution you then omit much of the details. I'd much rather see a much
longer commit message here describing the problem, or a longer
description under the "---" where you explain that labgrid uses minicom,
gets things up, drops the connection and uses ser2net and so there's a
chance we miss messages and so need something like this. Or, that you've
been debugging on *x86_64* a bunch of EFI_LOADER things and off the
shelf distributions aren't working (and ExitBootServices() is where you
had a problem with Ubuntu I gather still on x86_64).
>
> Regards,
> Simon
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v6 01/19] test: Allow signaling that U-Boot is ready
2024-10-31 18:28 ` Tom Rini
@ 2024-11-01 15:33 ` Simon Glass
2024-11-01 19:02 ` Tom Rini
0 siblings, 1 reply; 37+ messages in thread
From: Simon Glass @ 2024-11-01 15:33 UTC (permalink / raw)
To: Tom Rini; +Cc: u-boot
Hi Tom,
On Thu, 31 Oct 2024 at 19:28, Tom Rini <trini@konsulko.com> wrote:
>
> On Thu, Oct 31, 2024 at 07:03:19PM +0100, Simon Glass wrote:
> > Hi Tom,
> >
> > On Fri, 27 Sept 2024 at 04:52, Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Thu, Sep 26, 2024 at 11:36:00PM +0200, Simon Glass wrote:
> > > > Hi Tom,
> > > >
> > > > On Wed, 25 Sept 2024 at 19:26, Tom Rini <trini@konsulko.com> wrote:
> > > > >
> > > > > On Wed, Sep 25, 2024 at 02:49:56PM +0200, Simon Glass wrote:
> > > > > > Hi Tom,
> > > > > >
> > > > > > On Mon, 23 Sept 2024 at 22:35, Tom Rini <trini@konsulko.com> wrote:
> > > > > > >
> > > > > > > On Fri, Sep 20, 2024 at 08:01:36AM +0200, Simon Glass wrote:
> > > > > > >
> > > > > > >
> > > > > > > > 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(-)
> > > > > > >
> > > > > > > What happens is that labgrid can also be told to look for and then
> > > > > > > interrupt autoboot, just like our pytests can do, and the system is at
> > > > > > > the prompt. But this is also what it's like for a system with autoboot
> > > > > > > disabled. Do we actually need this patch to achieve the functionality
> > > > > > > you want? Doesn't that already just happen?
> > > > > >
> > > > > > The point of this patch is actually to remove code in pytest, by
> > > > > > allowing it to skip all the banner-detection stuff. It does not affect
> > > > > > things in Labgrid, since it still needs to watch for banners, etc.
> > > > >
> > > > > But you can't remove code from pytest, people can and will run the suite
> > > > > outside of labgrid.
> > > >
> > > > Yes, that's right. I meant that with Labgrid the code is not used.
> > >
> > > OK. I still think this (and I assume the related flag to tell whichever
> > > helper that was that the platform is ready to go) are too implementation
> > > specific.
> >
> > OK, but it does have the virtue of working properly on all the boards
> > I have, rather than just a subset, without this patch.
> >
> > >
> > > > > > Without this patch, we have to tell Labgrid's U-Boot driver to do
> > > > > > nothing, so that pytest does it. But that is not a good idea, since
> > > > > > Labgrid has a lot more info about the board than pytest has. For
> > > > > > example, look at all the SPL-banner-count stuff.
> > > > >
> > > > > Why do you have to tell it to do nothing? The pytest suite works fine,
> > > > > today, if the board stops at the prompt automatically. To be clear, the
> > > > > labgrid yaml file I'm using with my scripts has the information to stop
> > > > > autoboot in it and it's not causing a problem.
> > > >
> > > > But if it wasn't causing a problem I would not have invented this
> > > > annoying scheme.
> > >
> > > Well, I honestly thought it might be a remnant from development that
> > > wasn't needed once everything else was done. I do that from time to time
> > > myself.
> >
> > OK.
> >
> > >
> > > > For several boards, by the time pytest gets to see
> > > > the output it is too late to press a key and stop.
> > >
> > > Why / how? Do we perhaps need to adjust the timeout upwards, slightly?
> > > Especially in light of the changes you also did to detect a "dead" board
> > > and so we don't wait 30 minutes for a test run to fail.
> >
> > Why / how - because the board prints its SPL banner as soon as reset
> > is released, then waits for U-Boot proper to be sent, at which case
> > the U-Boot banner is sent .Then labgrid disconnects from the console
> > and runs microcom, which connects to the console over ser2net and
> > misses some of what has already been sent.
> >
> > This is actually a fundamental problem, not something we can adjust
> > with timeouts.
>
> Wait, what? *Labgrid* is the thing that's the problem here with dropping
> the connection to the board and re-establishing it at some point?
Er, yes. Somehow I must have forgotten to mention that.
I actually have a change proposed to Labgrid to adjust that, but it
might take a while...in any case my change is just an option, not the
default.
>
> That is the kind of detail which (a) needs to be in the commit message
> (b) perhaps an issue filed (or if already exists Link:'d in the commit
> message) and (c) would have made it clear why we need this work-around
> and then I'd have been a lot happier to accept it as a work-around back
> on v1 or so.
Well you can call it a hack, but it is a design feature of Labgrid
(having an external terminal program). I don't think they will change
it, but I'm hoping they will accept an 'internal' terminal as an
option. No one has even looked at my patch yet, though, so far as I
can see.
>
> > > > Also, some boards
> > > > have a different prompt which is not detected by pytest.
> > > >
> > > > For example:
> > > > configs/am62x_beagleplay_a53_defconfig:CONFIG_AUTOBOOT_PROMPT="Press
> > > > SPACE to abort autoboot in %d seconds\n"
> > >
> > > Yeah, I had forgotten that as I turn that off along with turning on
> > > other tests via config fragment, on that platform. That really is a
> > > deficiency in our pytest version of this.
> >
> > OK
> >
> > >
> > > > > > Basically, without this patch we cannot use '-s uboot' to tell the
> > > > > > Labgrid strategy to take us to a U-Boot prompt. We must just use a raw
> > > > > > console with no strategy, relying on pytest to do all the work.
> > > > > >
> > > > > > I hope that helps explain the problem?
> > > > >
> > > > > I think I see what you're saying, and it's based on the assumption that
> > > > > we'll make everyone either use labgrid?
> > > >
> > > > Not at all. I tested this version of the series again with my
> > > > pre-Labgrid lab and it works fine. But I do believe that the hooks
> > > > that pytest has are not ideal for use with Labgrid.
> > >
> > > OK. But, why can't you make use of the labgrid functionality to pause
> > > the board? It's the state flag for labgrid-client yes? Dropping that in
> > > with my labgrid support would be just a tweak to the console script to
> > > check if the board set labgrid_strategy or something, and I think you
> > > could adapt yours to that too?
> >
> > I am really not sure what to say here. I have explained the reality of
> > the situation. I have spent hours debugging it and figuring out the
> > root cause. There is no other tweak I can think of that will solve
> > this problem.
>
> Well, I hate to get on a tangent here, but I think a common thread is
> that after you've spent hours debugging a problem and working out a
> solution you then omit much of the details. I'd much rather see a much
> longer commit message here describing the problem, or a longer
> description under the "---" where you explain that labgrid uses minicom,
> gets things up, drops the connection and uses ser2net and so there's a
> chance we miss messages and so need something like this. Or, that you've
> been debugging on *x86_64* a bunch of EFI_LOADER things and off the
> shelf distributions aren't working (and ExitBootServices() is where you
> had a problem with Ubuntu I gather still on x86_64).
Sure, I will try to be more descriptive in future.
The problem in some cases is that I write the commits much later
(weeks, months) and forget what they were for. It actually took quite
a long time to get my lab running with Labgrid and for some of my
responses I had to go and try things to figure out what on earth the
change was for...some of them I dropped. I should have dug more into
this one to really clarify the problem.
Regards,
Simon
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v6 01/19] test: Allow signaling that U-Boot is ready
2024-11-01 15:33 ` Simon Glass
@ 2024-11-01 19:02 ` Tom Rini
2024-11-02 16:32 ` Simon Glass
0 siblings, 1 reply; 37+ messages in thread
From: Tom Rini @ 2024-11-01 19:02 UTC (permalink / raw)
To: Simon Glass; +Cc: u-boot
[-- Attachment #1: Type: text/plain, Size: 6560 bytes --]
On Fri, Nov 01, 2024 at 04:33:41PM +0100, Simon Glass wrote:
> Hi Tom,
>
> On Thu, 31 Oct 2024 at 19:28, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Thu, Oct 31, 2024 at 07:03:19PM +0100, Simon Glass wrote:
> > > Hi Tom,
> > >
> > > On Fri, 27 Sept 2024 at 04:52, Tom Rini <trini@konsulko.com> wrote:
> > > >
> > > > On Thu, Sep 26, 2024 at 11:36:00PM +0200, Simon Glass wrote:
> > > > > Hi Tom,
> > > > >
> > > > > On Wed, 25 Sept 2024 at 19:26, Tom Rini <trini@konsulko.com> wrote:
> > > > > >
> > > > > > On Wed, Sep 25, 2024 at 02:49:56PM +0200, Simon Glass wrote:
> > > > > > > Hi Tom,
> > > > > > >
> > > > > > > On Mon, 23 Sept 2024 at 22:35, Tom Rini <trini@konsulko.com> wrote:
> > > > > > > >
> > > > > > > > On Fri, Sep 20, 2024 at 08:01:36AM +0200, Simon Glass wrote:
> > > > > > > >
> > > > > > > >
> > > > > > > > > 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(-)
> > > > > > > >
> > > > > > > > What happens is that labgrid can also be told to look for and then
> > > > > > > > interrupt autoboot, just like our pytests can do, and the system is at
> > > > > > > > the prompt. But this is also what it's like for a system with autoboot
> > > > > > > > disabled. Do we actually need this patch to achieve the functionality
> > > > > > > > you want? Doesn't that already just happen?
> > > > > > >
> > > > > > > The point of this patch is actually to remove code in pytest, by
> > > > > > > allowing it to skip all the banner-detection stuff. It does not affect
> > > > > > > things in Labgrid, since it still needs to watch for banners, etc.
> > > > > >
> > > > > > But you can't remove code from pytest, people can and will run the suite
> > > > > > outside of labgrid.
> > > > >
> > > > > Yes, that's right. I meant that with Labgrid the code is not used.
> > > >
> > > > OK. I still think this (and I assume the related flag to tell whichever
> > > > helper that was that the platform is ready to go) are too implementation
> > > > specific.
> > >
> > > OK, but it does have the virtue of working properly on all the boards
> > > I have, rather than just a subset, without this patch.
> > >
> > > >
> > > > > > > Without this patch, we have to tell Labgrid's U-Boot driver to do
> > > > > > > nothing, so that pytest does it. But that is not a good idea, since
> > > > > > > Labgrid has a lot more info about the board than pytest has. For
> > > > > > > example, look at all the SPL-banner-count stuff.
> > > > > >
> > > > > > Why do you have to tell it to do nothing? The pytest suite works fine,
> > > > > > today, if the board stops at the prompt automatically. To be clear, the
> > > > > > labgrid yaml file I'm using with my scripts has the information to stop
> > > > > > autoboot in it and it's not causing a problem.
> > > > >
> > > > > But if it wasn't causing a problem I would not have invented this
> > > > > annoying scheme.
> > > >
> > > > Well, I honestly thought it might be a remnant from development that
> > > > wasn't needed once everything else was done. I do that from time to time
> > > > myself.
> > >
> > > OK.
> > >
> > > >
> > > > > For several boards, by the time pytest gets to see
> > > > > the output it is too late to press a key and stop.
> > > >
> > > > Why / how? Do we perhaps need to adjust the timeout upwards, slightly?
> > > > Especially in light of the changes you also did to detect a "dead" board
> > > > and so we don't wait 30 minutes for a test run to fail.
> > >
> > > Why / how - because the board prints its SPL banner as soon as reset
> > > is released, then waits for U-Boot proper to be sent, at which case
> > > the U-Boot banner is sent .Then labgrid disconnects from the console
> > > and runs microcom, which connects to the console over ser2net and
> > > misses some of what has already been sent.
> > >
> > > This is actually a fundamental problem, not something we can adjust
> > > with timeouts.
> >
> > Wait, what? *Labgrid* is the thing that's the problem here with dropping
> > the connection to the board and re-establishing it at some point?
>
> Er, yes. Somehow I must have forgotten to mention that.
>
> I actually have a change proposed to Labgrid to adjust that, but it
> might take a while...in any case my change is just an option, not the
> default.
Can you please link to that specific one? Thanks.
[snip]
> > Well, I hate to get on a tangent here, but I think a common thread is
> > that after you've spent hours debugging a problem and working out a
> > solution you then omit much of the details. I'd much rather see a much
> > longer commit message here describing the problem, or a longer
> > description under the "---" where you explain that labgrid uses minicom,
> > gets things up, drops the connection and uses ser2net and so there's a
> > chance we miss messages and so need something like this. Or, that you've
> > been debugging on *x86_64* a bunch of EFI_LOADER things and off the
> > shelf distributions aren't working (and ExitBootServices() is where you
> > had a problem with Ubuntu I gather still on x86_64).
>
> Sure, I will try to be more descriptive in future.
>
> The problem in some cases is that I write the commits much later
> (weeks, months) and forget what they were for. It actually took quite
> a long time to get my lab running with Labgrid and for some of my
> responses I had to go and try things to figure out what on earth the
> change was for...some of them I dropped. I should have dug more into
> this one to really clarify the problem.
A thing I learned this year at some point was that you can pass -m to
git commit multiple times and get multiple paragraphs. It won't be line
wrapped correctly however. But it helps make "commit early and often" be
more like "commit early and often with whatever you were thinking"
easier to dump more information in to. I think something like that might
help with review on your patches, which I know is a problem we all want
to get resolved in a positive manner.
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v6 01/19] test: Allow signaling that U-Boot is ready
2024-11-01 19:02 ` Tom Rini
@ 2024-11-02 16:32 ` Simon Glass
2024-11-02 21:39 ` Tom Rini
0 siblings, 1 reply; 37+ messages in thread
From: Simon Glass @ 2024-11-02 16:32 UTC (permalink / raw)
To: Tom Rini; +Cc: u-boot
Hi Tom,
On Fri, 1 Nov 2024 at 20:02, Tom Rini <trini@konsulko.com> wrote:
>
> On Fri, Nov 01, 2024 at 04:33:41PM +0100, Simon Glass wrote:
> > Hi Tom,
> >
> > On Thu, 31 Oct 2024 at 19:28, Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Thu, Oct 31, 2024 at 07:03:19PM +0100, Simon Glass wrote:
> > > > Hi Tom,
> > > >
> > > > On Fri, 27 Sept 2024 at 04:52, Tom Rini <trini@konsulko.com> wrote:
> > > > >
> > > > > On Thu, Sep 26, 2024 at 11:36:00PM +0200, Simon Glass wrote:
> > > > > > Hi Tom,
> > > > > >
> > > > > > On Wed, 25 Sept 2024 at 19:26, Tom Rini <trini@konsulko.com> wrote:
> > > > > > >
> > > > > > > On Wed, Sep 25, 2024 at 02:49:56PM +0200, Simon Glass wrote:
> > > > > > > > Hi Tom,
> > > > > > > >
> > > > > > > > On Mon, 23 Sept 2024 at 22:35, Tom Rini <trini@konsulko.com> wrote:
> > > > > > > > >
> > > > > > > > > On Fri, Sep 20, 2024 at 08:01:36AM +0200, Simon Glass wrote:
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > > 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(-)
> > > > > > > > >
> > > > > > > > > What happens is that labgrid can also be told to look for and then
> > > > > > > > > interrupt autoboot, just like our pytests can do, and the system is at
> > > > > > > > > the prompt. But this is also what it's like for a system with autoboot
> > > > > > > > > disabled. Do we actually need this patch to achieve the functionality
> > > > > > > > > you want? Doesn't that already just happen?
> > > > > > > >
> > > > > > > > The point of this patch is actually to remove code in pytest, by
> > > > > > > > allowing it to skip all the banner-detection stuff. It does not affect
> > > > > > > > things in Labgrid, since it still needs to watch for banners, etc.
> > > > > > >
> > > > > > > But you can't remove code from pytest, people can and will run the suite
> > > > > > > outside of labgrid.
> > > > > >
> > > > > > Yes, that's right. I meant that with Labgrid the code is not used.
> > > > >
> > > > > OK. I still think this (and I assume the related flag to tell whichever
> > > > > helper that was that the platform is ready to go) are too implementation
> > > > > specific.
> > > >
> > > > OK, but it does have the virtue of working properly on all the boards
> > > > I have, rather than just a subset, without this patch.
> > > >
> > > > >
> > > > > > > > Without this patch, we have to tell Labgrid's U-Boot driver to do
> > > > > > > > nothing, so that pytest does it. But that is not a good idea, since
> > > > > > > > Labgrid has a lot more info about the board than pytest has. For
> > > > > > > > example, look at all the SPL-banner-count stuff.
> > > > > > >
> > > > > > > Why do you have to tell it to do nothing? The pytest suite works fine,
> > > > > > > today, if the board stops at the prompt automatically. To be clear, the
> > > > > > > labgrid yaml file I'm using with my scripts has the information to stop
> > > > > > > autoboot in it and it's not causing a problem.
> > > > > >
> > > > > > But if it wasn't causing a problem I would not have invented this
> > > > > > annoying scheme.
> > > > >
> > > > > Well, I honestly thought it might be a remnant from development that
> > > > > wasn't needed once everything else was done. I do that from time to time
> > > > > myself.
> > > >
> > > > OK.
> > > >
> > > > >
> > > > > > For several boards, by the time pytest gets to see
> > > > > > the output it is too late to press a key and stop.
> > > > >
> > > > > Why / how? Do we perhaps need to adjust the timeout upwards, slightly?
> > > > > Especially in light of the changes you also did to detect a "dead" board
> > > > > and so we don't wait 30 minutes for a test run to fail.
> > > >
> > > > Why / how - because the board prints its SPL banner as soon as reset
> > > > is released, then waits for U-Boot proper to be sent, at which case
> > > > the U-Boot banner is sent .Then labgrid disconnects from the console
> > > > and runs microcom, which connects to the console over ser2net and
> > > > misses some of what has already been sent.
> > > >
> > > > This is actually a fundamental problem, not something we can adjust
> > > > with timeouts.
> > >
> > > Wait, what? *Labgrid* is the thing that's the problem here with dropping
> > > the connection to the board and re-establishing it at some point?
> >
> > Er, yes. Somehow I must have forgotten to mention that.
> >
> > I actually have a change proposed to Labgrid to adjust that, but it
> > might take a while...in any case my change is just an option, not the
> > default.
>
> Can you please link to that specific one? Thanks.
Yes:
https://github.com/labgrid-project/labgrid/pull/1411/commits/446a800959d85e08bcc610d8d3b8a90ffc66e6c0
>
> [snip]
> > > Well, I hate to get on a tangent here, but I think a common thread is
> > > that after you've spent hours debugging a problem and working out a
> > > solution you then omit much of the details. I'd much rather see a much
> > > longer commit message here describing the problem, or a longer
> > > description under the "---" where you explain that labgrid uses minicom,
> > > gets things up, drops the connection and uses ser2net and so there's a
> > > chance we miss messages and so need something like this. Or, that you've
> > > been debugging on *x86_64* a bunch of EFI_LOADER things and off the
> > > shelf distributions aren't working (and ExitBootServices() is where you
> > > had a problem with Ubuntu I gather still on x86_64).
> >
> > Sure, I will try to be more descriptive in future.
> >
> > The problem in some cases is that I write the commits much later
> > (weeks, months) and forget what they were for. It actually took quite
> > a long time to get my lab running with Labgrid and for some of my
> > responses I had to go and try things to figure out what on earth the
> > change was for...some of them I dropped. I should have dug more into
> > this one to really clarify the problem.
>
> A thing I learned this year at some point was that you can pass -m to
> git commit multiple times and get multiple paragraphs. It won't be line
> wrapped correctly however. But it helps make "commit early and often" be
> more like "commit early and often with whatever you were thinking"
> easier to dump more information in to. I think something like that might
> help with review on your patches, which I know is a problem we all want
> to get resolved in a positive manner.
My workflow for larger things is that I create a load of local
comments. Some of these are real, some are just wip. Then I start
turning into patches once I have something finished and working. Yes I
should add more details of why something didn't work.
Please let me know what can be done to get this integration in.
Regards,
Simon
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v6 01/19] test: Allow signaling that U-Boot is ready
2024-11-02 16:32 ` Simon Glass
@ 2024-11-02 21:39 ` Tom Rini
0 siblings, 0 replies; 37+ messages in thread
From: Tom Rini @ 2024-11-02 21:39 UTC (permalink / raw)
To: Simon Glass; +Cc: u-boot
[-- Attachment #1: Type: text/plain, Size: 919 bytes --]
On Sat, Nov 02, 2024 at 10:32:18AM -0600, Simon Glass wrote:
[snip]
> My workflow for larger things is that I create a load of local
> comments. Some of these are real, some are just wip. Then I start
> turning into patches once I have something finished and working. Yes I
> should add more details of why something didn't work.
>
> Please let me know what can be done to get this integration in.
I think adopting a suffix to your scripts, along with seeing what ever
else of the feedback that was in that long thread on v3 you can
implement will help. Maybe also audit your commit messages and see if
any other details about needing X in order to work around problem Y
spring to mind, or concrete examples. Which is not because people don't
believe you found a problem, it's because we're all a bunch of engineers
and so enjoy solving a problem too and maybe have an alternative idea.
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v6 02/19] test: Use a constant for the test timeout
2024-09-20 6:01 [PATCH v6 00/19] labgrid: Provide an integration with Labgrid Simon Glass
2024-09-20 6:01 ` [PATCH v6 01/19] test: Allow signaling that U-Boot is ready Simon Glass
@ 2024-09-20 6:01 ` Simon Glass
2024-09-23 20:35 ` Tom Rini
2024-09-20 6:01 ` [PATCH v6 03/19] test: Release board after tests complete Simon Glass
` (17 subsequent siblings)
19 siblings, 1 reply; 37+ messages in thread
From: Simon Glass @ 2024-09-20 6:01 UTC (permalink / raw)
To: u-boot; +Cc: Tom Rini, Simon Glass
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.43.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v6 03/19] test: Release board after tests complete
2024-09-20 6:01 [PATCH v6 00/19] labgrid: Provide an integration with Labgrid Simon Glass
2024-09-20 6:01 ` [PATCH v6 01/19] test: Allow signaling that U-Boot is ready Simon Glass
2024-09-20 6:01 ` [PATCH v6 02/19] test: Use a constant for the test timeout Simon Glass
@ 2024-09-20 6:01 ` Simon Glass
2024-09-20 6:01 ` [PATCH v6 04/19] test: Allow connecting to a running board Simon Glass
` (16 subsequent siblings)
19 siblings, 0 replies; 37+ messages in thread
From: Simon Glass @ 2024-09-20 6:01 UTC (permalink / raw)
To: u-boot; +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.43.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v6 04/19] test: Allow connecting to a running board
2024-09-20 6:01 [PATCH v6 00/19] labgrid: Provide an integration with Labgrid Simon Glass
` (2 preceding siblings ...)
2024-09-20 6:01 ` [PATCH v6 03/19] test: Release board after tests complete Simon Glass
@ 2024-09-20 6:01 ` Simon Glass
2024-09-23 20:35 ` Tom Rini
2024-09-20 6:01 ` [PATCH v6 05/19] test: Avoid failing skipped tests Simon Glass
` (15 subsequent siblings)
19 siblings, 1 reply; 37+ messages in thread
From: Simon Glass @ 2024-09-20 6:01 UTC (permalink / raw)
To: u-boot; +Cc: Tom Rini, Simon Glass
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.43.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [PATCH v6 04/19] test: Allow connecting to a running board
2024-09-20 6:01 ` [PATCH v6 04/19] test: Allow connecting to a running board Simon Glass
@ 2024-09-23 20:35 ` Tom Rini
2024-09-25 12:50 ` Simon Glass
0 siblings, 1 reply; 37+ messages in thread
From: Tom Rini @ 2024-09-23 20:35 UTC (permalink / raw)
To: Simon Glass; +Cc: u-boot
[-- Attachment #1: Type: text/plain, Size: 1456 bytes --]
On Fri, Sep 20, 2024 at 08:01:39AM +0200, Simon Glass wrote:
> 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")
I don't think this is the right name for what the commit message says we
have? Maybe something like --use-running-system or otherwise make it
clear we're using the board as it's already up and ready to go?
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 37+ messages in thread* Re: [PATCH v6 04/19] test: Allow connecting to a running board
2024-09-23 20:35 ` Tom Rini
@ 2024-09-25 12:50 ` Simon Glass
0 siblings, 0 replies; 37+ messages in thread
From: Simon Glass @ 2024-09-25 12:50 UTC (permalink / raw)
To: Tom Rini; +Cc: u-boot
Hi Tom,
On Mon, 23 Sept 2024 at 22:35, Tom Rini <trini@konsulko.com> wrote:
>
> On Fri, Sep 20, 2024 at 08:01:39AM +0200, Simon Glass wrote:
>
> > 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")
>
> I don't think this is the right name for what the commit message says we
> have? Maybe something like --use-running-system or otherwise make it
> clear we're using the board as it's already up and ready to go?
OK, I can change it. The name I have chosen refers to the behaviour
rather than the effect, but switching to a flag which describes the
effect is fine with me.
Regards,
Simon
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v6 05/19] test: Avoid failing skipped tests
2024-09-20 6:01 [PATCH v6 00/19] labgrid: Provide an integration with Labgrid Simon Glass
` (3 preceding siblings ...)
2024-09-20 6:01 ` [PATCH v6 04/19] test: Allow connecting to a running board Simon Glass
@ 2024-09-20 6:01 ` Simon Glass
2024-09-20 6:01 ` [PATCH v6 06/19] test: Create a common function to get the config Simon Glass
` (14 subsequent siblings)
19 siblings, 0 replies; 37+ messages in thread
From: Simon Glass @ 2024-09-20 6:01 UTC (permalink / raw)
To: u-boot; +Cc: Tom Rini, Simon Glass
When a test returns -EAGAIN this should not be considered a failure.
Fix what seems to be a problem case, where the pytests see a failure
when a test has merely been skipped.
We cannot squash the -EAGAIN error in ut_run_test() since the failure
count is incremented by its caller, ut_run_test_live_flat()
The specific example here is on snow, where a test is compiled into the
image but cannot run, so returns -EAGAIN to skip:
test/py/tests/test_ut.py sssnow # ut bdinfo bdinfo_test_eth
Test: bdinfo_test_eth: bdinfo.c
Skipping: Console recording disabled
test/test-main.c:486, ut_run_test_live_flat(): 0 == ut_run_test(uts,
test, test->name): Expected 0x0 (0), got 0xfffffff5 (-11)
Test bdinfo_test_eth failed 1 times
Skipped: 1, Failures: 1
snow # F+u-boot-test-reset snow snow
The fix is simply to respect the return code from ut_run_test(), so do
that.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
(no changes since v4)
Changes in v4:
- Expand the commit message
test/test-main.c | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/test/test-main.c b/test/test-main.c
index b3d3e24cdce..b613fc6a4d3 100644
--- a/test/test-main.c
+++ b/test/test-main.c
@@ -486,7 +486,7 @@ static int ut_run_test(struct unit_test_state *uts, struct unit_test *test,
static int ut_run_test_live_flat(struct unit_test_state *uts,
struct unit_test *test)
{
- int runs;
+ int runs, ret;
if ((test->flags & UTF_OTHER_FDT) && !IS_ENABLED(CONFIG_SANDBOX))
return skip_test(uts);
@@ -496,8 +496,11 @@ static int ut_run_test_live_flat(struct unit_test_state *uts,
if (CONFIG_IS_ENABLED(OF_LIVE)) {
if (!(test->flags & UTF_FLAT_TREE)) {
uts->of_live = true;
- ut_assertok(ut_run_test(uts, test, test->name));
- runs++;
+ ret = ut_run_test(uts, test, test->name);
+ if (ret != -EAGAIN) {
+ ut_assertok(ret);
+ runs++;
+ }
}
}
@@ -521,8 +524,11 @@ static int ut_run_test_live_flat(struct unit_test_state *uts,
(!runs || ut_test_run_on_flattree(test)) &&
!(gd->flags & GD_FLG_FDT_CHANGED)) {
uts->of_live = false;
- ut_assertok(ut_run_test(uts, test, test->name));
- runs++;
+ ret = ut_run_test(uts, test, test->name);
+ if (ret != -EAGAIN) {
+ ut_assertok(ret);
+ runs++;
+ }
}
return 0;
--
2.43.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v6 06/19] test: Create a common function to get the config
2024-09-20 6:01 [PATCH v6 00/19] labgrid: Provide an integration with Labgrid Simon Glass
` (4 preceding siblings ...)
2024-09-20 6:01 ` [PATCH v6 05/19] test: Avoid failing skipped tests Simon Glass
@ 2024-09-20 6:01 ` Simon Glass
2024-09-20 6:01 ` [PATCH v6 07/19] test: Introduce the concept of a role Simon Glass
` (13 subsequent siblings)
19 siblings, 0 replies; 37+ messages in thread
From: Simon Glass @ 2024-09-20 6:01 UTC (permalink / raw)
To: u-boot; +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.43.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v6 07/19] test: Introduce the concept of a role
2024-09-20 6:01 [PATCH v6 00/19] labgrid: Provide an integration with Labgrid Simon Glass
` (5 preceding siblings ...)
2024-09-20 6:01 ` [PATCH v6 06/19] test: Create a common function to get the config Simon Glass
@ 2024-09-20 6:01 ` Simon Glass
2024-09-20 6:01 ` [PATCH v6 08/19] test: Move the receive code into a function Simon Glass
` (12 subsequent siblings)
19 siblings, 0 replies; 37+ messages in thread
From: Simon Glass @ 2024-09-20 6:01 UTC (permalink / raw)
To: u-boot; +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 v5)
Changes in v5:
- Add a few more comments
- Comment out the debugging, which might be useful later
test/py/conftest.py | 38 ++++++++++++++++++++++++++++++++++----
1 file changed, 34 insertions(+), 4 deletions(-)
diff --git a/test/py/conftest.py b/test/py/conftest.py
index 6547c6922c6..03dfd8ab562 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,40 @@ 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')
+
+ # Get a few provided parameters
build_dir = config.getoption('build_dir')
+ if role:
+ # When using a role, build_dir and build_dir_extra are normally not set,
+ # since they are picked up from Labgrid via the u-boot-test-getrole
+ # script
+ 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)
+ # For debugging
+ # 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
+ # For debugging
+ # 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.43.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v6 08/19] test: Move the receive code into a function
2024-09-20 6:01 [PATCH v6 00/19] labgrid: Provide an integration with Labgrid Simon Glass
` (6 preceding siblings ...)
2024-09-20 6:01 ` [PATCH v6 07/19] test: Introduce the concept of a role Simon Glass
@ 2024-09-20 6:01 ` Simon Glass
2024-09-20 6:01 ` [PATCH v6 09/19] test: Separate out the exception handling Simon Glass
` (11 subsequent siblings)
19 siblings, 0 replies; 37+ messages in thread
From: Simon Glass @ 2024-09-20 6:01 UTC (permalink / raw)
To: u-boot; +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 97e95e07c80..69a2cd55816 100644
--- a/test/py/u_boot_spawn.py
+++ b/test/py/u_boot_spawn.py
@@ -137,6 +137,32 @@ class Spawn:
os.write(self.fd, data.encode(errors='replace'))
+ def receive(self, num_bytes):
+ """Receive data from the sub-process's stdin.
+
+ Args:
+ num_bytes (int): Maximum number of bytes to read
+
+ Returns:
+ str: The data received
+
+ Raises:
+ ValueError if U-Boot died
+ """
+ try:
+ c = os.read(self.fd, num_bytes).decode(errors='replace')
+ except OSError as err:
+ # With sandbox, try to detect when U-Boot exits when it
+ # shouldn't and explain why. This is much more friendly than
+ # just dying with an I/O error
+ if self.decode_signal and err.errno == 5: # I/O error
+ alive, _, info = self.checkalive()
+ if alive:
+ raise err
+ raise ValueError('U-Boot exited with %s' % info)
+ raise
+ return c
+
def expect(self, patterns):
"""Wait for the sub-process to emit specific data.
@@ -193,18 +219,7 @@ class Spawn:
events = self.poll.poll(poll_maxwait)
if not events:
raise Timeout()
- try:
- c = os.read(self.fd, 1024).decode(errors='replace')
- except OSError as err:
- # With sandbox, try to detect when U-Boot exits when it
- # shouldn't and explain why. This is much more friendly than
- # just dying with an I/O error
- if self.decode_signal and err.errno == 5: # I/O error
- alive, _, info = self.checkalive()
- if alive:
- raise err
- raise ValueError('U-Boot exited with %s' % info)
- raise
+ c = self.receive(1024)
if self.logfile_read:
self.logfile_read.write(c)
self.buf += c
--
2.43.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v6 09/19] test: Separate out the exception handling
2024-09-20 6:01 [PATCH v6 00/19] labgrid: Provide an integration with Labgrid Simon Glass
` (7 preceding siblings ...)
2024-09-20 6:01 ` [PATCH v6 08/19] test: Move the receive code into a function Simon Glass
@ 2024-09-20 6:01 ` Simon Glass
2024-09-20 6:01 ` [PATCH v6 10/19] test: Detect dead connections Simon Glass
` (10 subsequent siblings)
19 siblings, 0 replies; 37+ messages in thread
From: Simon Glass @ 2024-09-20 6:01 UTC (permalink / raw)
To: u-boot; +Cc: Tom Rini, Simon Glass
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 69a2cd55816..bcba7b5cd43 100644
--- a/test/py/u_boot_spawn.py
+++ b/test/py/u_boot_spawn.py
@@ -16,6 +16,17 @@ import traceback
class Timeout(Exception):
"""An exception sub-class that indicates that a timeout occurred."""
+class BootFail(Exception):
+ """An exception sub-class that indicates that a boot failure occurred.
+
+ This is used when a bad pattern is seen when waiting for the boot prompt.
+ It is regarded as fatal, to avoid trying to boot the again and again to no
+ avail.
+ """
+
+class Unexpected(Exception):
+ """An exception sub-class that indicates that unexpected test was seen."""
+
class Spawn:
"""Represents the stdio of a freshly created sub-process. Commands may be
sent to the process, and responses waited for.
--
2.43.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v6 10/19] test: Detect dead connections
2024-09-20 6:01 [PATCH v6 00/19] labgrid: Provide an integration with Labgrid Simon Glass
` (8 preceding siblings ...)
2024-09-20 6:01 ` [PATCH v6 09/19] test: Separate out the exception handling Simon Glass
@ 2024-09-20 6:01 ` Simon Glass
2024-09-20 6:01 ` [PATCH v6 11/19] test: Tidy up remaining exceptions Simon Glass
` (9 subsequent siblings)
19 siblings, 0 replies; 37+ messages in thread
From: Simon Glass @ 2024-09-20 6:01 UTC (permalink / raw)
To: u-boot; +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 03dfd8ab562..9eea65b5929 100644
--- a/test/py/conftest.py
+++ b/test/py/conftest.py
@@ -25,6 +25,7 @@ import re
from _pytest.runner import runtestprotocol
import subprocess
import sys
+from u_boot_spawn import BootFail, Timeout, Unexpected, handle_exception
# Globals: The HTML log file, and the connection to the U-Boot console.
log = None
@@ -287,6 +288,7 @@ def pytest_configure(config):
ubconfig.gdbserver = gdbserver
ubconfig.no_prompt_wait = config.getoption('no_prompt_wait')
ubconfig.dtb = build_dir + '/arch/sandbox/dts/test.dtb'
+ ubconfig.connection_ok = True
env_vars = (
'board_type',
@@ -453,8 +455,21 @@ def u_boot_console(request):
Returns:
The fixture value.
"""
-
- console.ensure_spawned()
+ if not ubconfig.connection_ok:
+ pytest.skip('Cannot get target connection')
+ return None
+ try:
+ console.ensure_spawned()
+ except OSError as err:
+ handle_exception(ubconfig, console, log, err, 'Lab failure', True)
+ except Timeout as err:
+ handle_exception(ubconfig, console, log, err, 'Lab timeout', True)
+ except BootFail as err:
+ handle_exception(ubconfig, console, log, err, 'Boot fail', True,
+ console.get_spawn_output())
+ except Unexpected:
+ handle_exception(ubconfig, console, log, err, 'Unexpected test output',
+ False)
return console
anchors = {}
diff --git a/test/py/u_boot_spawn.py b/test/py/u_boot_spawn.py
index bcba7b5cd43..24d369035e5 100644
--- a/test/py/u_boot_spawn.py
+++ b/test/py/u_boot_spawn.py
@@ -8,6 +8,7 @@ Logic to spawn a sub-process and interact with its stdio.
import os
import re
import pty
+import pytest
import signal
import select
import time
@@ -27,6 +28,43 @@ class BootFail(Exception):
class Unexpected(Exception):
"""An exception sub-class that indicates that unexpected test was seen."""
+
+def handle_exception(ubconfig, console, log, err, name, fatal, output=''):
+ """Handle an exception from the console
+
+ Exceptions can occur when there is unexpected output or due to the board
+ crashing or hanging. Some exceptions are likely fatal, where retrying will
+ just chew up time to no available. In those cases it is best to cause
+ further tests be skipped.
+
+ Args:
+ ubconfig (ArbitraryAttributeContainer): ubconfig object
+ log (Logfile): Place to log errors
+ console (ConsoleBase): Console to clean up, if fatal
+ err (Exception): Exception which was thrown
+ name (str): Name of problem, to log
+ fatal (bool): True to abort all tests
+ output (str): Extra output to report on boot failure. This can show the
+ target's console output as it tried to boot
+ """
+ msg = f'{name}: '
+ if fatal:
+ msg += 'Marking connection bad - no other tests will run'
+ else:
+ msg += 'Assuming that lab is healthy'
+ print(msg)
+ log.error(msg)
+ log.error(f'Error: {err}')
+
+ if output:
+ msg += f'; output {output}'
+
+ if fatal:
+ ubconfig.connection_ok = False
+ console.cleanup_spawn()
+ pytest.exit(msg)
+
+
class Spawn:
"""Represents the stdio of a freshly created sub-process. Commands may be
sent to the process, and responses waited for.
--
2.43.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v6 11/19] test: Tidy up remaining exceptions
2024-09-20 6:01 [PATCH v6 00/19] labgrid: Provide an integration with Labgrid Simon Glass
` (9 preceding siblings ...)
2024-09-20 6:01 ` [PATCH v6 10/19] test: Detect dead connections Simon Glass
@ 2024-09-20 6:01 ` Simon Glass
2024-09-20 6:01 ` [PATCH v6 12/19] test: Introduce lab mode Simon Glass
` (8 subsequent siblings)
19 siblings, 0 replies; 37+ messages in thread
From: Simon Glass @ 2024-09-20 6:01 UTC (permalink / raw)
To: u-boot; +Cc: Tom Rini, Simon Glass
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.43.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v6 12/19] test: Introduce lab mode
2024-09-20 6:01 [PATCH v6 00/19] labgrid: Provide an integration with Labgrid Simon Glass
` (10 preceding siblings ...)
2024-09-20 6:01 ` [PATCH v6 11/19] test: Tidy up remaining exceptions Simon Glass
@ 2024-09-20 6:01 ` Simon Glass
2024-09-20 6:01 ` [PATCH v6 13/19] test: Improve handling of sending commands Simon Glass
` (7 subsequent siblings)
19 siblings, 0 replies; 37+ messages in thread
From: Simon Glass @ 2024-09-20 6:01 UTC (permalink / raw)
To: u-boot; +Cc: Tom Rini, Simon Glass
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.43.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v6 13/19] test: Improve handling of sending commands
2024-09-20 6:01 [PATCH v6 00/19] labgrid: Provide an integration with Labgrid Simon Glass
` (11 preceding siblings ...)
2024-09-20 6:01 ` [PATCH v6 12/19] test: Introduce lab mode Simon Glass
@ 2024-09-20 6:01 ` Simon Glass
2024-09-20 6:01 ` [PATCH v6 14/19] test: Fix mulptiplex_log typo Simon Glass
` (6 subsequent siblings)
19 siblings, 0 replies; 37+ messages in thread
From: Simon Glass @ 2024-09-20 6:01 UTC (permalink / raw)
To: u-boot; +Cc: Tom Rini, Simon Glass
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.43.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v6 14/19] test: Fix mulptiplex_log typo
2024-09-20 6:01 [PATCH v6 00/19] labgrid: Provide an integration with Labgrid Simon Glass
` (12 preceding siblings ...)
2024-09-20 6:01 ` [PATCH v6 13/19] test: Improve handling of sending commands Simon Glass
@ 2024-09-20 6:01 ` Simon Glass
2024-09-20 6:01 ` [PATCH v6 15/19] test: Avoid double echo when starting up Simon Glass
` (5 subsequent siblings)
19 siblings, 0 replies; 37+ messages in thread
From: Simon Glass @ 2024-09-20 6:01 UTC (permalink / raw)
To: u-boot; +Cc: Tom Rini, Simon Glass
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.43.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v6 15/19] test: Avoid double echo when starting up
2024-09-20 6:01 [PATCH v6 00/19] labgrid: Provide an integration with Labgrid Simon Glass
` (13 preceding siblings ...)
2024-09-20 6:01 ` [PATCH v6 14/19] test: Fix mulptiplex_log typo Simon Glass
@ 2024-09-20 6:01 ` Simon Glass
2024-09-20 6:01 ` [PATCH v6 16/19] test: Try to shut down the lab console gracefully Simon Glass
` (4 subsequent siblings)
19 siblings, 0 replies; 37+ messages in thread
From: Simon Glass @ 2024-09-20 6:01 UTC (permalink / raw)
To: u-boot; +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>
---
Changes in v6:
- Rebase without an earlier patch
Changes in v2:
- Only disable echo if a terminal is detected
test/py/u_boot_spawn.py | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/test/py/u_boot_spawn.py b/test/py/u_boot_spawn.py
index 24d369035e5..f2398098a00 100644
--- a/test/py/u_boot_spawn.py
+++ b/test/py/u_boot_spawn.py
@@ -11,6 +11,8 @@ import pty
import pytest
import signal
import select
+import sys
+import termios
import time
import traceback
@@ -115,11 +117,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.43.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v6 16/19] test: Try to shut down the lab console gracefully
2024-09-20 6:01 [PATCH v6 00/19] labgrid: Provide an integration with Labgrid Simon Glass
` (14 preceding siblings ...)
2024-09-20 6:01 ` [PATCH v6 15/19] test: Avoid double echo when starting up Simon Glass
@ 2024-09-20 6:01 ` Simon Glass
2024-09-20 6:01 ` [PATCH v6 17/19] test: Add a section for closing the connection Simon Glass
` (3 subsequent siblings)
19 siblings, 0 replies; 37+ messages in thread
From: Simon Glass @ 2024-09-20 6:01 UTC (permalink / raw)
To: u-boot; +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. Only do this when
labgrid is being used (detected with an env var).
If that doesn't work, try the less graceful approach.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v6:
- Avoid doing the special shutdown unless USE_LABGRID is enabled
test/py/u_boot_spawn.py | 22 +++++++++++++++++++---
1 file changed, 19 insertions(+), 3 deletions(-)
diff --git a/test/py/u_boot_spawn.py b/test/py/u_boot_spawn.py
index f2398098a00..72d3d5e77b1 100644
--- a/test/py/u_boot_spawn.py
+++ b/test/py/u_boot_spawn.py
@@ -16,6 +16,9 @@ import termios
import time
import traceback
+# Character to send (twice) to exit the terminal
+EXIT_CHAR = 0x1d # FS (Ctrl + ])
+
class Timeout(Exception):
"""An exception sub-class that indicates that a timeout occurred."""
@@ -303,15 +306,28 @@ class Spawn:
None.
Returns:
- Nothing.
+ str: Type of closure completed
"""
-
+ # For Labgrid, ask it is exit gracefully, so it can transition the board
+ # to the final state (like 'off') before exiting.
+ if os.environ.get('USE_LABGRID'):
+ self.send(chr(EXIT_CHAR) * 2)
+
+ # Wait about 10 seconds for Labgrid to close and power off the board
+ for _ in range(100):
+ if not self.isalive():
+ return 'normal'
+ time.sleep(0.1)
+
+ # That didn't work, so try closing the PTY
os.close(self.fd)
for _ in range(100):
if not self.isalive():
- break
+ return 'break'
time.sleep(0.1)
+ return 'timeout'
+
def get_expect_output(self):
"""Return the output read by expect()
--
2.43.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v6 17/19] test: Add a section for closing the connection
2024-09-20 6:01 [PATCH v6 00/19] labgrid: Provide an integration with Labgrid Simon Glass
` (15 preceding siblings ...)
2024-09-20 6:01 ` [PATCH v6 16/19] test: Try to shut down the lab console gracefully Simon Glass
@ 2024-09-20 6:01 ` Simon Glass
2024-09-20 6:01 ` [PATCH v6 18/19] test: Support testing with two board-builds Simon Glass
` (2 subsequent siblings)
19 siblings, 0 replies; 37+ messages in thread
From: Simon Glass @ 2024-09-20 6:01 UTC (permalink / raw)
To: u-boot; +Cc: Tom Rini, Simon Glass
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.43.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v6 18/19] test: Support testing with two board-builds
2024-09-20 6:01 [PATCH v6 00/19] labgrid: Provide an integration with Labgrid Simon Glass
` (16 preceding siblings ...)
2024-09-20 6:01 ` [PATCH v6 17/19] test: Add a section for closing the connection Simon Glass
@ 2024-09-20 6:01 ` Simon Glass
2024-09-20 6:01 ` [PATCH v6 19/19] CI: Allow running tests on sjg lab Simon Glass
2024-09-23 20:36 ` [PATCH v6 00/19] labgrid: Provide an integration with Labgrid Tom Rini
19 siblings, 0 replies; 37+ messages in thread
From: Simon Glass @ 2024-09-20 6:01 UTC (permalink / raw)
To: u-boot; +Cc: Tom Rini, Simon Glass
The Beagleplay board uses two entirely separate builds to produce an
image, rather than using an SPL build for this purpose.
Handle this in test.py by adding more parameters.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
(no changes since v5)
Changes in v5:
- Add a patch to support testing with two board-builds
test/py/conftest.py | 36 +++++++++++++++++++++++++++++++-----
1 file changed, 31 insertions(+), 5 deletions(-)
diff --git a/test/py/conftest.py b/test/py/conftest.py
index 9eea65b5929..542c0d53e46 100644
--- a/test/py/conftest.py
+++ b/test/py/conftest.py
@@ -66,12 +66,16 @@ def pytest_addoption(parser):
parser.addoption('--build-dir', default=None,
help='U-Boot build directory (O=)')
+ parser.addoption('--build-dir-extra', default=None,
+ help='U-Boot build directory for extra build (O=)')
parser.addoption('--result-dir', default=None,
help='U-Boot test result/tmp directory')
parser.addoption('--persistent-data-dir', default=None,
help='U-Boot test persistent generated data directory')
parser.addoption('--board-type', '--bd', '-B', default='sandbox',
help='U-Boot board type')
+ parser.addoption('--board-type-extra', '--bde', default='sandbox',
+ help='U-Boot extra board type')
parser.addoption('--board-identity', '--id', default='na',
help='U-Boot board identity/instance')
parser.addoption('--build', default=False, action='store_true',
@@ -129,14 +133,17 @@ def get_details(config):
Returns:
tuple:
str: Board type (U-Boot build name)
+ str: Extra board type (where two U-Boot builds are needed)
str: Identity for the lab board
str: Build directory
+ str: Extra build directory (where two U-Boot builds are needed)
str: Source directory
"""
role = config.getoption('role')
# Get a few provided parameters
build_dir = config.getoption('build_dir')
+ build_dir_extra = config.getoption('build_dir_extra')
if role:
# When using a role, build_dir and build_dir_extra are normally not set,
# since they are picked up from Labgrid via the u-boot-test-getrole
@@ -146,6 +153,8 @@ def get_details(config):
env = os.environ.copy()
if build_dir:
env['U_BOOT_BUILD_DIR'] = build_dir
+ if build_dir_extra:
+ env['U_BOOT_BUILD_DIR_EXTRA'] = build_dir_extra
proc = subprocess.run(cmd, capture_output=True, encoding='utf-8',
env=env)
if proc.returncode:
@@ -159,24 +168,36 @@ def get_details(config):
vals[k] = value
# For debugging
# print('conftest: lab info:', vals)
- board_type, default_build_dir, source_dir = (vals['board'],
- vals['build_dir'], vals['source_dir'])
+
+ # Read the build directories here, in case none were provided in the
+ # command-line arguments
+ (board_type, board_type_extra, default_build_dir,
+ default_build_dir_extra, source_dir) = (vals['board'],
+ vals['board_extra'], vals['build_dir'], vals['build_dir_extra'],
+ vals['source_dir'])
else:
board_type = config.getoption('board_type')
+ board_type_extra = config.getoption('board_type_extra')
board_identity = config.getoption('board_identity')
source_dir = os.path.dirname(os.path.dirname(TEST_PY_DIR))
default_build_dir = source_dir + '/build-' + board_type
+ default_build_dir_extra = source_dir + '/build-' + board_type_extra
+
+ # Use the provided command-line arguments if present, else fall back to
if not build_dir:
build_dir = default_build_dir
+ if not build_dir_extra:
+ build_dir_extra = default_build_dir_extra
- return board_type, board_identity, build_dir, source_dir
+ return (board_type, board_type_extra, board_identity, build_dir,
+ build_dir_extra, source_dir)
def pytest_xdist_setupnodes(config, specs):
"""Clear out any 'done' file from a previous build"""
global build_done_file
- build_dir = get_details(config)[2]
+ build_dir = get_details(config)[3]
build_done_file = Path(build_dir) / 'build.done'
if build_done_file.exists():
@@ -216,7 +237,8 @@ def pytest_configure(config):
global console
global ubconfig
- board_type, board_identity, build_dir, source_dir = get_details(config)
+ (board_type, board_type_extra, board_identity, build_dir, build_dir_extra,
+ source_dir) = get_details(config)
board_type_filename = board_type.replace('-', '_')
board_identity_filename = board_identity.replace('-', '_')
@@ -281,9 +303,11 @@ def pytest_configure(config):
ubconfig.test_py_dir = TEST_PY_DIR
ubconfig.source_dir = source_dir
ubconfig.build_dir = build_dir
+ ubconfig.build_dir_extra = build_dir_extra
ubconfig.result_dir = result_dir
ubconfig.persistent_data_dir = persistent_data_dir
ubconfig.board_type = board_type
+ ubconfig.board_type_extra = board_type_extra
ubconfig.board_identity = board_identity
ubconfig.gdbserver = gdbserver
ubconfig.no_prompt_wait = config.getoption('no_prompt_wait')
@@ -292,10 +316,12 @@ def pytest_configure(config):
env_vars = (
'board_type',
+ 'board_type_extra',
'board_identity',
'source_dir',
'test_py_dir',
'build_dir',
+ 'build_dir_extra',
'result_dir',
'persistent_data_dir',
)
--
2.43.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v6 19/19] CI: Allow running tests on sjg lab
2024-09-20 6:01 [PATCH v6 00/19] labgrid: Provide an integration with Labgrid Simon Glass
` (17 preceding siblings ...)
2024-09-20 6:01 ` [PATCH v6 18/19] test: Support testing with two board-builds Simon Glass
@ 2024-09-20 6:01 ` Simon Glass
2024-09-23 20:36 ` [PATCH v6 00/19] labgrid: Provide an integration with Labgrid Tom Rini
19 siblings, 0 replies; 37+ messages in thread
From: Simon Glass @ 2024-09-20 6:01 UTC (permalink / raw)
To: u-boot; +Cc: Tom Rini, Simon Glass, Andrejs Cainikovs
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 v6:
- Drop patch 'test: Pass stderr to stdout'
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 7d621031b85..4d66e53b5b2 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -17,6 +17,7 @@ stages:
- testsuites
- test.py
- world build
+ - sjg-lab
.buildman_and_testpy_template: &buildman_and_testpy_dfn
stage: test.py
@@ -491,3 +492,155 @@ coreboot test.py:
TEST_PY_TEST_SPEC: "not sleep"
TEST_PY_ID: "--id qemu"
<<: *buildman_and_testpy_dfn
+
+.lab_template: &lab_dfn
+ stage: sjg-lab
+ rules:
+ - if: $SJG_LAB == "1"
+ when: always
+ - when: manual
+ tags: [ 'lab' ]
+ script:
+ - if [[ -z "${SJG_LAB}" ]]; then
+ exit 0;
+ fi
+ # Environment:
+ # SRC - source tree
+ # OUT - output directory for builds
+ - export SRC="$(pwd)"
+ - export OUT="${SRC}/build/${BOARD}"
+ - export PATH=$PATH:~/bin
+ - export PATH=$PATH:/vid/software/devel/ubtest/u-boot-test-hooks/bin
+
+ # Load it on the device
+ - ret=0
+ - echo "role ${ROLE}"
+ - export strategy="-s uboot -e off"
+ # export verbose="-v"
+ - ${SRC}/test/py/test.py --role ${ROLE} --build-dir "${OUT}"
+ --capture=tee-sys -k "not bootstd"|| ret=$?
+ - U_BOOT_BOARD_IDENTITY="${ROLE}" u-boot-test-release || true
+ - if [[ $ret -ne 0 ]]; then
+ exit $ret;
+ fi
+ artifacts:
+ when: always
+ paths:
+ - "build/${BOARD}/test-log.html"
+ - "build/${BOARD}/multiplexed_log.css"
+ expire_in: 1 week
+
+rpi3:
+ variables:
+ ROLE: rpi3
+ <<: *lab_dfn
+
+opi_pc:
+ variables:
+ ROLE: opi_pc
+ <<: *lab_dfn
+
+pcduino3_nano:
+ variables:
+ ROLE: pcduino3_nano
+ <<: *lab_dfn
+
+samus:
+ variables:
+ ROLE: samus
+ <<: *lab_dfn
+
+link:
+ variables:
+ ROLE: link
+ <<: *lab_dfn
+
+jerry:
+ variables:
+ ROLE: jerry
+ <<: *lab_dfn
+
+minnowmax:
+ variables:
+ ROLE: minnowmax
+ <<: *lab_dfn
+
+opi_pc2:
+ variables:
+ ROLE: opi_pc2
+ <<: *lab_dfn
+
+bpi:
+ variables:
+ ROLE: bpi
+ <<: *lab_dfn
+
+rpi2:
+ variables:
+ ROLE: rpi2
+ <<: *lab_dfn
+
+bob:
+ variables:
+ ROLE: bob
+ <<: *lab_dfn
+
+ff3399:
+ variables:
+ ROLE: ff3399
+ <<: *lab_dfn
+
+coral:
+ variables:
+ ROLE: coral
+ <<: *lab_dfn
+
+rpi3z:
+ variables:
+ ROLE: rpi3z
+ <<: *lab_dfn
+
+bbb:
+ variables:
+ ROLE: bbb
+ <<: *lab_dfn
+
+kevin:
+ variables:
+ ROLE: kevin
+ <<: *lab_dfn
+
+pine64:
+ variables:
+ ROLE: pine64
+ <<: *lab_dfn
+
+c4:
+ variables:
+ ROLE: c4
+ <<: *lab_dfn
+
+rpi4:
+ variables:
+ ROLE: rpi4
+ <<: *lab_dfn
+
+rpi0:
+ variables:
+ ROLE: rpi0
+ <<: *lab_dfn
+
+snow:
+ variables:
+ ROLE: snow
+ <<: *lab_dfn
+
+pcduino3:
+ variables:
+ ROLE: pcduino3
+ <<: *lab_dfn
+
+nyan-big:
+ variables:
+ ROLE: nyan-big
+ <<: *lab_dfn
--
2.43.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [PATCH v6 00/19] labgrid: Provide an integration with Labgrid
2024-09-20 6:01 [PATCH v6 00/19] labgrid: Provide an integration with Labgrid Simon Glass
` (18 preceding siblings ...)
2024-09-20 6:01 ` [PATCH v6 19/19] CI: Allow running tests on sjg lab Simon Glass
@ 2024-09-23 20:36 ` Tom Rini
2024-09-25 12:50 ` Simon Glass
19 siblings, 1 reply; 37+ messages in thread
From: Tom Rini @ 2024-09-23 20:36 UTC (permalink / raw)
To: Simon Glass; +Cc: u-boot
[-- Attachment #1: Type: text/plain, Size: 2749 bytes --]
On Fri, Sep 20, 2024 at 08:01:35AM +0200, Simon Glass wrote:
> Labgrid provides access to a hardware lab in an automated way. It is
> possible to boot U-Boot on boards in the lab without physically touching
> them. It relies on relays, USB UARTs and SD muxes, among other things.
>
> By way of background, about 4 years ago I wrong a thing called Labman[1]
> which allowed my lab of about 30 devices to be operated remotely, using
> tbot for the console and build integration. While it worked OK and I
> used it for many bisects, I didn't take it any further.
>
> It turns out that there was already an existing program, called Labgrid,
> which I did not know about at time (thank you Tom for telling me). It is
> more rounded than Labman and has a number of advantages:
>
> - does not need udev rules, mostly
> - has several existing users who rely on it
> - supports multiple machines exporting their devices
>
> It lacks a 'lab check' feature and a few other things, but these can be
> remedied.
>
> On and off over the past several weeks I have been experimenting with
> Labgrid. I have managed to create an initial U-Boot integration (this
> series) by adding various features to Labgrid[2] and the U-Boot test
> hooks.
>
> I hope that this might inspire others to set up boards and run tests
> automatically, rather than relying on infrequent, manual test. Perhaps
> it may even be possible to have a number of labs available.
>
> Included in the integration are a number of simple scripts which make it
> easy to connect to boards and run tests:
>
> ub-int <target>
> Build and boot on a target, starting an interactive session
>
> ub-cli <target>
> Build and boot on a target, ensure U-Boot starts and provide an interactive
> session from there
>
> ub-smoke <target>
> Smoke test U-Boot to check that it boots to a prompt on a target
>
> ub-bisect <target>
> Bisect a git tree to locate a failure on a particular target
>
> ub-pyt <target> <testspec>
> Run U-Boot pytests on a target
>
> Some of these help to provide the same tbot[4] workflow which I have
> relied on for several years, albeit much simpler versions.
>
> The goal here is to create some sort of script which can collect
> patches from the mailing list, apply them and test them on a selection
> of boards. I suspect that script already exists, so please let me know
> what you suggest.
I still really wish you would split the labgrid parts of this series out
from the pytest fixes and improvement parts. Things are largely fine
outside of the labgrid part for example, which I'll probably just take
after confirming it doesn't break using labgrid other ways.
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 37+ messages in thread* Re: [PATCH v6 00/19] labgrid: Provide an integration with Labgrid
2024-09-23 20:36 ` [PATCH v6 00/19] labgrid: Provide an integration with Labgrid Tom Rini
@ 2024-09-25 12:50 ` Simon Glass
2024-09-25 17:26 ` Tom Rini
0 siblings, 1 reply; 37+ messages in thread
From: Simon Glass @ 2024-09-25 12:50 UTC (permalink / raw)
To: Tom Rini; +Cc: u-boot
Hi Tom,
On Mon, 23 Sept 2024 at 22:36, Tom Rini <trini@konsulko.com> wrote:
>
> On Fri, Sep 20, 2024 at 08:01:35AM +0200, Simon Glass wrote:
>
> > Labgrid provides access to a hardware lab in an automated way. It is
> > possible to boot U-Boot on boards in the lab without physically touching
> > them. It relies on relays, USB UARTs and SD muxes, among other things.
> >
> > By way of background, about 4 years ago I wrong a thing called Labman[1]
> > which allowed my lab of about 30 devices to be operated remotely, using
> > tbot for the console and build integration. While it worked OK and I
> > used it for many bisects, I didn't take it any further.
> >
> > It turns out that there was already an existing program, called Labgrid,
> > which I did not know about at time (thank you Tom for telling me). It is
> > more rounded than Labman and has a number of advantages:
> >
> > - does not need udev rules, mostly
> > - has several existing users who rely on it
> > - supports multiple machines exporting their devices
> >
> > It lacks a 'lab check' feature and a few other things, but these can be
> > remedied.
> >
> > On and off over the past several weeks I have been experimenting with
> > Labgrid. I have managed to create an initial U-Boot integration (this
> > series) by adding various features to Labgrid[2] and the U-Boot test
> > hooks.
> >
> > I hope that this might inspire others to set up boards and run tests
> > automatically, rather than relying on infrequent, manual test. Perhaps
> > it may even be possible to have a number of labs available.
> >
> > Included in the integration are a number of simple scripts which make it
> > easy to connect to boards and run tests:
> >
> > ub-int <target>
> > Build and boot on a target, starting an interactive session
> >
> > ub-cli <target>
> > Build and boot on a target, ensure U-Boot starts and provide an interactive
> > session from there
> >
> > ub-smoke <target>
> > Smoke test U-Boot to check that it boots to a prompt on a target
> >
> > ub-bisect <target>
> > Bisect a git tree to locate a failure on a particular target
> >
> > ub-pyt <target> <testspec>
> > Run U-Boot pytests on a target
> >
> > Some of these help to provide the same tbot[4] workflow which I have
> > relied on for several years, albeit much simpler versions.
> >
> > The goal here is to create some sort of script which can collect
> > patches from the mailing list, apply them and test them on a selection
> > of boards. I suspect that script already exists, so please let me know
> > what you suggest.
>
> I still really wish you would split the labgrid parts of this series out
> from the pytest fixes and improvement parts. Things are largely fine
> outside of the labgrid part for example, which I'll probably just take
> after confirming it doesn't break using labgrid other ways.
I have had a few attempts at this, but it ends up being somewhat arbitrary.
Perhaps I could split out a series with the exception-handling changes
and the typo? Perhaps 'Avoid failing skipped tests' also?
Regards,
Simon
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v6 00/19] labgrid: Provide an integration with Labgrid
2024-09-25 12:50 ` Simon Glass
@ 2024-09-25 17:26 ` Tom Rini
0 siblings, 0 replies; 37+ messages in thread
From: Tom Rini @ 2024-09-25 17:26 UTC (permalink / raw)
To: Simon Glass; +Cc: u-boot
[-- Attachment #1: Type: text/plain, Size: 3548 bytes --]
On Wed, Sep 25, 2024 at 02:50:08PM +0200, Simon Glass wrote:
> Hi Tom,
>
> On Mon, 23 Sept 2024 at 22:36, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Fri, Sep 20, 2024 at 08:01:35AM +0200, Simon Glass wrote:
> >
> > > Labgrid provides access to a hardware lab in an automated way. It is
> > > possible to boot U-Boot on boards in the lab without physically touching
> > > them. It relies on relays, USB UARTs and SD muxes, among other things.
> > >
> > > By way of background, about 4 years ago I wrong a thing called Labman[1]
> > > which allowed my lab of about 30 devices to be operated remotely, using
> > > tbot for the console and build integration. While it worked OK and I
> > > used it for many bisects, I didn't take it any further.
> > >
> > > It turns out that there was already an existing program, called Labgrid,
> > > which I did not know about at time (thank you Tom for telling me). It is
> > > more rounded than Labman and has a number of advantages:
> > >
> > > - does not need udev rules, mostly
> > > - has several existing users who rely on it
> > > - supports multiple machines exporting their devices
> > >
> > > It lacks a 'lab check' feature and a few other things, but these can be
> > > remedied.
> > >
> > > On and off over the past several weeks I have been experimenting with
> > > Labgrid. I have managed to create an initial U-Boot integration (this
> > > series) by adding various features to Labgrid[2] and the U-Boot test
> > > hooks.
> > >
> > > I hope that this might inspire others to set up boards and run tests
> > > automatically, rather than relying on infrequent, manual test. Perhaps
> > > it may even be possible to have a number of labs available.
> > >
> > > Included in the integration are a number of simple scripts which make it
> > > easy to connect to boards and run tests:
> > >
> > > ub-int <target>
> > > Build and boot on a target, starting an interactive session
> > >
> > > ub-cli <target>
> > > Build and boot on a target, ensure U-Boot starts and provide an interactive
> > > session from there
> > >
> > > ub-smoke <target>
> > > Smoke test U-Boot to check that it boots to a prompt on a target
> > >
> > > ub-bisect <target>
> > > Bisect a git tree to locate a failure on a particular target
> > >
> > > ub-pyt <target> <testspec>
> > > Run U-Boot pytests on a target
> > >
> > > Some of these help to provide the same tbot[4] workflow which I have
> > > relied on for several years, albeit much simpler versions.
> > >
> > > The goal here is to create some sort of script which can collect
> > > patches from the mailing list, apply them and test them on a selection
> > > of boards. I suspect that script already exists, so please let me know
> > > what you suggest.
> >
> > I still really wish you would split the labgrid parts of this series out
> > from the pytest fixes and improvement parts. Things are largely fine
> > outside of the labgrid part for example, which I'll probably just take
> > after confirming it doesn't break using labgrid other ways.
>
> I have had a few attempts at this, but it ends up being somewhat arbitrary.
>
> Perhaps I could split out a series with the exception-handling changes
> and the typo? Perhaps 'Avoid failing skipped tests' also?
Well, in a previous series I said where I thought each patch went in
terms of what-it-did, so yes, that sounds like one of the sets of
patches I thought was unrelated to labgrid. Thanks.
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 37+ messages in thread