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

[Alexander Graf]

> Am 11.11.2016 um 17:17 schrieb Simon Glass <sjg at chromium.org>:
> 
> 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.

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.


Alex