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

[Simon Glass]

Hi Alex,

On 14 November 2016 at 14:24, Alexander Graf  wrote:
>
>
> On 14/11/2016 21:58, Simon Glass wrote:
>>
>> Hi Alex,
>>
>> On 14 November 2016 at 13:46, Alexander Graf  wrote:
>>>
>>>
>>>
>>> On 14/11/2016 21:44, Simon Glass wrote:


 Hi Alex,

 On 11 November 2016 at 23:23, Alexander Graf  wrote:
>
>
>
>
>> Am 11.11.2016 um 17:17 schrieb Simon Glass :
>>
>> Hi Alex,
>>
>>> On 7 November 2016 at 09:32, Alexander Graf  wrote:
>>>
>>>
 On 07/11/2016 10:46, Simon Glass wrote:

 Hi Alex,

> On 19 October 2016 at 01:09, Alexander Graf  wrote:
>
>
>
>> On 18/10/2016 22:37, Simon Glass wrote:
>>
>> Hi Alex,
>>
>>> On 18 October 2016 at 01:14, Alexander Graf 
>>> 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 
 ---

 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.
>>

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

[Alexander Graf]

On 14/11/2016 21:58, Simon Glass wrote:

Hi Alex,

On 14 November 2016 at 13:46, Alexander Graf  wrote:



On 14/11/2016 21:44, Simon Glass wrote:


Hi Alex,

On 11 November 2016 at 23:23, Alexander Graf  wrote:





Am 11.11.2016 um 17:17 schrieb Simon Glass :

Hi Alex,


On 7 November 2016 at 09:32, Alexander Graf  wrote:



On 07/11/2016 10:46, Simon Glass wrote:

Hi Alex,


On 19 October 2016 at 01:09, Alexander Graf  wrote:




On 18/10/2016 22:37, Simon Glass wrote:

Hi Alex,


On 18 October 2016 at 01:14, Alexander Graf  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 
---

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.



Alex
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

[Simon Glass]

Hi Alex,

On 14 November 2016 at 13:46, Alexander Graf  wrote:
>
>
> On 14/11/2016 21:44, Simon Glass wrote:
>>
>> Hi Alex,
>>
>> On 11 November 2016 at 23:23, Alexander Graf  wrote:
>>>
>>>
>>>
 Am 11.11.2016 um 17:17 schrieb Simon Glass :

 Hi Alex,

> On 7 November 2016 at 09:32, Alexander Graf  wrote:
>
>
>> On 07/11/2016 10:46, Simon Glass wrote:
>>
>> Hi Alex,
>>
>>> On 19 October 2016 at 01:09, Alexander Graf  wrote:
>>>
>>>
>>>
 On 18/10/2016 22:37, Simon Glass wrote:

 Hi Alex,

> On 18 October 2016 at 01:14, Alexander Graf  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 
>> ---
>>
>> 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 

Re: [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  wrote:




Am 11.11.2016 um 17:17 schrieb Simon Glass :

Hi Alex,


On 7 November 2016 at 09:32, Alexander Graf  wrote:



On 07/11/2016 10:46, Simon Glass wrote:

Hi Alex,


On 19 October 2016 at 01:09, Alexander Graf  wrote:




On 18/10/2016 22:37, Simon Glass wrote:

Hi Alex,


On 18 October 2016 at 01:14, Alexander Graf  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 
---

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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

[Simon Glass]

Hi Alex,

On 11 November 2016 at 23:23, Alexander Graf  wrote:
>
>
>> Am 11.11.2016 um 17:17 schrieb Simon Glass :
>>
>> Hi Alex,
>>
>>> On 7 November 2016 at 09:32, Alexander Graf  wrote:
>>>
>>>
 On 07/11/2016 10:46, Simon Glass wrote:

 Hi Alex,

> On 19 October 2016 at 01:09, Alexander Graf  wrote:
>
>
>
>> On 18/10/2016 22:37, Simon Glass wrote:
>>
>> Hi Alex,
>>
>>> On 18 October 2016 at 01:14, Alexander Graf  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 
 ---

 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? Also, are you
planning to write the 'larger' test? How do you test this all in suse?

Regards,
Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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


___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [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  wrote:
>
>
> On 07/11/2016 10:46, Simon Glass wrote:
>>
>> Hi Alex,
>>
>> On 19 October 2016 at 01:09, Alexander Graf  wrote:
>>>
>>>
>>>
>>> On 18/10/2016 22:37, Simon Glass wrote:

 Hi Alex,

 On 18 October 2016 at 01:14, Alexander Graf  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 
>> ---
>>
>> 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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

[Alexander Graf]

On 07/11/2016 10:46, Simon Glass wrote:

Hi Alex,

On 19 October 2016 at 01:09, Alexander Graf  wrote:



On 18/10/2016 22:37, Simon Glass wrote:

Hi Alex,

On 18 October 2016 at 01:14, Alexander Graf  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 
---

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.







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.



Alex
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

[Simon Glass]

Hi Alex,

On 19 October 2016 at 01:09, Alexander Graf  wrote:
>
>
> On 18/10/2016 22:37, Simon Glass wrote:
>> Hi Alex,
>>
>> On 18 October 2016 at 01:14, Alexander Graf  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 
 ---

 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?

>
>> 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.

Regards,
Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

[Alexander Graf]

On 18/10/2016 22:37, Simon Glass wrote:
> Hi Alex,
> 
> On 18 October 2016 at 01:14, Alexander Graf  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 
>>> ---
>>>
>>> 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.

> 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).


Alex
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

[Simon Glass]

Hi Alex,

On 18 October 2016 at 01:14, Alexander Graf  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 
>> ---
>>
>> 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.

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.

Regards,
Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

[Alexander Graf]

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 
---

Changes in v3:
- Include a link to the program instead of adding it to the tree


So, uh, where is the link?

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?



Alex

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

[Bin Meng]

On Tue, Oct 18, 2016 at 10: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 
> ---
>
> Changes in v3:
> - Include a link to the program instead of adding it to the tree
>
> Changes in v2: None
>
>  arch/x86/lib/Makefile | 1 +
>  1 file changed, 1 insertion(+)
>

Reviewed-by: Bin Meng 
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

[Simon Glass]

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 
---

Changes in v3:
- Include a link to the program instead of adding it to the tree

Changes in v2: None

 arch/x86/lib/Makefile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/x86/lib/Makefile b/arch/x86/lib/Makefile
index e46e7f1..1d9ebc0 100644
--- a/arch/x86/lib/Makefile
+++ b/arch/x86/lib/Makefile
@@ -12,6 +12,7 @@ obj-$(CONFIG_CMD_BOOTM) += bootm.o
 obj-y  += cmd_boot.o
 obj-$(CONFIG_SEABIOS) += coreboot_table.o
 obj-$(CONFIG_EFI) += efi/
+obj-$(CONFIG_CMD_BOOTEFI_HELLO) += HelloWorld32.o
 obj-y  += e820.o
 obj-y  += gcc.o
 obj-y  += init_helpers.o
-- 
2.8.0.rc3.226.g39d4020

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot