Remove test keys from images we test with in the test harness.

Remove test keys from images we test with in the test harness.

Cleaned up _InsertKeyIntoImage to be more general (able to remove key if no key set).  Also made it much more intelligent ... only doing things when it needs to.

Moved mount / unmount code to cros_lib.

BUG=chromium-os:12684
TEST=Ran it a lot ... inspected images after and saw tests pass.

Committed: http://chrome-svn/viewvc/chromeos?view=rev&revision=31b641e

[sosa, 2011-03-16 21:38:39]

Based on off-line petkov feedback, refactored public key logic into its own
module.

PTAL

[petkov, 2011-03-16 22:03:47]

mostly nits

http://codereview.chromium.org/6698017/diff/6001/bin/au_test_harness/cros_au_...
File bin/au_test_harness/cros_au_test_harness.py (right):

http://codereview.chromium.org/6698017/diff/6001/bin/au_test_harness/cros_au_...
bin/au_test_harness/cros_au_test_harness.py:86: if not options.delta:
au_test.AUTest.modified_images.add(target)
can you make modified_images local to this routine now and remove it from
AUTest?

http://codereview.chromium.org/6698017/diff/6001/bin/au_test_harness/cros_au_...
bin/au_test_harness/cros_au_test_harness.py:265: # Un-modify any target images
we modified.  We don't need to de-modify
Un-modify or de-modify? Pick on non-word and stick with it :-)

http://codereview.chromium.org/6698017/diff/6001/bin/au_test_harness/public_k...
File bin/au_test_harness/public_key_manager.py (right):

http://codereview.chromium.org/6698017/diff/6001/bin/au_test_harness/public_k...
bin/au_test_harness/public_key_manager.py:14: target_key_path =
'usr/share/update_engine/update-payload-key.pub.pem'
TARGET_KEY_PATH?

http://codereview.chromium.org/6698017/diff/6001/bin/au_test_harness/public_k...
bin/au_test_harness/public_key_manager.py:51: self.image_path)
missing return

http://codereview.chromium.org/6698017/diff/6001/bin/au_test_harness/public_k...
bin/au_test_harness/public_key_manager.py:54: cros_lib.Info('Copying %s into %s'
% (self.key_path, self.image_path))
move before try

http://codereview.chromium.org/6698017/diff/6001/bin/au_test_harness/public_k...
bin/au_test_harness/public_key_manager.py:59: if not os.path.exists(dir_path):
why check rather than always mkdir?

http://codereview.chromium.org/6698017/diff/6001/bin/au_test_harness/public_k...
bin/au_test_harness/public_key_manager.py:82: """Makes the image bootable.  Only
useful on non-vm images"""
second sentence is not a sentence and doesn't have a period.

http://codereview.chromium.org/6698017/diff/6001/bin/au_test_harness/public_k...
bin/au_test_harness/public_key_manager.py:83: crosutils_dir =
os.path.abspath(__file__).rsplit('/', 3)[0]
this should be in a library somewhere, no? can't you use _CROSUTILS_DIRECTORY?

http://codereview.chromium.org/6698017/diff/6001/bin/au_test_harness/public_k...
bin/au_test_harness/public_key_manager.py:86: if not 'qemu' in
os.path.basename(self.image_path):
make this check at the beginning of the routine and simply return.

[sosa, 2011-03-16 22:47:02]

Addressed comments.  Re-tested without test_prefix :).  PTAL

http://codereview.chromium.org/6698017/diff/6001/bin/au_test_harness/cros_au_...
File bin/au_test_harness/cros_au_test_harness.py (right):

http://codereview.chromium.org/6698017/diff/6001/bin/au_test_harness/cros_au_...
bin/au_test_harness/cros_au_test_harness.py:86: if not options.delta:
au_test.AUTest.modified_images.add(target)
On 2011/03/16 22:03:48, petkov wrote:
> can you make modified_images local to this routine now and remove it from
> AUTest?

Done.

http://codereview.chromium.org/6698017/diff/6001/bin/au_test_harness/cros_au_...
bin/au_test_harness/cros_au_test_harness.py:265: # Un-modify any target images
we modified.  We don't need to de-modify
I enjoy making up words. Done.

On 2011/03/16 22:03:48, petkov wrote:
> Un-modify or de-modify? Pick on non-word and stick with it :-)

http://codereview.chromium.org/6698017/diff/6001/bin/au_test_harness/public_k...
File bin/au_test_harness/public_key_manager.py (right):

http://codereview.chromium.org/6698017/diff/6001/bin/au_test_harness/public_k...
bin/au_test_harness/public_key_manager.py:14: target_key_path =
'usr/share/update_engine/update-payload-key.pub.pem'
On 2011/03/16 22:03:48, petkov wrote:
> TARGET_KEY_PATH?

Done.

http://codereview.chromium.org/6698017/diff/6001/bin/au_test_harness/public_k...
bin/au_test_harness/public_key_manager.py:51: self.image_path)
On 2011/03/16 22:03:48, petkov wrote:
> missing return

Done.

http://codereview.chromium.org/6698017/diff/6001/bin/au_test_harness/public_k...
bin/au_test_harness/public_key_manager.py:54: cros_lib.Info('Copying %s into %s'
% (self.key_path, self.image_path))
On 2011/03/16 22:03:48, petkov wrote:
> move before try

Done.

http://codereview.chromium.org/6698017/diff/6001/bin/au_test_harness/public_k...
bin/au_test_harness/public_key_manager.py:59: if not os.path.exists(dir_path):
On 2011/03/16 22:03:48, petkov wrote:
> why check rather than always mkdir?

Done.

http://codereview.chromium.org/6698017/diff/6001/bin/au_test_harness/public_k...
bin/au_test_harness/public_key_manager.py:83: crosutils_dir =
os.path.abspath(__file__).rsplit('/', 3)[0]
On 2011/03/16 22:03:48, petkov wrote:
> this should be in a library somewhere, no? can't you use _CROSUTILS_DIRECTORY?

Done.

http://codereview.chromium.org/6698017/diff/6001/bin/au_test_harness/public_k...
bin/au_test_harness/public_key_manager.py:86: if not 'qemu' in
os.path.basename(self.image_path):
On 2011/03/16 22:03:48, petkov wrote:
> make this check at the beginning of the routine and simply return.

Done.

[petkov, 2011-03-16 22:56:11]

LGTM

http://codereview.chromium.org/6698017/diff/9003/bin/au_test_harness/cros_au_...
File bin/au_test_harness/cros_au_test_harness.py (right):

http://codereview.chromium.org/6698017/diff/9003/bin/au_test_harness/cros_au_...
bin/au_test_harness/cros_au_test_harness.py:84: modified_images = set ([])
you could just say set(), I think