Merge suite and benchmark decorators

Merge suite and benchmark decorators

Currently, the suite decorator at line 29 is being ignored.

BUG=351114
Committed: https://crrev.com/5691d78e3d49a17b71e11f3da781f4459d17b1c6
Cr-Commit-Position: refs/heads/master@{#329899}

[achuithb, 2015-05-05 19:14:06]

achuith@chromium.org changed reviewers:
+ dtu@chromium.org

[achuithb, 2015-05-05 19:14:06]

D2, could you please take a look?

[dtu, 2015-05-05 21:09:09]

Thanks for the fix!

https://codereview.chromium.org/1128433007/diff/20001/tools/perf/benchmarks/b...
File tools/perf/benchmarks/benchmark_smoke_unittest.py (right):

https://codereview.chromium.org/1128433007/diff/20001/tools/perf/benchmarks/b...
tools/perf/benchmarks/benchmark_smoke_unittest.py:108: for attr in
['_enabled_strings', '_disabled_strings']:
style nit: don't abbreviate (not in google style guide, but in telemetry/cocoa
style)

https://codereview.chromium.org/1128433007/diff/20001/tools/perf/benchmarks/b...
tools/perf/benchmarks/benchmark_smoke_unittest.py:109: tmp = set()
Shorter:

merged_attributes = set(getattr(method, attribute, []) +
                        getattr(benchmark, attribute, []))

[achuithb, 2015-05-05 22:01:10]

Thanks for the feedback. PTAL, D2!

https://codereview.chromium.org/1128433007/diff/20001/tools/perf/benchmarks/b...
File tools/perf/benchmarks/benchmark_smoke_unittest.py (right):

https://codereview.chromium.org/1128433007/diff/20001/tools/perf/benchmarks/b...
tools/perf/benchmarks/benchmark_smoke_unittest.py:108: for attr in
['_enabled_strings', '_disabled_strings']:
On 2015/05/05 21:09:08, dtu wrote:
> style nit: don't abbreviate (not in google style guide, but in telemetry/cocoa
> style)

Done.

https://codereview.chromium.org/1128433007/diff/20001/tools/perf/benchmarks/b...
tools/perf/benchmarks/benchmark_smoke_unittest.py:109: tmp = set()
On 2015/05/05 21:09:08, dtu wrote:
> Shorter:
> 
> merged_attributes = set(getattr(method, attribute, []) +
>                         getattr(benchmark, attribute, []))

Done.

[dtu, 2015-05-06 18:42:40]

lgtm

[achuithb, 2015-05-06 18:43:21]

The CQ bit was checked by achuith@chromium.org

[commit-bot: I haz the power, 2015-05-06 18:44:41]

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

[commit-bot: I haz the power, 2015-05-06 18:51:09]

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

[commit-bot: I haz the power, 2015-05-06 18:51:10]

Try jobs failed on following builders:
  mac_perf_bisect on tryserver.chromium.perf (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.perf/builders/mac_perf_bisect/...)

[achuithb, 2015-05-06 19:11:13]

The CQ bit was checked by achuith@chromium.org

[commit-bot: I haz the power, 2015-05-06 19:11:59]

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

[commit-bot: I haz the power, 2015-05-06 21:50:43]

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

[commit-bot: I haz the power, 2015-05-06 21:50:44]

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

[achuithb, 2015-05-08 04:42:25]

The CQ bit was checked by achuith@chromium.org

[commit-bot: I haz the power, 2015-05-08 04:42:34]

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

[commit-bot: I haz the power, 2015-05-08 07:34:26]

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

[commit-bot: I haz the power, 2015-05-08 07:34:28]

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

[achuithb, 2015-05-12 22:31:55]

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

[achuithb, 2015-05-12 22:31:55]

The patchset sent to the CQ was uploaded after l-g-t-m from dtu@chromium.org
Link to the patchset: https://codereview.chromium.org/1128433007/#ps60001
(title: "fix global Disable")

[commit-bot: I haz the power, 2015-05-12 22:33:12]

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

[achuithb, 2015-05-12 22:34:20]

Dave, the bots caught an error in my logic. @Disabled actually sets the
_disabled_strings to [], which ends up getting ignored because we've used [] in
getattr as a sentinel for not set attribute.

I've fixed this, but the fix may be a bit clunky. PTAL, and feel free to suggest
improvements.

[commit-bot: I haz the power, 2015-05-13 00:36:24]

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

[commit-bot: I haz the power, 2015-05-13 00:36:25]

Dry run: Try jobs failed on following builders:
  win_perf_bisect on tryserver.chromium.perf (JOB_TIMED_OUT, no build URL)

[dtu, 2015-05-14 00:04:46]

On 2015/05/12 22:34:20, achuithb wrote:
> Dave, the bots caught an error in my logic. @Disabled actually sets the
> _disabled_strings to [], which ends up getting ignored because we've used []
in
> getattr as a sentinel for not set attribute.
> 
> I've fixed this, but the fix may be a bit clunky. PTAL, and feel free to
suggest
> improvements.

lgtm. Seems like the decorators were implemented in a clunky way, and fix should
be in their logic. If you have any ideas on that side, that would be great too!

[achuithb, 2015-05-14 00:07:14]

On 2015/05/14 00:04:46, dtu wrote:
> On 2015/05/12 22:34:20, achuithb wrote:
> > Dave, the bots caught an error in my logic. @Disabled actually sets the
> > _disabled_strings to [], which ends up getting ignored because we've used []
> in
> > getattr as a sentinel for not set attribute.
> > 
> > I've fixed this, but the fix may be a bit clunky. PTAL, and feel free to
> suggest
> > improvements.
> 
> lgtm. Seems like the decorators were implemented in a clunky way, and fix
should
> be in their logic. If you have any ideas on that side, that would be great
too!

Yup, I thought of changing the way _disabled_strings represents all platforms,
but I didn't want to break the world.

I'll take a look if I have cycles. Thanks for the review.

[achuithb, 2015-05-14 00:07:22]

The CQ bit was checked by achuith@chromium.org

[commit-bot: I haz the power, 2015-05-14 00:09:04]

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

[commit-bot: I haz the power, 2015-05-14 02:10:46]

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

[commit-bot: I haz the power, 2015-05-14 02:10:47]

Try jobs failed on following builders:
  linux_perf_bisect on tryserver.chromium.perf (JOB_TIMED_OUT, no build URL)

[achuithb, 2015-05-14 09:41:40]

The CQ bit was checked by achuith@chromium.org

[commit-bot: I haz the power, 2015-05-14 09:41:51]

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

[commit-bot: I haz the power, 2015-05-14 11:42:27]

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

[commit-bot: I haz the power, 2015-05-14 11:42:28]

Try jobs failed on following builders:
  win_perf_bisect on tryserver.chromium.perf (JOB_TIMED_OUT, no build URL)

[achuithb, 2015-05-14 19:24:16]

The CQ bit was checked by achuith@chromium.org

[commit-bot: I haz the power, 2015-05-14 19:27:58]

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

[commit-bot: I haz the power, 2015-05-14 19:39:24]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-05-14 19:41:08]

Patchset 4 (id:??) landed as
https://crrev.com/5691d78e3d49a17b71e11f3da781f4459d17b1c6
Cr-Commit-Position: refs/heads/master@{#329899}