Cleanups to test updater code. Handle service restart better, copy update-engine logs after tests.

Cleanups to test updater code. Handle service restart better, copy update-engine logs after tests.

Change-Id: Ie5bd40996753bfd4c27ab2ba34d5fd525b1566f6

[petkov, 2010-08-30 18:27:58]

LGTM w/ some nits.

http://codereview.chromium.org/3257003/diff/1/4
File client/common_lib/chromiumos_updater.py (right):

http://codereview.chromium.org/3257003/diff/1/4#newcode81
client/common_lib/chromiumos_updater.py:81: try:
Is there any downside to always reset the update engine, regardless of its
current status? This would reduce this code to a single line.

http://codereview.chromium.org/3257003/diff/1/4#newcode159
client/common_lib/chromiumos_updater.py:159: builder_num = '-b%s' %
build_match.group(1)
I'd say either log builder_num, or remove all the code related to determining
it...

http://codereview.chromium.org/3257003/diff/1/4#newcode162
client/common_lib/chromiumos_updater.py:162: # for now, discard the builder
number and just return version-release.
s/for/For/

[seano, 2010-08-30 18:55:47]

PTAL.

Dropping the builder number, until the build folks figure out if they want to
start generating it for dogfood builds, or if we simply don't care.

http://codereview.chromium.org/3257003/diff/1/4
File client/common_lib/chromiumos_updater.py (right):

http://codereview.chromium.org/3257003/diff/1/4#newcode81
client/common_lib/chromiumos_updater.py:81: try:
On 2010/08/30 18:27:59, petkov wrote:
> Is there any downside to always reset the update engine, regardless of its
> current status? This would reduce this code to a single line.
> 

SGTM. Done.

http://codereview.chromium.org/3257003/diff/1/4#newcode159
client/common_lib/chromiumos_updater.py:159: builder_num = '-b%s' %
build_match.group(1)
On 2010/08/30 18:27:59, petkov wrote:
> I'd say either log builder_num, or remove all the code related to determining
> it...
> 

This is a workaround due to some (possibly temporary) naming scheme of dogfood
builds.

I've removed it completely now, we can bring it back if it really matters (I
think it does not.)

http://codereview.chromium.org/3257003/diff/1/4#newcode162
client/common_lib/chromiumos_updater.py:162: # for now, discard the builder
number and just return version-release.
On 2010/08/30 18:27:59, petkov wrote:
> s/for/For/
> 

Done.

[petkov, 2010-08-30 19:52:37]

still lgtm

http://codereview.chromium.org/3257003/diff/6001/7003
File client/common_lib/chromiumos_updater.py (right):

http://codereview.chromium.org/3257003/diff/6001/7003#newcode151
client/common_lib/chromiumos_updater.py:151: # Continuous builds have an extra
"builder number" on the end.
This code should go away too?

[seano, 2010-08-31 13:54:03]

Added support for reverting the update in case of failed statefuldev tools
install.

Addressed petkov's comment.

Pushing.

On 2010/08/30 19:52:37, petkov wrote:
> still lgtm
> 
> http://codereview.chromium.org/3257003/diff/6001/7003
> File client/common_lib/chromiumos_updater.py (right):
> 
> http://codereview.chromium.org/3257003/diff/6001/7003#newcode151
> client/common_lib/chromiumos_updater.py:151: # Continuous builds have an extra
> "builder number" on the end.
> This code should go away too?