Hi Alex,
On 14 November 2016 at 14:24, Alexander Graf agraf@suse.de wrote:
On 14/11/2016 21:58, Simon Glass wrote:
Hi Alex,
On 14 November 2016 at 13:46, Alexander Graf agraf@suse.de wrote:
On 14/11/2016 21:44, Simon Glass wrote:
Hi Alex,
On 11 November 2016 at 23:23, Alexander Graf agraf@suse.de wrote:
Am 11.11.2016 um 17:17 schrieb Simon Glass sjg@chromium.org:
Hi Alex,
> On 7 November 2016 at 09:32, Alexander Graf agraf@suse.de wrote: > > >> On 07/11/2016 10:46, Simon Glass wrote: >> >> Hi Alex, >> >>> On 19 October 2016 at 01:09, Alexander Graf agraf@suse.de wrote: >>> >>> >>> >>>> On 18/10/2016 22:37, Simon Glass wrote: >>>> >>>> Hi Alex, >>>> >>>>> On 18 October 2016 at 01:14, Alexander Graf agraf@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 sjg@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