Hi Simon,
2022年2月28日(月) 22:56 Simon Glass sjg@chromium.org:
Hi,
On Mon, 28 Feb 2022 at 06:48, Tom Rini trini@konsulko.com wrote:
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@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@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@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@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@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.9457820... > > > > 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.
My point is that we should start with a test that runs on sandbox. It should be a unit test, i.e. 'ut xxx' and not require changes in the console handling and should use the sysreset driver to operate. This should test *all* the code of the new feature, including failure modes.
Hmm, but the capsule update on disk requires to prepare - the capsule file image which is properly composed - the disk which has an EFI system partition - the EFI variables required for the capsule update on disk I guess this is the main reason why the test is written by the script...
Thank you,
As to qemu and other platforms, they are really just going to be happy-path tests. I suggest splitting the reset functionality out so that it happens after the update is ready. Perhaps it is possible for the test itself to be in control of the reset? We don't really need to test that a board can reset, right? If we do, then we are going to need some magic like this patch, but it is quite messy.
So please, unit test first.
Regards, Simon