Add a presubmit script and pylintrc for content/test/gpu

Add a presubmit script and pylintrc for content/test/gpu

BUG=544291
Committed: https://crrev.com/5b8b85e33293743b41412b8797816d46744fa884
Cr-Commit-Position: refs/heads/master@{#355474}

[cwallez, 2015-10-21 16:14:49]

Description was changed from

==========
Add a presubmit script and pylintrc for content/test/gpu

BUG=544291
==========

to

==========
Add a presubmit script and pylintrc for content/test/gpu

BUG=544291
==========

[cwallez, 2015-10-21 16:14:49]

cwallez@google.com changed reviewers:
+ kbr@chromium.org

[cwallez, 2015-10-21 16:15:01]

On 2015/10/21 16:14:49, cwallez wrote:
> mailto:cwallez@google.com changed reviewers:
> + mailto:kbr@chromium.org

PTAL

[Corentin Wallez, 2015-10-21 17:06:18]

cwallez@chromium.org changed reviewers:
+ zmo@chromium.org

[Corentin Wallez, 2015-10-21 17:07:28]

On 2015/10/21 16:15:01, cwallez wrote:
> On 2015/10/21 16:14:49, cwallez wrote:
> > mailto:cwallez@google.com changed reviewers:
> > + mailto:kbr@chromium.org
> 
> PTAL

Also can one of you launch tryjobs? It is needed to test that pylint fixes do
not break code but I can't launch tryjobs myself.

[Ken Russell (switch to Gerrit), 2015-10-21 22:01:42]

The CQ bit was checked by kbr@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-10-21 22:04:33]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1413883003/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1413883003/1

[Ken Russell (switch to Gerrit), 2015-10-21 22:12:08]

Thank you for putting this in place. Very nice cleanups. LGTM overall. A few
comments.

https://codereview.chromium.org/1413883003/diff/1/content/test/gpu/PRESUBMIT.py
File content/test/gpu/PRESUBMIT.py (right):

https://codereview.chromium.org/1413883003/diff/1/content/test/gpu/PRESUBMIT....
content/test/gpu/PRESUBMIT.py:1: # Copyright 2012 The Chromium Authors. All
rights reserved.
2012 -> 2015

https://codereview.chromium.org/1413883003/diff/1/content/test/gpu/PRESUBMIT....
content/test/gpu/PRESUBMIT.py:24:
input_api.os_path.join(input_api.PresubmitLocalPath(), 'run_unittests.py'),
I think we should run all of the GPU unittests in this directory upon presubmit.
They're fast and will catch more errors that might otherwise slip in.

https://codereview.chromium.org/1413883003/diff/1/content/test/gpu/PRESUBMIT....
content/test/gpu/PRESUBMIT.py:36: cmd_name =
'run_webgl_conformance_expectations_unittests',
run_gpu_unittests instead?

https://codereview.chromium.org/1413883003/diff/1/content/test/gpu/gpu_tests/...
File content/test/gpu/gpu_tests/cloud_storage_test_base.py (right):

https://codereview.chromium.org/1413883003/diff/1/content/test/gpu/gpu_tests/...
content/test/gpu/gpu_tests/cloud_storage_test_base.py:59:
gpu_test_base.ValidatorBase.__init__(self)
It's undesirable to name the superclass explicitly. Can you find another way to
avoid the presubmit warning? Maybe name this one CloudStorageValidatorBase?

https://codereview.chromium.org/1413883003/diff/1/content/test/gpu/gpu_tests/...
File content/test/gpu/gpu_tests/gpu_rasterization.py (right):

https://codereview.chromium.org/1413883003/diff/1/content/test/gpu/gpu_tests/...
content/test/gpu/gpu_tests/gpu_rasterization.py:74: def __init__(self,
max_failures=None):
Unfortunate that useless code has to be added to avoid a pylint warning.

https://codereview.chromium.org/1413883003/diff/1/content/test/gpu/gpu_tests/...
File content/test/gpu/gpu_tests/gpu_test_base.py (right):

