[U-Boot] [PATCH v3 7/8] x86: efi: Add a hello world test program

[Simon Glass]

Hi Alex,

On 7 November 2016 at 09:32, Alexander Graf <agraf at suse.de> wrote:
>
>
> On 07/11/2016 10:46, Simon Glass wrote:
>>
>> Hi Alex,
>>
>> On 19 October 2016 at 01:09, Alexander Graf <agraf at suse.de> wrote:
>>>
>>>
>>>
>>> On 18/10/2016 22:37, Simon Glass wrote:
>>>>
>>>> Hi Alex,
>>>>
>>>> On 18 October 2016 at 01:14, Alexander Graf <agraf at 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 at 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.

Regards,
Simon