[PATCH] test/py: efi_capsule: Handle expected reset after capsule on disk

[Simon Glass]

Hi Masami,

On Mon, 14 Mar 2022 at 18:40, Masami Hiramatsu
<masami.hiramatsu at linaro.org> wrote:
>
> Hi Simon,
>
> 2022年3月15日(火) 3:24 Simon Glass <sjg at chromium.org>:
> >
> > > > OK, well 'reset by a user' presumably starts the board up and then
> > > > runs some code to do the update in U-Boot? Is that right? If so, we
> > > > just need to trigger that update from the test. We don't need to test
> > > > the actual reset, at least not with sandbox. As I said, we need to
> > > > write the code so that it is easy to test.
> > >
> > > Actually, we already have that command, "efidebug capsule disk-update"
> > > which kicks the capsule update code even without the 'reset by a
> > > user'. So we can just kick this command for checking whether the
> > > U-Boot UEFI code correctly find the capsule file from ESP which
> > > specified by UEFI vars.
> > >
> > > However, the 'capsule update on-disk' feature is also expected (and
> > > defined in the spec?) to run when the UEFI subsystem is initialized.
> > > This behavior will not be tested if we skip the 'reset by a user'. I
> > > guess Takahiro's current test case tries to check it.
> >
> > The 'UEFI subsystem is intialised' is a problem, actually, since if it
> > were better integrated into driver model, it would not have separate
> > structures or they would be present and enabled when driver model is.
> > I hope that it can be fixed and Takahiro's series is a start in that
> > direction.
>
> OK.
>
> > But as to a test that an update is called when UEFI starts, that seems
> > like a single line of code. Sure it is nice to test it, but it is much
> > more important to test the installation of the update and the
> > execution of the update. I suppose another way to test that is  to
> > shut down the UEFI subsystem and start it up?
>
> Yes, currently we call do_reset() after install the capsule file.
> (This reset can be avoided if we replace it with
> sysreset_walk_halt(SYSRESET_COLD) as you said, right?)
>
> Here is how I tested it on my machine;
>
> > usb start
> > fatload usb 0 $kernel_addr_r test.cap
> > fatwrite mmc 0 $fileaddr EFI/UpdateCapsule/test.cap $filesize
> > efidebug capsule disk-update
> (run install process and reboot the machine)
>
> So, if we can avoid the last reset, we can test the below without
> reset on sandbox (depends on scenarios).
> - confirm that the capsule update on disk can find the capsule file
> from ESP specified by the BOOTXXXX EFI variable.
> - confirm that the capsule update on disk writes the firmware
> correctly to the storage which specified by DFU.
> - confirm that the capsule update on disk success if the capsule image
> type is supported.
> - confirm that the capsule update on disk fails if the capsule image
> type is not supported.
> - confirm that the capsule update on disk will reboot after update
> even if the update is failed.
>
> The only spec we can not test is
> - confirm that the capsule update on disk is kicked when the UEFI is
> initialized.

Even that could be tested, by installing an update and then initing UEFI?

>
> >
> > Anyway we should design subsystems so they are easy to test.
>
> Here I guess you mean the unit test, not system test, am I correct?

Yes. Easy testing is so important for developer productivity and
happiness. It is fine to have large system/functional tests as a fall
back or catch-all, but they tend to test the happy path only. When
they fail, they are hard to debug because they cover such as large
area of the code and they often have complex setup requirements so are
hard to run manually.

My hope is that all the functionality should be covered by unit tests
or integration tests, so that system/functional almost never fail.

>
> >
> > >
> > > > > Masami's patch (this series) fixes issues around those two resets
> > > > > in pytest.
> > > >
> > > > Yes and that is the problem I have, at least on sandbox.
> > >
> > > So If I I call sysreset_walk_halt(SYSRESET_COLD) after capsule update,
> > > it could help?
> >
> > Yes that can help, because sandbox can detect that and turn it into a nop.
>
> OK, let me submit a patch to update it.

OK thank you.

Regards,
Simon