https://codereview.chromium.org/1413883003/diff/1/content/test/gpu/gpu_tests/...
content/test/gpu/gpu_tests/gpu_test_base.py:53: def ValidateAndMeasurePage(self,
page, tab, result):
Again, unfortunate that useless code is required to satisfy pylint.

https://codereview.chromium.org/1413883003/diff/1/content/test/gpu/gpu_tests/...
content/test/gpu/gpu_tests/gpu_test_base.py:61: def RunStoryWithRetries(cls,
shared_state, results):
Consider importing shared_page_state as shared_page_state_module so that these
variable renamings can be avoided.

https://codereview.chromium.org/1413883003/diff/1/content/test/gpu/gpu_tests/...
File content/test/gpu/gpu_tests/gpu_test_base_unittest.py (right):

https://codereview.chromium.org/1413883003/diff/1/content/test/gpu/gpu_tests/...
content/test/gpu/gpu_tests/gpu_test_base_unittest.py:6: from telemetry import
benchmark as telemetry_benchmark
This seems fine, or also consider "benchmark_module".

https://codereview.chromium.org/1413883003/diff/1/content/test/gpu/gpu_tests/...
File content/test/gpu/gpu_tests/test_expectations_unittest.py (left):

https://codereview.chromium.org/1413883003/diff/1/content/test/gpu/gpu_tests/...
content/test/gpu/gpu_tests/test_expectations_unittest.py:33: return
self.system_info
Good catch.

https://codereview.chromium.org/1413883003/diff/1/content/test/gpu/gpu_tests/...
File content/test/gpu/gpu_tests/webgl_conformance_expectations.py (right):

https://codereview.chromium.org/1413883003/diff/1/content/test/gpu/gpu_tests/...
content/test/gpu/gpu_tests/webgl_conformance_expectations.py:13:
GpuTestExpectations.__init__(self)
I'd rather use the super() syntax to avoid explicitly naming the superclass if
possible.

[commit-bot: I haz the power, 2015-10-21 22:43:05]

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

[commit-bot: I haz the power, 2015-10-21 22:43:05]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[Ken Russell (switch to Gerrit), 2015-10-21 22:53:09]

On 2015/10/21 22:43:05, commit-bot: I haz the power wrote:
> Dry run: Try jobs failed on following builders:
>   win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

That's a strange failure. Let's try it again.

[Corentin Wallez, 2015-10-21 23:07:25]

https://codereview.chromium.org/1413883003/diff/1/content/test/gpu/PRESUBMIT.py
File content/test/gpu/PRESUBMIT.py (right):

https://codereview.chromium.org/1413883003/diff/1/content/test/gpu/PRESUBMIT....
content/test/gpu/PRESUBMIT.py:1: # Copyright 2012 The Chromium Authors. All
rights reserved.
On 2015/10/21 22:12:08, Ken Russell wrote:
> 2012 -> 2015

Done.

https://codereview.chromium.org/1413883003/diff/1/content/test/gpu/PRESUBMIT....
content/test/gpu/PRESUBMIT.py:24:
input_api.os_path.join(input_api.PresubmitLocalPath(), 'run_unittests.py'),
On 2015/10/21 22:12:08, Ken Russell wrote:
> I think we should run all of the GPU unittests in this directory upon
presubmit.
> They're fast and will catch more errors that might otherwise slip in.

Done

https://codereview.chromium.org/1413883003/diff/1/content/test/gpu/PRESUBMIT....
content/test/gpu/PRESUBMIT.py:36: cmd_name =
'run_webgl_conformance_expectations_unittests',
On 2015/10/21 22:12:08, Ken Russell wrote:
> run_gpu_unittests instead?

Done.

https://codereview.chromium.org/1413883003/diff/1/content/test/gpu/gpu_tests/...
File content/test/gpu/gpu_tests/cloud_storage_test_base.py (right):

https://codereview.chromium.org/1413883003/diff/1/content/test/gpu/gpu_tests/...
content/test/gpu/gpu_tests/cloud_storage_test_base.py:59:
gpu_test_base.ValidatorBase.__init__(self)
On 2015/10/21 22:12:08, Ken Russell wrote:
> It's undesirable to name the superclass explicitly. Can you find another way
to
> avoid the presubmit warning? Maybe name this one CloudStorageValidatorBase?
> 

