Hi Simon, 2022年2月28日(月) 22:56 Simon Glass <s...@chromium.org>: > > Hi, > > On Mon, 28 Feb 2022 at 06:48, Tom Rini <tr...@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 <tr...@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 <tr...@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.g...@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.hirama...@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.hirama...@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. > > 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 -- Masami Hiramatsu
