[PATCH v5 3/4] test/py: Handle expected reboot while booting sandbox

[Tom Rini]

On Sun, Feb 27, 2022 at 02:45:36PM -0700, Simon Glass wrote:
> Hi Tom,
> 
> On Sun, 27 Feb 2022 at 13:58, Tom Rini <trini at konsulko.com> wrote:
> >
> > On Sun, Feb 27, 2022 at 12:11:01PM -0700, Simon Glass wrote:
> > > Hi Tom,
> > >
> > > On Sun, 27 Feb 2022 at 11:14, Tom Rini <trini at konsulko.com> wrote:
> > > >
> > > > On Sun, Feb 27, 2022 at 08:39:30AM -0700, Simon Glass wrote:
> > > > > Hi Heinrich,
> > > > >
> > > > > On Sun, 27 Feb 2022 at 01:22, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
> > > > > >
> > > > > > On 2/26/22 19:37, Simon Glass wrote:
> > > > > > > Hi Masami,
> > > > > > >
> > > > > > > On Tue, 15 Feb 2022 at 23:16, Masami Hiramatsu
> > > > > > > <masami.hiramatsu at linaro.org> wrote:
> > > > > > >>
> > > > > > >> Add expected_reset optional argument to ConsoleBase::ensure_spawned(),
> > > > > > >> ConsoleBase::restart_uboot() and ConsoleSandbox::restart_uboot_with_flags()
> > > > > > >> so that it can handle a reset while the 1st boot process after main
> > > > > > >> boot logo before prompt correctly.
> > > > > > >>
> > > > > > >> Signed-off-by: Masami Hiramatsu <masami.hiramatsu at linaro.org>
> > > > > > >> ---
> > > > > > >>   Changes in v5:
> > > > > > >>    - Rename parameter to expect_reset and update the description to clarify
> > > > > > >>      the reset will happen between main boot and the command prompt.
> > > > > > >> ---
> > > > > > >>   test/py/u_boot_console_base.py    |   48 ++++++++++++++++++++++---------------
> > > > > > >>   test/py/u_boot_console_sandbox.py |    7 ++++-
> > > > > > >>   2 files changed, 33 insertions(+), 22 deletions(-)
> > > > > > >>
> > > > > > >
> > > > > > > Didn't I already comment on this patch? Why did it come back?
> > > > > >
> > > > > > Dear Simon,
> > > > > >
> > > > > > The discussion is in
> > > > > > https://patchwork.ozlabs.org/project/uboot/patch/164491595065.536855.9457820061065514578.stgit@localhost/
> > > > > >
> > > > > > You suggested: "We have a means to avoid actually doing the reset, see
> > > > > > the reset driver."
> > > > > >
> > > > > > We need a real reset on the sandbox and no fake reset as already said in
> > > > > > the referenced thread.
> > > > >
> > > > > Why?
> > > > >
> > > > > The fake reset is there for use by tests. We don't need this load of
> > > > > Python code at all for sandbox. We should worry about it later.
> > > >
> > > > Well, isn't this going to make the tests more sandbox-centric then, and
> > > > then need changes later to be able to test on real hardware?
> > >
> > > Yes, but it keeps the sandbox case simple. At present the sandbox
> > > tests can run within U-Boot (see the 'ut' command) and I very much
> > > want to keep it that way. That is, after all, why I wrote the reset
> > > driver.
> > >
> > > While tests on real hardware have value, I hope that all logic bugs
> > > are found on sandbox.
> >
> > Yes, it's important to test the code in sandbox before testing it on
> > hardware, to avoid "obvious" oops-it-broke changes, it's still very
> > important to be able to easily run this on real hardware.  Ideally, I
> > hope to see updates to the pytest hook repository to flash the hardware
> > via capsule, as well as running a more formal pytest on hardware.  To
> 
> Can you be specific about what bugs you are trying to catch in that
> case? I am conscious of the nightmare that is Zephyr's thousands of
> QEMU-based tests that take 20 minutes to run in parallel on a 64-core
> machine, so I'd would like to make sure that real bugs are found in
> unit tests where possible.
> 
> > that end, is it not most important to make sandbox look like a real
> > hardware platform, rather than adapt the test to know about special
> > sandbox things?  Or am I missing something here and the test shouldn't
> 
> The key thing is that sandbox runs essentially the same 'code under
> test' as the real board and that we can quickly verified (using the
> 'ut xxx' command) that it works. In this case, we want to run the EFI
> code under sandbox and make sure that it works.
> 
> > need changes / special handling to support both sandbox and real
> > hardware, with what you're suggesting?
> 
> The title of this patch refers to specific hacks in pytest to handle
> sandbox, doesn't it? So I think this is around the wrong way...that is
> in fact my objection. It simply should not be done that way, with
> special code in pytest, or whatever.
> 
> It should be possible to run 'ut xxx' and have the test run from start
> to finish, without any outside influence. Sandbox has a sysreset
> driver, just like any other board. We can make it do whatever we
> want...see sandbox_sysreset_request().
> 
> We do use pytest to set things up beforehand, or to verify that things
> worked after the run, but we should not need it to even just run a
> unit test. In particular, it should not be necessary for sandbox to be
> restarted by an outside influence.

Maybe we're talking at cross purposes here, or maybe I misread which
sets of tests this is ultimately for.  Functionality wise, I'm talking
about the capsule update functionality, and I want to ensure that we can
have something under test/py/tests/test_efi_capsule/ to update U-Boot
for a platform.  And that test should be abstracted such that it can run
on (a) sandbox (b) qemu as platformX (c) platformX in a HW lab.  If that
doesn't require changes under test/py/u_boot_console_sandbox.py then,
great!  I've just been confused.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20220228/6a83607a/attachment.sig>