Skip to content
This repository has been archived by the owner on Jan 12, 2021. It is now read-only.

Add network and efi_loader test coverage to Travis-CI #9

Closed
wants to merge 4 commits into from

Conversation

agraf
Copy link
Contributor

@agraf agraf commented Nov 16, 2016

So far the Travis checks did not include network or efi tests. With these patches, they can.

To actually make use of these, additional patches to the U-Boot tree are necessary. But having the files in this tree first shouldn't hurt.

@agraf agraf force-pushed the master branch 4 times, most recently from d477232 to dc5e042 Compare November 17, 2016 16:56
@agraf
Copy link
Contributor Author

agraf commented Nov 17, 2016

Sorry, it's now ready to be merged :)

Copy link
Owner

@swarren swarren left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks OK overall; just a couple nits. Once you've confirmed that the first commit is OK I can apply that now, and wait for the second commit later.

@@ -21,7 +21,7 @@
console_impl=qemu
qemu_machine="vexpress-a15"
qemu_binary="qemu-system-arm"
qemu_extra_args="-nographic -m 1G -tftp /tftpboot"
qemu_extra_args="-nographic -m 1G -tftp ${UBOOT_TRAVIS_BUILD_DIR}"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The x86 script has "-netdev user,id=net0" in qemu_extra_args. Is something similar not needed for the vexpress scripts too, or do they default to having an emulated NIC?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The x86 script creates a new e1000 network device (-device e1000) and user space network as backend for it. The arm ones already have a network device as part of their board definition and the -tftp option modifies properties of the user space netdev that is spawned by the board.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this is also another copy/paste example from my local setup. Telling QEMU to add a network device is more along the lines of "Add this type of thing here" rather than "add a sensible network device" sadly.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, it depends. You can use the old-style -net commands. qemu-system-arm -M vexpress-a15 -net nic -net user,... should give you a similar effect to the -device and -netdev commands, just that device creation happens explicitly in the board file rather than generic bus slot magic.

But this is diverging and going into QEMU internals. All of the variants should work to some extent (except for -device for sysbus devices). It doesn't actually matter which one we use, as long as the guest gets to see the correct network device.

@@ -12,3 +12,11 @@
"size": os.path.getsize(u_boot_bin),
"crc32": hex(binascii.crc32(open(u_boot_bin, 'rb').read()) & 0xffffffff)[2:],
}

helloworld_bin = os.environ['UBOOT_TRAVIS_BUILD_DIR'] + "/lib/efi_loader/helloworld.efi"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess I'll wait to apply this commit until I see the u-boot.git changes that implement this get checked in (or you let me know if I miss it).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They should be pretty much independent of each other :)

env__efi_loader_helloworld_file = {
"fn": "lib/efi_loader/helloworld.efi",
"size": os.path.getsize(helloworld_bin),
"crc32": hex(binascii.crc32(open(helloworld_bin, 'rb').read()) & 0xffffffff)[2:],
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is used a lot now! I'd suggest implementing a utilty module/function for set the crc32 value, or perhaps even to create the whole dict object.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think having one for the whole dict makes a lot of sense. Especially because there will be more files coming with grub2 testing.

The following patches will allow us to share files between the
build host and the qemu instance running our test case.

To reduce code duplication, expose a simple helper wrapper that
creates an env dict including checksum from a file name.

Signed-off-by: Alexander Graf <agraf@suse.de>
With Travis we can just load files from the host directory structure
into our guests and verify network connectivity that way. Do so for all
boards that we currently already expose tftp on.

While at it, expose the u-boot output binary as test file for the tftp
test case.

Signed-off-by: Alexander Graf <agraf@suse.de>
We have a new test framework for efi_loader in test/py now. That framework
can fetch a hello world application via tftp, but it needs to have it
referenced in the environment.

Fortunately U-Boot builds said hello world efi binary as part of its build
process, so let's expose that to the test case.

Signed-off-by: Alexander Graf <agraf@suse.de>
Out test framework for efi_loader in test/py gained support to test grub2
functionality. Expose the respective grub2 file as environment variable
to the test, so that it can access it.

Signed-off-by: Alexander Graf <agraf@suse.de>
@agraf
Copy link
Contributor Author

agraf commented Nov 18, 2016

Alright, I updated the repo. Better now?

Copy link
Contributor

@trini trini left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, thanks!

@@ -0,0 +1,11 @@
import os
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eventually we should move this into some common location that all hosts can share, perhaps even into the U-Boot test/py source tree. However, putting it here is just fine for now.

@swarren
Copy link
Owner

swarren commented Nov 18, 2016

Applied, thanks.

P.S. I'm rather swamped at work; I haven't had time to look at the U-Boot side of these changes (i.e. the new test scripts). Not sure when I'll get to that. Either way, the changes here look fine and can easily be updated if needed due to review of the test/py code.

@swarren swarren closed this Nov 18, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants