[Telemetry] Resolve errors due to new pylint checker.

[Telemetry] Resolve errors due to new pylint checker.

BUG=

Committed: https://crrev.com/50cb444351e5d43e428abdd5fcd597a6a9c232e4
Cr-Commit-Position: refs/heads/master@{#304834}

[nednguyen, 2014-11-19 00:44:34]

nednguyen@google.com changed reviewers:
+ chrishenry@google.com, dtu@chromium.org, tonyg@chromium.org,
vapier@chromium.org

[nednguyen, 2014-11-19 00:44:35]

I can't solve the cyclic import error with pylint. Is there easy way to get
around it?


** Presubmit Warnings **
Pylint (436 files) (40.00s) failed
************* Module unittest_data.test_simple_two_page_set
R:  1, 0: Cyclic import (telemetry.core.platform ->
telemetry.core.platform.linux_platform_backend ->
telemetry.util.support_binaries -> telemetry.util.cloud_storage) (cyclic-import)
R:  1, 0: Cyclic import (telemetry.core.platform ->
telemetry.core.platform.linux_platform_backend ->
telemetry.core.platform.posix_platform_backend ->
telemetry.core.platform.desktop_platform_backend ->
telemetry.util.support_binaries -> telemetry.util.cloud_storage) (cyclic-import)
R:  1, 0: Cyclic import (telemetry.core.platform ->
telemetry.core.platform.linux_platform_backend -> telemetry.util.cloud_storage)
(cyclic-import)
R:  1, 0: Cyclic import (telemetry.core.platform ->
telemetry.core.platform.mac_platform_backend ->
telemetry.core.platform.posix_platform_backend ->
telemetry.core.platform.desktop_platform_backend ->
telemetry.util.support_binaries -> telemetry.util.cloud_storage) (cyclic-import)
R:  1, 0: Cyclic import (telemetry.core.platform ->
telemetry.core.platform.win_platform_backend ->
telemetry.core.platform.desktop_platform_backend ->
telemetry.util.support_binaries -> telemetry.util.cloud_storage) (cyclic-import)
R:  1, 0: Cyclic import (telemetry.core.backends.chrome.chrome_browser_options
-> telemetry.core.browser_options -> telemetry.core.browser_finder ->
telemetry.core.backends.chrome.cros_browser_finder ->
telemetry.core.backends.chrome.cros_browser_with_oobe ->
telemetry.core.backends.chrome.cros_browser_backend) (cyclic-import)
R:  1, 0: Cyclic import (telemetry.core.backends.chrome.chrome_browser_options
-> telemetry.core.browser_options) (cyclic-import)
R:  1, 0: Cyclic import (telemetry.core.backends.chrome.cros_browser_finder ->
telemetry.core.browser_finder) (cyclic-import)
R:  1, 0: Cyclic import (telemetry.core.platform ->
telemetry.core.platform.win_platform_backend -> telemetry.util.cloud_storage)
(cyclic-import)
R:  1, 0: Cyclic import (telemetry.core.backends.chrome.chrome_browser_options
-> telemetry.core.browser_options -> telemetry.core.browser_finder ->
telemetry.core.backends.chrome.cros_browser_finder ->
telemetry.core.backends.chrome.cros_browser_backend) (cyclic-import)

[tonyg, 2014-11-19 01:35:37]

lgtm w/ nits

Wow! Thanks for doing this. I am a little disappointed this hassle for the new
lint checks didn't find any really interesting bugs (except maybe the
ios_browser_backend).

https://codereview.chromium.org/739623005/diff/1/tools/telemetry/telemetry/co...
File tools/telemetry/telemetry/core/platform/cros_interface.py (right):

https://codereview.chromium.org/739623005/diff/1/tools/telemetry/telemetry/co...
tools/telemetry/telemetry/core/platform/cros_interface.py:152: r'hosts.\s\n',
'', toClean)
It seems odd to implicitly concatenate a raw string and non-raw string. We
should probably make them both raw and fix this escaping.

https://codereview.chromium.org/739623005/diff/1/tools/telemetry/telemetry/co...
File tools/telemetry/telemetry/core/platform/mac_platform_backend.py (right):

https://codereview.chromium.org/739623005/diff/1/tools/telemetry/telemetry/co...
tools/telemetry/telemetry/core/platform/mac_platform_backend.py:82:
super(ProcTaskInfo, self).__init__()  # pylint: disable=bad-super-call
I don't understand this one. Should we fix the call instead or is lint confused?

