Restore logging level after test is finished.

Restore logging level after test is finished.

In run_test.Main, there are two places that logging level may be changed:
1. BrowserFinderOptions, when calling parser.parse_args, in which logging level
is set based on verbosity's value.
2. run_test.Main, in which logging level is set to WARN if verbosity is 0.

Such change on logging level causes problem to caller like autotest, which
relies on logging level to log details during the test, and output messages
from different logging level to different log file, e.g., client.DEBUG,
client.INFO etc.

This CL added some fix to:
1. Cache the logging level at the beginning of the Main method.
2. Always restore logging level at the end of the Main method.

BUG=chromium:284763
TEST=verified in local dut with autotest:
test_that --fast -b lumpy  172.22.75.225 telemetry_UnitTests

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=223504

[achuithb, 2013-09-07 03:17:10]

lgtm from me, but I'm not too familiar with this.

Dave, can you please take a look?

https://codereview.chromium.org/23458027/diff/1/tools/telemetry/telemetry/uni...
File tools/telemetry/telemetry/unittest/run_tests.py (right):

https://codereview.chromium.org/23458027/diff/1/tools/telemetry/telemetry/uni...
tools/telemetry/telemetry/unittest/run_tests.py:134: # Always restore logging
level, which may be changed in parser.parse_args.
nit: don't need 'Always'

[dshi, 2013-09-12 21:40:44]

https://codereview.chromium.org/23458027/diff/1/tools/telemetry/telemetry/uni...
File tools/telemetry/telemetry/unittest/run_tests.py (right):

https://codereview.chromium.org/23458027/diff/1/tools/telemetry/telemetry/uni...
tools/telemetry/telemetry/unittest/run_tests.py:134: # Always restore logging
level, which may be changed in parser.parse_args.
On 2013/09/07 03:17:10, achuith.bhandarkar wrote:
> nit: don't need 'Always'

Done.

[dtu, 2013-09-12 21:45:51]

https://codereview.chromium.org/23458027/diff/4001/tools/telemetry/telemetry/...
File tools/telemetry/telemetry/unittest/run_tests.py (right):

https://codereview.chromium.org/23458027/diff/4001/tools/telemetry/telemetry/...
tools/telemetry/telemetry/unittest/run_tests.py:135:
logging.getLogger().setLevel(logging_level)
Theoretically, you want this try/finally to go around everything, right? If an
error happens in this file before this try block, your logging level will still
be messed up. Maybe add a wrapper function to do this without changing the
indent level.

[dshi, 2013-09-13 20:28:33]

https://codereview.chromium.org/23458027/diff/4001/tools/telemetry/telemetry/...
File tools/telemetry/telemetry/unittest/run_tests.py (right):

https://codereview.chromium.org/23458027/diff/4001/tools/telemetry/telemetry/...
tools/telemetry/telemetry/unittest/run_tests.py:135:
logging.getLogger().setLevel(logging_level)
On 2013/09/12 21:45:51, dtu wrote:
> Theoretically, you want this try/finally to go around everything, right? If an
> error happens in this file before this try block, your logging level will
still
> be messed up. Maybe add a wrapper function to do this without changing the
> indent level.

Done.

[dtu, 2013-09-13 21:46:29]

lgtm with nit

https://chromiumcodereview.appspot.com/23458027/diff/9001/tools/telemetry/tel...
File tools/telemetry/telemetry/unittest/run_tests.py (right):

https://chromiumcodereview.appspot.com/23458027/diff/9001/tools/telemetry/tel...
tools/telemetry/telemetry/unittest/run_tests.py:83: def run(args, start_dir,
top_level_dir, runner):
_LoggingWrapper or something? This doesn't fit our naming convention.

[dshi, 2013-09-13 21:58:53]

https://chromiumcodereview.appspot.com/23458027/diff/9001/tools/telemetry/tel...
File tools/telemetry/telemetry/unittest/run_tests.py (right):

