On Tue, Mar 15, 2022 at 09:13:15PM -0600, Simon Glass wrote:
Hi Masami,
On Tue, 15 Mar 2022 at 02:36, Masami Hiramatsu masami.hiramatsu@linaro.org wrote:
Hi Simon,
2022年3月15日(火) 14:04 Simon Glass sjg@chromium.org:
Hi Masami,
On Mon, 14 Mar 2022 at 18:40, Masami Hiramatsu masami.hiramatsu@linaro.org wrote:
Hi Simon,
2022年3月15日(火) 3:24 Simon Glass sjg@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?
yeah, if the UEFI is not initialized yet, we can run some UEFI related command (e.g. printenv -e) instead of efidebug capsule... to execute the capsule update on disk. But anyway, this is only available at the first time. We need a way to reset UEFI subsystem without system reset.
Yes. It is certainly possible, but I'm not sure how easy it is. Perhaps just drop all the EFI data structures and run the EFI init again? We have something similar with driver model. See dm_test_pre_run()
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.
My another question is how small is the granularity of the unit test. As I showed, the UEFI capsule update needs to prepare a capsule file installed in the storage. That seems to be very system-level. But you think that is still be a unit test? (I expected that the 'Unit test' is something like KUnit in Linux)
Well I am using your terminology here. Technically many of the U-Boot tests (executed by 'ut' command) are not really unit tests. They bring in a lot of code and run one test case using it.
For example, one of the tests brings up the USB subsystem, including a fake USB stick, then checks it can read data from the stick, using the USB stack.
Another one writes some things to the emulated display and then checks that the correct pixels are there.
Perhaps a better name would be integration test. But the point is that we can run these tests very, very quickly and (setup aside) without outside influence, or without restarting the executable, etc.
I don't really understand why you want and need to avoid a real reset in, what you call, integration test.
-Takahiro Akashi
> > 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