Update test harness to take in optional public and private keys to sign payloads.

Update test harness to take in optional public and private keys to sign payloads.

In addition some refactoring to make this cleaner and easier.

Change-Id: I1607700d065c71aff2b2833b10acbd3ebace68ce

BUG=chromium-os:8212
TEST=Ran with Simple ... running now with full

Committed: http://chrome-svn/viewvc/chromeos?view=rev&revision=8dad50d

[sosa, 2011-02-10 06:14:35]

http://codereview.chromium.org/6482017/diff/1/bin/cros_run_vm_update
File bin/cros_run_vm_update (right):

http://codereview.chromium.org/6482017/diff/1/bin/cros_run_vm_update#newcode38
bin/cros_run_vm_update:38: DEFINE_string image "" "Path of the image to update
to." u
Wanted to make this common with image_to_live

http://codereview.chromium.org/6482017/diff/4/bin/cros_au_test_harness.py
File bin/cros_au_test_harness.py (right):

http://codereview.chromium.org/6482017/diff/4/bin/cros_au_test_harness.py#new...
bin/cros_au_test_harness.py:64: def _PrepareRealBase(self, image_path):
These two are moved out so we can call them in the Pregenerate Class

http://codereview.chromium.org/6482017/diff/4/bin/cros_au_test_harness.py#new...
bin/cros_au_test_harness.py:191: key_to_use = private_key_path
Allows you to override so you can specify a bad key in a test.

http://codereview.chromium.org/6482017/diff/4/bin/cros_au_test_harness.py#new...
bin/cros_au_test_harness.py:841: command = ['./enter_chroot.sh',
Just a refactor to just append rather than +

http://codereview.chromium.org/6482017/diff/4/bin/cros_au_test_harness.py#new...
bin/cros_au_test_harness.py:863: if not test_result.wasSuccessful():
This was a bug being missing.

http://codereview.chromium.org/6482017/diff/4/bin/cros_au_test_harness.py#new...
bin/cros_au_test_harness.py:931: def _InsertPrivateKeyIntoImage(image_path,
private_key_path):
Core component that adds the public key in the right place.

[dgarrett, 2011-02-10 20:01:35]

This LGTM, but I'd like to do some cleanup in the future like we discussed in
person. I don't mind doing that after this is finished.

[sosa, 2011-02-11 00:11:38]

PTAL.

I found a bug in my previous code ... and I realized it's really difficult to
keep things in sync (with private / public keys).  I also realized that Simon's
force-copy change should have been defaulted to False.

[dgarrett, 2011-02-11 01:25:55]

Mostly LGTM, with a couple small points.

http://codereview.chromium.org/6482017/diff/6002/bin/cros_au_test_harness.py
File bin/cros_au_test_harness.py (right):

http://codereview.chromium.org/6482017/diff/6002/bin/cros_au_test_harness.py#...
bin/cros_au_test_harness.py:965: RunCommand(['bin/cros_make_image_bootable',
from_dir, image, ],
Should this be in the finally clause, or unindented a level?

http://codereview.chromium.org/6482017/diff/6002/bin/cros_au_test_harness.py#...
bin/cros_au_test_harness.py:1037: if options.private_key or options.public_key:
If either is specified, you verify that both exist with asserts. Seems like it
would be better to give a friendlier message explaining that if either is
specified, both should be specified. Not a big deal, since this is mostly only
used from test scripts.

http://codereview.chromium.org/6482017/diff/6002/bin/cros_au_test_harness.py#...
bin/cros_au_test_harness.py:1044: # Clean up previous work if were requested to.
s/were/we're/

[sosa, 2011-02-11 21:20:01]

PTAL ... resolved comments and also actually added unittests to ctest when
calling Cros_au_test_harness

http://codereview.chromium.org/6482017/diff/6002/bin/cros_au_test_harness.py
File bin/cros_au_test_harness.py (right):

http://codereview.chromium.org/6482017/diff/6002/bin/cros_au_test_harness.py#...
bin/cros_au_test_harness.py:965: RunCommand(['bin/cros_make_image_bootable',
from_dir, image, ],
On 2011/02/11 01:25:55, dgarrett wrote:
> Should this be in the finally clause, or unindented a level?

Done.

http://codereview.chromium.org/6482017/diff/6002/bin/cros_au_test_harness.py#...
bin/cros_au_test_harness.py:1037: if options.private_key or options.public_key:
On 2011/02/11 01:25:55, dgarrett wrote:
> If either is specified, you verify that both exist with asserts. Seems like it
> would be better to give a friendlier message explaining that if either is
> specified, both should be specified. Not a big deal, since this is mostly only
> used from test scripts.

Done.

http://codereview.chromium.org/6482017/diff/6002/bin/cros_au_test_harness.py#...
bin/cros_au_test_harness.py:1044: # Clean up previous work if were requested to.
On 2011/02/11 01:25:55, dgarrett wrote:
> s/were/we're/

Done.

[dgarrett, 2011-02-11 21:36:44]

Looks Good To Me.