Hi Alex, On 14 November 2016 at 14:24, Alexander Graf <ag...@suse.de> wrote: > > > On 14/11/2016 21:58, Simon Glass wrote: >> >> Hi Alex, >> >> On 14 November 2016 at 13:46, Alexander Graf <ag...@suse.de> wrote: >>> >>> >>> >>> On 14/11/2016 21:44, Simon Glass wrote: >>>> >>>> >>>> Hi Alex, >>>> >>>> On 11 November 2016 at 23:23, Alexander Graf <ag...@suse.de> wrote: >>>>> >>>>> >>>>> >>>>> >>>>>> Am 11.11.2016 um 17:17 schrieb Simon Glass <s...@chromium.org>: >>>>>> >>>>>> Hi Alex, >>>>>> >>>>>>> On 7 November 2016 at 09:32, Alexander Graf <ag...@suse.de> wrote: >>>>>>> >>>>>>> >>>>>>>> On 07/11/2016 10:46, Simon Glass wrote: >>>>>>>> >>>>>>>> Hi Alex, >>>>>>>> >>>>>>>>> On 19 October 2016 at 01:09, Alexander Graf <ag...@suse.de> wrote: >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>>> On 18/10/2016 22:37, Simon Glass wrote: >>>>>>>>>> >>>>>>>>>> Hi Alex, >>>>>>>>>> >>>>>>>>>>> On 18 October 2016 at 01:14, Alexander Graf <ag...@suse.de> >>>>>>>>>>> wrote: >>>>>>>>>>> >>>>>>>>>>>> On 10/18/2016 04:29 AM, Simon Glass wrote: >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> It is useful to have a basic sanity check for EFI loader >>>>>>>>>>>> support. >>>>>>>>>>>> Add >>>>>>>>>>>> a >>>>>>>>>>>> 'bootefi hello' command which loads HelloWord.efi and runs it >>>>>>>>>>>> under >>>>>>>>>>>> U-Boot. >>>>>>>>>>>> >>>>>>>>>>>> Signed-off-by: Simon Glass <s...@chromium.org> >>>>>>>>>>>> --- >>>>>>>>>>>> >>>>>>>>>>>> Changes in v3: >>>>>>>>>>>> - Include a link to the program instead of adding it to the tree >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> So, uh, where is the link? >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> I put it in the README (see the arm patch). >>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> I'm really not convinced this command buys us anything yet. I do >>>>>>>>>>> agree >>>>>>>>>>> that >>>>>>>>>>> we want automated testing - but can't we get that using QEMU and >>>>>>>>>>> a >>>>>>>>>>> downloadable image file that we pass in as disk and have the >>>>>>>>>>> distro >>>>>>>>>>> boot do >>>>>>>>>>> its magic? >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> That seems very heavyweight as a sanity check, although I agree it >>>>>>>>>> is >>>>>>>>>> useful. >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> It's not really much more heavy weight. The "image file" could >>>>>>>>> simply >>>>>>>>> contain your hello world binary. But with this we don't just verify >>>>>>>>> whether "bootefi" works, but also whether the default boot path >>>>>>>>> works >>>>>>>>> ok. >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> I don't think I understand what you mean by 'image file'. Is it >>>>>>>> something other than the .efi file? Do you mean a disk image? >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> Yes. For reasonable test coverage, we should also verify that the >>>>>>> distro >>>>>>> defaults wrote a sane boot script that automatically searches for a >>>>>>> default >>>>>>> EFI binary in /efi/boot/bootx86.efi on the first partition of all >>>>>>> devices >>>>>>> and runs it. >>>>>>> >>>>>>> So if we just provide an SD card image or hard disk image to QEMU >>>>>>> which >>>>>>> contains a hello world .efi binary as that default boot file, we >>>>>>> don't >>>>>>> only >>>>>>> test whether the "bootefi" command works, but also whether the distro >>>>>>> boot >>>>>>> script works. >>>>>> >>>>>> >>>>>> >>>>>> That's right. >>>>>> >>>>>>> >>>>>>>> >>>>>>>>> >>>>>>>>>> Here I am just making sure that EFI programs can start, print >>>>>>>>>> output >>>>>>>>>> and exit. It is a test that we can easily run without a lot of >>>>>>>>>> overhead, much less than a full distro boot. >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> Again, I don't think it's much more overhead and I do believe it >>>>>>>>> gives >>>>>>>>> us much cleaner separation between responsibilities of code (tests >>>>>>>>> go >>>>>>>>> where tests are). >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> You are talking about a functional test, something that tests things >>>>>>>> end to end. I prefer to at least start with a smaller test. Granted >>>>>>>> it >>>>>>>> takes a little more work but it means there are fewer things to hunt >>>>>>>> through when something goes wrong. >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> Yes, I personally find unit tests terribly annoying and unproductive >>>>>>> and >>>>>>> functional tests very helpful :). And in this case, the effort to >>>>>>> write >>>>>>> it >>>>>>> is about the same for both, just that the functional test actually >>>>>>> tells you >>>>>>> that things work or don't work at the end of the day. >>>>>>> >>>>>>> With a code base like U-Boot, a simple functional test like the above >>>>>>> plus >>>>>>> git bisect should get you to an offending patch very quickly. >>>>>> >>>>>> >>>>>> >>>>>> This is not a unit test - in fact the EFI stuff has no unit tests. I >>>>>> suppose if we are trying to find a name this is a small functional >>>>>> test since it exercises the general functionality. >>>>>> >>>>>> I am much keener on small tests than large ones for finding simple >>>>>> bugs. Of course you can generally bisect to find a bug, but the more >>>>>> layers of software you need to look for the harder this is. >>>>>> >>>>>> We could definitely use a pytest which checks an EFI boot into an >>>>>> image, but I don't think this obviates the need for a smaller targeted >>>>>> test like this one. >>>>> >>>>> >>>>> >>>>> I think arguing over this is moot :). More tests is usually a good >>>>> thing, >>>>> so whoever gets to write them gets to push them ;). As long as the >>>>> licenses >>>>> are sound at least. >>>> >>>> >>>> >>>> OK good, well please can you review this at some point? >>> >>> >>> >>> Review what exactly? >> >> >> I mean the patches. There should be ~14 in your queue. > > > Interesting. I see them on patchwork, but neither in my U-Boot folder, my > inbox nor my spam folder. I wonder what happened there.
I'm really not sure but I have seen similar things. I will see if I can figure it out. Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
