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

[Alexander Graf]

On 14/11/2016 21:44, Simon Glass wrote:
> Hi Alex,
>
> On 11 November 2016 at 23:23, Alexander Graf <agraf at suse.de> wrote:
>>
>>
>>> 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.
>
> OK good, well please can you review this at some point?

Review what exactly?

> Also, are you
> planning to write the 'larger' test? How do you test this all in suse?

Planning yes, but I'm very good at not writing tests :).

Currently I'm testing this all in suse by running systems which rely on 
the code to work.


Alex