[Telemetry]Do not run power.idle_platform for reference build runs

[Telemetry]Do not run power.idle_platform for reference build runs

BUG=700022
Review-Url: https://codereview.chromium.org/2822723002
Cr-Commit-Position: refs/heads/master@{#464822}
Committed: https://chromium.googlesource.com/chromium/src/+/5194feb68616e1a322380a5e8b5d3f38f28404e4

[rnephew (Reviews Here), 2017-04-14 16:28:20]

rnephew@chromium.org changed reviewers:
+ nednguyen@google.com

[rnephew (Reviews Here), 2017-04-14 16:28:21]

https://codereview.chromium.org/2822723002/diff/1/tools/perf/benchmark.csv
File tools/perf/benchmark.csv (right):

https://codereview.chromium.org/2822723002/diff/1/tools/perf/benchmark.csv#ne...
tools/perf/benchmark.csv:165:
tab_switching.typical_25,vovoy@chromium.org,OS>Performance
Not 100% sure why this change showed when I generated the json files.

[nednguyen, 2017-04-14 16:32:46]

I think the change you were seeing probably caused by some sort of rebase error
when we revert CLs :-?

https://codereview.chromium.org/2822723002/diff/1/tools/perf/core/perf_data_g...
File tools/perf/core/perf_data_generator.py (right):

https://codereview.chromium.org/2822723002/diff/1/tools/perf/core/perf_data_g...
tools/perf/core/perf_data_generator.py:584: tester_config, benchmarks,
benchmark_sharding_map, use_whitelist):
can you make add a benchmark_ref_build_blacklist param here (and modify the call
site of this to pass
benchmark_ref_build_blacklist=BENCHMARK_REF_BUILD_BLACKLIST) so we can add
unittest for this?

[rnephew (Reviews Here), 2017-04-14 17:21:59]

Patchset #2 (id:20001) has been deleted

[rnephew (Reviews Here), 2017-04-14 17:22:21]

Added unittests.

https://codereview.chromium.org/2822723002/diff/1/tools/perf/core/perf_data_g...
File tools/perf/core/perf_data_generator.py (right):

https://codereview.chromium.org/2822723002/diff/1/tools/perf/core/perf_data_g...
tools/perf/core/perf_data_generator.py:584: tester_config, benchmarks,
benchmark_sharding_map, use_whitelist):
On 2017/04/14 16:32:45, nednguyen wrote:
> can you make add a benchmark_ref_build_blacklist param here (and modify the
call
> site of this to pass
> benchmark_ref_build_blacklist=BENCHMARK_REF_BUILD_BLACKLIST) so we can add
> unittest for this?

I get "W:590, 0: Dangerous default value BENCHMARK_REF_BUILD_BLACKLIST
(__builtin__.list) as argument (dangerous-default-value)" when I do that. I have
to do it in a slightly more round about way to not get that linter warning.

[nednguyen, 2017-04-14 18:09:02]

https://codereview.chromium.org/2822723002/diff/40001/tools/perf/core/perf_da...
File tools/perf/core/perf_data_generator.py (right):

https://codereview.chromium.org/2822723002/diff/40001/tools/perf/core/perf_da...
tools/perf/core/perf_data_generator.py:587: benchmark_ref_build_blacklist =
BENCHMARK_REF_BUILD_BLACKLIST
Sorry, I mean that we should update the caller of generate_telemetry_tests to
set benchmark_ref_build_blacklist=BENCHMARK_REF_BUILD_BLACKLIST

[rnephew (Reviews Here), 2017-04-14 18:19:40]

https://codereview.chromium.org/2822723002/diff/40001/tools/perf/core/perf_da...
File tools/perf/core/perf_data_generator.py (right):

https://codereview.chromium.org/2822723002/diff/40001/tools/perf/core/perf_da...
tools/perf/core/perf_data_generator.py:587: benchmark_ref_build_blacklist =
BENCHMARK_REF_BUILD_BLACKLIST
On 2017/04/14 18:09:02, nednguyen wrote:
> Sorry, I mean that we should update the caller of generate_telemetry_tests to
> set benchmark_ref_build_blacklist=BENCHMARK_REF_BUILD_BLACKLIST

Done.

[nednguyen, 2017-04-14 20:45:59]

lgtm with nits

https://codereview.chromium.org/2822723002/diff/60001/tools/perf/core/perf_da...
File tools/perf/core/perf_data_generator.py (right):

