Linaro Image Tools

Merge lp://qastaging/~pfalcon/linaro-image-tools/copy_files-lhc into lp://qastaging/linaro-image-tools/11.11

copy_files-lhc
Merge into trunk

Proposed by Paul Sokolovsky on 2012-08-29

Status:	Merged
Merged at revision:	557
Proposed branch:	lp://qastaging/~pfalcon/linaro-image-tools/copy_files-lhc
Merge into:	lp://qastaging/linaro-image-tools/11.11
Diff against target:	486 lines (+251/-49) 14 files modified linaro_image_tools/hwpack/builder.py (+98/-46) linaro_image_tools/hwpack/hardwarepack.py (+3/-1) linaro_image_tools/hwpack/tests/data/pkgs/README (+4/-0) linaro_image_tools/hwpack/tests/data/pkgs/make-mock-debs.sh (+12/-0) linaro_image_tools/hwpack/tests/data/pkgs/mock-base/mock-base.control (+7/-0) linaro_image_tools/hwpack/tests/data/pkgs/mock-u-boot1/copyright (+2/-0) linaro_image_tools/hwpack/tests/data/pkgs/mock-u-boot1/mock-u-boot1.control (+9/-0) linaro_image_tools/hwpack/tests/data/pkgs/mock-u-boot1/u-boot.img (+2/-0) linaro_image_tools/hwpack/tests/data/pkgs/mock-u-boot2/copyright (+2/-0) linaro_image_tools/hwpack/tests/data/pkgs/mock-u-boot2/mock-u-boot2.control (+9/-0) linaro_image_tools/hwpack/tests/data/pkgs/mock-u-boot2/u-boot.img (+2/-0) linaro_image_tools/hwpack/tests/data/test.hwpack.conf (+49/-0) linaro_image_tools/hwpack/tests/test_builder.py (+18/-2) linaro_image_tools/hwpack/tests/test_linaro_hwpack_create.py (+34/-0)
To merge this branch:	bzr merge lp://qastaging/~pfalcon/linaro-image-tools/copy_files-lhc
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
James Tunnicliffe (community)		2012-08-29	Needs Fixing on 2012-08-29
Review via email: mp+121875@code.qastaging.launchpad.net

Description of the change

This implements initial "copy_files" element support (only simple "source" file specification per original hwpack spec, not extended form being settled now), and related changes to make it work, like getting rid of hardcoded bootloader paths, proper namespacing of multiple bootloaders (both global and board-specific), other discovered issues. It allows adds framework for end-to-end tests for linaro-hwpack-create utility (from hwpack config to hwpack tarball).

Following yet is on todo:

1. Implement full copy_files syntax, once it is finalized.
2. Remove old unused code and hardcodings.
3. Elaborate tests.

It is recommended to follow the work started in this branch (i.e. fix any issues and continue with the list above), or at least use it as the base for implementing copy_files support in linaro-media-create tool.

lp://qastaging/~pfalcon/linaro-image-tools/copy_files-lhc updated on 2012-08-29

564. By Paul Sokolovsky on 2012-08-29: Remove _set_new_values(), fully replaced by extract_files().

Revision history for this message

James Tunnicliffe (dooferlad) wrote on 2012-08-29:

If you look in linaro_image_tools/tests/test_script.py you will find that it is dedicated to testing linaro-hwpack-create. I don't know if the intention was to test other scripts as well, but it doesn't. I just added a V3 hardware pack test to it of my own. If you take a look you will see that there is a package mocking set up that you can use instead of introducing mock packages.

=== added file 'linaro_image_tools/hwpack/tests/data/test.hwpack.conf'
In the YAML you have added here there are several commented out lines. Probably better if they are deleted. Since I have just introduced a bit of YAML into the hwpack testing, we can look at sharing it.

The added non-test code looks fine (perhaps a bit light in the docstring department). Since you are on holiday I will address these issues if possible and create a new MP.

Thanks for this. I am not surprised you missed the existing linaro-hwpack-create tests. I will probably rename the file they are in to make them easier to find!

review: Needs Fixing

Revision history for this message

Paul Sokolovsky (pfalcon) wrote on 2012-08-29:

> In the YAML you have added here there are several commented out lines.

Definitely, as I noted in teh patch description, that needs further elaboration cleanup. That's basically as much as was able to do today.

> If you look in linaro_image_tools/tests/test_script.py
> If you take a look you will see that there is a package mocking set up that you can use instead of introducing mock packages.

Well, looking into it, it's way too cryptic. I kinda see that there a script really appears to be run, but I have no idea if it's real integration tests which create mock packages and make tool operate on them, or there's still some unit-testing short-cuts. And if it fails, how do I investigate the failure? First approach is to run the tool manually with the same packages/config, where are those? All I can see is again cryptic stuff like "'[hwpack]\nname=ahwpack\npackages=%s\narchitectures=armel\n'
'\n[ubuntu]\nsources-entry=%s\n'"

That's why I keep asking - more data-drivenness, folks, please. If creation of .deb packages is outside of our test scope, let's exclude it from tests, and just have a good test data corpus (and that can be created/maintained by well-known external tools). Then, when you want to write a new integration test, you just grep tree for .deb files (granted, that's what I did to see if I should start on them), find one, see where they're referenced, clone into new test, easily debuggable too.

So, I consider approach started by test_linaro_hwpack_create.py a big improvement wrt what was before ;-).

Thanks for taking care of this further!