I was having a pylint error related to using super() with an old-style class.
Deleting all the .pyc as the telemetry guys suggested seems to have removed that
warning so I removed this change

Done

https://codereview.chromium.org/1413883003/diff/1/content/test/gpu/gpu_tests/...
File content/test/gpu/gpu_tests/gpu_rasterization.py (right):

https://codereview.chromium.org/1413883003/diff/1/content/test/gpu/gpu_tests/...
content/test/gpu/gpu_tests/gpu_rasterization.py:74: def __init__(self,
max_failures=None):
On 2015/10/21 22:12:08, Ken Russell wrote:
> Unfortunate that useless code has to be added to avoid a pylint warning.

Yes, this makes me sad too.

https://codereview.chromium.org/1413883003/diff/1/content/test/gpu/gpu_tests/...
File content/test/gpu/gpu_tests/gpu_test_base.py (right):

https://codereview.chromium.org/1413883003/diff/1/content/test/gpu/gpu_tests/...
content/test/gpu/gpu_tests/gpu_test_base.py:53: def ValidateAndMeasurePage(self,
page, tab, result):
On 2015/10/21 22:12:08, Ken Russell wrote:
> Again, unfortunate that useless code is required to satisfy pylint.

Acknowledged.

https://codereview.chromium.org/1413883003/diff/1/content/test/gpu/gpu_tests/...
content/test/gpu/gpu_tests/gpu_test_base.py:61: def RunStoryWithRetries(cls,
shared_state, results):
On 2015/10/21 22:12:08, Ken Russell wrote:
> Consider importing shared_page_state as shared_page_state_module so that these
> variable renamings can be avoided.

Done.

https://codereview.chromium.org/1413883003/diff/1/content/test/gpu/gpu_tests/...
File content/test/gpu/gpu_tests/gpu_test_base_unittest.py (right):

https://codereview.chromium.org/1413883003/diff/1/content/test/gpu/gpu_tests/...
content/test/gpu/gpu_tests/gpu_test_base_unittest.py:6: from telemetry import
benchmark as telemetry_benchmark
On 2015/10/21 22:12:08, Ken Russell wrote:
> This seems fine, or also consider "benchmark_module".

Done.

https://codereview.chromium.org/1413883003/diff/1/content/test/gpu/gpu_tests/...
File content/test/gpu/gpu_tests/test_expectations_unittest.py (left):

https://codereview.chromium.org/1413883003/diff/1/content/test/gpu/gpu_tests/...
content/test/gpu/gpu_tests/test_expectations_unittest.py:33: return
self.system_info
On 2015/10/21 22:12:08, Ken Russell wrote:
> Good catch.

Acknowledged.

https://codereview.chromium.org/1413883003/diff/1/content/test/gpu/gpu_tests/...
File content/test/gpu/gpu_tests/webgl_conformance_expectations.py (right):

https://codereview.chromium.org/1413883003/diff/1/content/test/gpu/gpu_tests/...
content/test/gpu/gpu_tests/webgl_conformance_expectations.py:13:
GpuTestExpectations.__init__(self)
On 2015/10/21 22:12:08, Ken Russell wrote:
> I'd rather use the super() syntax to avoid explicitly naming the superclass if
> possible.

Done.

[Ken Russell (switch to Gerrit), 2015-10-21 23:22:12]

Still LGTM. Please CQ when you're ready.

[Corentin Wallez, 2015-10-22 00:06:59]

The CQ bit was checked by cwallez@chromium.org

[Corentin Wallez, 2015-10-22 00:07:53]

On 2015/10/21 23:22:12, Ken Russell wrote:
> Still LGTM. Please CQ when you're ready.

The code in the backtrace in the win_chromium_rel_ng seems unrelated to this
patch. CQ'ing.

[commit-bot: I haz the power, 2015-10-22 00:08:18]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1413883003/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1413883003/20001

[commit-bot: I haz the power, 2015-10-22 01:39:45]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-10-22 01:40:41]

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