https://codereview.chromium.org/2822723002/diff/60001/tools/perf/core/perf_da...
tools/perf/core/perf_data_generator.py:585: benchmark_ref_build_blacklist=None):
remove the default param here

https://codereview.chromium.org/2822723002/diff/60001/tools/perf/core/perf_da...
File tools/perf/core/perf_data_generator_unittest.py (right):

https://codereview.chromium.org/2822723002/diff/60001/tools/perf/core/perf_da...
tools/perf/core/perf_data_generator_unittest.py:10: class
_MockBenchmark(object):
why not just inherits this from the real benchmark.Benchmark?

https://codereview.chromium.org/2822723002/diff/60001/tools/perf/core/perf_da...
tools/perf/core/perf_data_generator_unittest.py:137: expected_generated_tests =
[
I think we only need a a few test methods that test the generated data. For this
one, you just need to test that there are 3 tests get generated:

generated_test_names = set(t.name for t in tests)
self.assertEquals(generated_test_names, {'blacklisted', 'not_blacklisted', 
'not_blacklisted'})

[rnephew (Reviews Here), 2017-04-14 21:40:17]

https://codereview.chromium.org/2822723002/diff/60001/tools/perf/core/perf_da...
File tools/perf/core/perf_data_generator.py (right):

https://codereview.chromium.org/2822723002/diff/60001/tools/perf/core/perf_da...
tools/perf/core/perf_data_generator.py:585: benchmark_ref_build_blacklist=None):
On 2017/04/14 20:45:59, nednguyen wrote:
> remove the default param here

Done.

https://codereview.chromium.org/2822723002/diff/60001/tools/perf/core/perf_da...
File tools/perf/core/perf_data_generator_unittest.py (right):

https://codereview.chromium.org/2822723002/diff/60001/tools/perf/core/perf_da...
tools/perf/core/perf_data_generator_unittest.py:10: class
_MockBenchmark(object):
On 2017/04/14 20:45:59, nednguyen wrote:
> why not just inherits this from the real benchmark.Benchmark?

My intention was to not create unnecessary dependencies on catapult.

https://codereview.chromium.org/2822723002/diff/60001/tools/perf/core/perf_da...
tools/perf/core/perf_data_generator_unittest.py:137: expected_generated_tests =
[
On 2017/04/14 20:45:59, nednguyen wrote:
> I think we only need a a few test methods that test the generated data. For
this
> one, you just need to test that there are 3 tests get generated:
> 
> generated_test_names = set(t.name for t in tests)
> self.assertEquals(generated_test_names, {'blacklisted', 'not_blacklisted', 
> 'not_blacklisted'})

Done.

[rnephew (Reviews Here), 2017-04-14 21:50:59]

Its now using the telemetry benchmark class.

[rnephew (Reviews Here), 2017-04-14 21:51:06]

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

[commit-bot: I haz the power, 2017-04-14 21:51:31]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[nednguyen, 2017-04-14 21:53:19]

lgtm

[commit-bot: I haz the power, 2017-04-14 22:51:58]

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

[commit-bot: I haz the power, 2017-04-14 22:51:59]

Dry run: This issue passed the CQ dry run.

[rnephew (Reviews Here), 2017-04-14 23:13:58]

The CQ bit was checked by rnephew@chromium.org

[commit-bot: I haz the power, 2017-04-14 23:14:31]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-14 23:22:10]

CQ is committing da patch.

Bot data: {"patchset_id": 90001, "attempt_start_ts": 1492211638979150,
"parent_rev": "a77f67e8518b816756cbc169f8985cbe34cf45d5", "commit_rev":
"5194feb68616e1a322380a5e8b5d3f38f28404e4"}

[commit-bot: I haz the power, 2017-04-14 23:23:05]

Description was changed from

==========
[Telemetry]Do not run power.idle_platform for reference build runs

BUG=700022
==========

to

==========
[Telemetry]Do not run power.idle_platform for reference build runs

BUG=700022

Review-Url: https://codereview.chromium.org/2822723002
Cr-Commit-Position: refs/heads/master@{#464822}
Committed:
https://chromium.googlesource.com/chromium/src/+/5194feb68616e1a322380a5e8b5d...
==========

[commit-bot: I haz the power, 2017-04-14 23:23:06]

Committed patchset #5 (id:90001) as
https://chromium.googlesource.com/chromium/src/+/5194feb68616e1a322380a5e8b5d...