https://codereview.chromium.org/739623005/diff/1/tools/telemetry/telemetry/co...
File tools/telemetry/telemetry/core/platform/platform_backend.py (right):

https://codereview.chromium.org/739623005/diff/1/tools/telemetry/telemetry/co...
tools/telemetry/telemetry/core/platform/platform_backend.py:34: XP =          
OSVersion('xp',            5.1)  # pylint: disable=bad-whitespace
It might just be cleaner to "fix" the whitespace.

https://codereview.chromium.org/739623005/diff/1/tools/telemetry/telemetry/ut...
File tools/telemetry/telemetry/util/exception_formatter.py (right):

https://codereview.chromium.org/739623005/diff/1/tools/telemetry/telemetry/ut...
tools/telemetry/telemetry/util/exception_formatter.py:18: if not
bool(exception_class) == bool(exception) == bool(tb):
This was definitely more readable before. Is there a clearer way to write this
now?

[nednguyen, 2014-11-19 02:15:16]

https://codereview.chromium.org/739623005/diff/1/tools/telemetry/telemetry/co...
File tools/telemetry/telemetry/core/platform/cros_interface.py (right):

https://codereview.chromium.org/739623005/diff/1/tools/telemetry/telemetry/co...
tools/telemetry/telemetry/core/platform/cros_interface.py:152: r'hosts.\s\n',
'', toClean)
On 2014/11/19 01:35:36, tonyg wrote:
> It seems odd to implicitly concatenate a raw string and non-raw string. We
> should probably make them both raw and fix this escaping.

Done.

https://codereview.chromium.org/739623005/diff/1/tools/telemetry/telemetry/co...
File tools/telemetry/telemetry/core/platform/mac_platform_backend.py (right):

https://codereview.chromium.org/739623005/diff/1/tools/telemetry/telemetry/co...
tools/telemetry/telemetry/core/platform/mac_platform_backend.py:82:
super(ProcTaskInfo, self).__init__()  # pylint: disable=bad-super-call
On 2014/11/19 01:35:36, tonyg wrote:
> I don't understand this one. Should we fix the call instead or is lint
confused?

One pattern I've seen happen again and again is that this linter is confused
anytime we define a class inside function, which is a valid python thing.

https://codereview.chromium.org/739623005/diff/1/tools/telemetry/telemetry/co...
File tools/telemetry/telemetry/core/platform/platform_backend.py (right):

https://codereview.chromium.org/739623005/diff/1/tools/telemetry/telemetry/co...
tools/telemetry/telemetry/core/platform/platform_backend.py:34: XP =          
OSVersion('xp',            5.1)  # pylint: disable=bad-whitespace
On 2014/11/19 01:35:36, tonyg wrote:
> It might just be cleaner to "fix" the whitespace.

Done.

https://codereview.chromium.org/739623005/diff/1/tools/telemetry/telemetry/ut...
File tools/telemetry/telemetry/util/exception_formatter.py (right):

https://codereview.chromium.org/739623005/diff/1/tools/telemetry/telemetry/ut...
tools/telemetry/telemetry/util/exception_formatter.py:18: if not
bool(exception_class) == bool(exception) == bool(tb):
On 2014/11/19 01:35:36, tonyg wrote:
> This was definitely more readable before. Is there a clearer way to write this
> now?

I change it to assert. Arguably, the condition should never happen and program
should crash.

[nednguyen, 2014-11-19 04:41:28]

The CQ bit was checked by nednguyen@google.com

[commit-bot: I haz the power, 2014-11-19 04:41:50]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/739623005/40001

[commit-bot: I haz the power, 2014-11-19 05:03:40]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-11-19 05:03:42]

Try jobs failed on following builders:
  linux_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu_trigger...)

[nednguyen, 2014-11-19 05:07:14]

The CQ bit was checked by nednguyen@google.com

[commit-bot: I haz the power, 2014-11-19 05:08:16]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/739623005/60001

[commit-bot: I haz the power, 2014-11-19 06:15:34]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-11-19 06:15:36]

Try jobs failed on following builders:
  linux_chromium_rel_ng on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[nednguyen, 2014-11-19 15:51:36]

The CQ bit was checked by nednguyen@google.com

[commit-bot: I haz the power, 2014-11-19 15:52:50]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/739623005/80001

[commit-bot: I haz the power, 2014-11-19 17:56:13]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2014-11-19 17:56:56]

Patchset 5 (id:??) landed as
https://crrev.com/50cb444351e5d43e428abdd5fcd597a6a9c232e4
Cr-Commit-Position: refs/heads/master@{#304834}