https://chromiumcodereview.appspot.com/23458027/diff/9001/tools/telemetry/tel...
tools/telemetry/telemetry/unittest/run_tests.py:83: def run(args, start_dir,
top_level_dir, runner):
On 2013/09/13 21:46:29, dtu wrote:
> _LoggingWrapper or something? This doesn't fit our naming convention.

Done.

[dshi, 2013-09-16 15:48:04]

I got +2 on my earlier change set, but I think decorator is a much cleaner
approach to restore logging level and it's more re-usable. Could you please take
another look at the latest code? Thanks for your help!

[dtu, 2013-09-16 18:04:23]

https://chromiumcodereview.appspot.com/23458027/diff/9002/tools/telemetry/tel...
File tools/telemetry/telemetry/unittest/run_tests.py (right):

https://chromiumcodereview.appspot.com/23458027/diff/9002/tools/telemetry/tel...
tools/telemetry/telemetry/unittest/run_tests.py:84: def RestoreLoggingLevel():
Too many wrappers. If you use this like @RestoreLoggingLevel, you don't need
this outside function.

https://chromiumcodereview.appspot.com/23458027/diff/9002/tools/telemetry/tel...
tools/telemetry/telemetry/unittest/run_tests.py:86: def _FuncRestore(*args,
**kwargs):
_LoggingRestoreWrapper

https://chromiumcodereview.appspot.com/23458027/diff/9002/tools/telemetry/tel...
tools/telemetry/telemetry/unittest/run_tests.py:94: # This is needed to preserve
stack trace.
Why is this needed?

[dtu, 2013-09-16 18:05:22]

On 2013/09/16 18:04:23, dtu wrote:
>
https://chromiumcodereview.appspot.com/23458027/diff/9002/tools/telemetry/tel...
> File tools/telemetry/telemetry/unittest/run_tests.py (right):
> 
>
https://chromiumcodereview.appspot.com/23458027/diff/9002/tools/telemetry/tel...
> tools/telemetry/telemetry/unittest/run_tests.py:84: def RestoreLoggingLevel():
> Too many wrappers. If you use this like @RestoreLoggingLevel, you don't need
> this outside function.
> 
>
https://chromiumcodereview.appspot.com/23458027/diff/9002/tools/telemetry/tel...
> tools/telemetry/telemetry/unittest/run_tests.py:86: def _FuncRestore(*args,
> **kwargs):
> _LoggingRestoreWrapper
> 
>
https://chromiumcodereview.appspot.com/23458027/diff/9002/tools/telemetry/tel...
> tools/telemetry/telemetry/unittest/run_tests.py:94: # This is needed to
preserve
> stack trace.
> Why is this needed?

Good idea with a decorator.

[dshi, 2013-09-16 21:20:12]

https://codereview.chromium.org/23458027/diff/9002/tools/telemetry/telemetry/...
File tools/telemetry/telemetry/unittest/run_tests.py (right):

https://codereview.chromium.org/23458027/diff/9002/tools/telemetry/telemetry/...
tools/telemetry/telemetry/unittest/run_tests.py:84: def RestoreLoggingLevel():
On 2013/09/16 18:04:23, dtu wrote:
> Too many wrappers. If you use this like @RestoreLoggingLevel, you don't need
> this outside function.

Done.

https://codereview.chromium.org/23458027/diff/9002/tools/telemetry/telemetry/...
tools/telemetry/telemetry/unittest/run_tests.py:86: def _FuncRestore(*args,
**kwargs):
On 2013/09/16 18:04:23, dtu wrote:
> _LoggingRestoreWrapper

Done.

https://codereview.chromium.org/23458027/diff/9002/tools/telemetry/telemetry/...
tools/telemetry/telemetry/unittest/run_tests.py:94: # This is needed to preserve
stack trace.
Done. The code was from autotest retry decorator. I tested, seems it's not
needed to preserve stack trace.

On 2013/09/16 18:04:23, dtu wrote:
> Why is this needed?

[dtu, 2013-09-16 21:30:13]

lgtm

[commit-bot: I haz the power, 2013-09-16 21:48:31]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dshi@chromium.org/23458027/25001

[commit-bot: I haz the power, 2013-09-17 01:38:35]

Change committed as 223504