[U-Boot] [PATCH 00/23] efi_loader implement missing functions

[Simon Glass]

Hi Heinrich,

On 30 August 2017 at 06:03, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>
>
> On 08/29/2017 10:38 PM, Alexander Graf wrote:
>>
>>
>>> Am 29.08.2017 um 22:16 schrieb Simon Glass <sjg at chromium.org>:
>>>
>>> Hi,
>>>
>>>> On 29 August 2017 at 22:16, Rob Clark <robdclark at gmail.com> wrote:
>>>>> On Tue, Aug 29, 2017 at 8:57 AM, Leif Lindholm <leif.lindholm at linaro.org> wrote:
>>>>> On Tue, Aug 29, 2017 at 02:26:48PM +0200, Alexander Graf wrote:
>>>>>>>>> I would add command
>>>>>>>>> bootefi selftest.efi
>>>>>>>>> to run the tests and provide the python wrapper code to add it to the
>>>>>>>>> test suite.
>>>>>>>>
>>>>>>>> I think that's a great idea, yes.
>>>>>>> I wonder how far we are from running UEFI tests (either the official
>>>>>>> ones, or I seem to remember hearing about some other test suite which
>>>>>>> didn't require UEFI shell)?
>>>>>>
>>>>>> Let's ask Leif, Ard and Dong.
>>>>>>
>>>>>> The official test suite definitely needs the UEFI Shell. Is the suite
>>>>>> publicly available by now?
>>>>>
>>>>> In binary form, you can access it already from the links on
>>>>> http://uefi.org/testtools
>>>>>
>>>>> Yes, 2.5 is latest release. No this is not a restriction ... the SCT
>>>>> releases have been lagging the specification releases a fair bit.
>>>>>
>>>>> The 2.5a package contains AARCH64, IA32 and X64 support (not ARM).
>>>>> Not that it couldn't contain ARM, it just hasn't been packaged.
>>>>>
>>>>>>> That seems more useful long term than re-inventing comprehensive UEFI
>>>>>>> test suite.  (Also, beyond just running shim/fallback/grub, I don't
>>>>>>> really have time to invent new tests for the stack of efi_loader
>>>>>>> patches I have.)
>>>>>>
>>>>>> Yes and no - it depends on the availability of the official suite :/.
>>>>>
>>>>> UEFI SCT is not yet open source/free software. Obviously, this is
>>>>> something Linaro has been lobbying for since day one of our
>>>>> involvement. There used to be little understanding for this, but that
>>>>> attitude has shifted substantially.
>>>>
>>>> So, if/until UEFI SCT is not an option, what about:
>>>>
>>>>  https://01.org/linux-uefi-validation
>>>>
>>>> (thx to pjones for pointing that out to me)
>>>
>>> Well in any case I'm not looking for a large functional test suite at
>>> this stage. It certainly could be useful, but not as a replacement for
>>> unit tests. The latter is for fast verification (so that everyone can
>>> run it as part of 'make tests') and easy identification of the
>>> location of bugs.
>>
>> I fail to see the point here to be honest. The thing we care most about in the uefi implementation is abi compatibility with the spec. That's basically what SCT is supposed to get you. Running it should be a matter of seconds with kvm enabled, so 'make tests' should easily be able to run it if you have the correct target compiled.
>>
>>>
>>> These new tests should be written in C, run very quickly (similar to
>>> the existing tests) to verify that the code works. Then when each new
>>> feature is added, the test are expanded to cover the new
>>> functionality. Should the code be refactored, the tests should provide
>>> comfort that nothing broke.
>>
>> So far I've never seen that approach work well in (low level) open source projects, as you start to heavily raise the barrier for participation. Most people in this world do these tasks in their spare time and won't spend x2 the time to also write and update unit tests.

To Alex:

We need to encourage people to write tests in U-Boot for core code. As
a project we spend a lot more time maintaining and refactoring core
code than we do writing it. Also if people write the tests as they
develop it they generally discover some benefits - e.g. it is faster
to get something working if you have a quick test for it. There is
absolutely no way I could have developed driver model in any
reasonable time without tests.

U-Boot now has a fairly rich set of tests. There still are many gaps,
but this gain has been hard won and it is very valuable to the project
and future developers and users of U-Boot.

The problem with large functional tests is that they are too slow and
painful (and often flaky) for people to use in their normal
development flow. It takes a minute to run 'make tests' before sending
out a patch series, and you can particular tests in seconds. This is
very helpful for people to see that they have not broken something.
Breakages in core code are relatively expensive to debug, surprising
to users and adversely affect the reputation of the project.

>>
>> I really think having a working functional test suite that is easy to run is the best way forward in this case. We can then start thinking of ways to test hard to reach code sections to increase test coverage.
>>
>>
>> Alex
>
> Simon is right in that test code is needed to ensure that refactoring
> does not break existing functionality. The current git HEAD cannot read
> from an IDE disk anymore. This problem could have been caught by a
> functional test four weeks ago.
>
> Unit tests should be based on exposed APIs. Tests for lower level
> functions would not survive refactoring.
>
> We implement only part of the UEFI spec. If we do not want to blow up
> binary size we may even want to keep it that way. So a test suite for
> full UEFI compatibility will always fail partially if it runs at all.
>
> I am able to provide a test application that will cover the API
> functions that I have focused on (events, protocols, (dis)connect
> controllers). To limit my effort I would like to write it for the HEAD
> of my patch queue and not break it down into one test patch per
> implementation patch. I hope that is ok for you. My effort estimate is
> 40h+ so, please, be patient.

Yes that sounds like the right approach. I'd like to check about the
point you made about needing to write an EFI app to test things. Is it
not possible to build the test into the same binary as sandbox, and
make internal calls to the EFI API functions?

>
> I would be happy to receive some review for the queued patches in the
> meanwhile.

It is in my queue :-)

Regards,
Simon