Updating telemetry logic to return success when a disabled test is run.

Updating telemetry logic to return success when a disabled test is run.

BUG=chromium:647714
Committed: https://chromium.googlesource.com/external/github.com/catapult-project/catapult/+/addb27e71cc964df2305a739fe56bb0c628eb4af

[eyaich1, 2016-08-24 18:51:02]

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

[commit-bot: I haz the power, 2016-08-24 18:51:10]

Dry run: 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, 2016-08-24 21:53:41]

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

[commit-bot: I haz the power, 2016-08-24 21:53:41]

Dry run: This issue passed the CQ dry run.

[eyaich1, 2016-08-24 23:12:56]

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

[nednguyen, 2016-08-25 00:41:00]

lgtm

Can you make an announcement to telemetry-announce mailing list & wait for 1-2
days before landing this?

[eyaich1, 2016-09-16 17:07:29]

Description was changed from

==========
Updating telemetry logic to return success when a disabled test is run.

BUG=chromium:633253
==========

to

==========
Updating telemetry logic to return success when a disabled test is run.

BUG=chromium:647714
==========

[eyaich1, 2016-09-29 12:46:30]

On 2016/08/25 00:41:00, nednguyen wrote:
> lgtm
> 
> Can you make an announcement to telemetry-announce mailing list & wait for 1-2
> days before landing this?

As soon as https://codereview.chromium.org/2347163003/ lands this will be the
last piece of the puzzle for the swarming recipe and the waterfall should be
green.  PTAL as I have added the shared_page_state.py file

[nednguyen, 2016-09-29 12:55:52]

lgtm

https://codereview.chromium.org/2265423005/diff/20001/telemetry/telemetry/int...
File telemetry/telemetry/internal/story_runner.py (right):

https://codereview.chromium.org/2265423005/diff/20001/telemetry/telemetry/int...
telemetry/telemetry/internal/story_runner.py:285: logging.warning('%s is
disabled on the selected browser', benchmark.Name())
Can you change this from warning to print since not everyone run benchmark with
logging enabled?

https://codereview.chromium.org/2265423005/diff/20001/telemetry/telemetry/int...
telemetry/telemetry/internal/story_runner.py:288: 'Running benchmark anyway due
to: --also-run-disabled-tests')
same here

https://codereview.chromium.org/2265423005/diff/20001/telemetry/telemetry/int...
telemetry/telemetry/internal/story_runner.py:290: logging.warning(
same here

[eyaich1, 2016-09-29 13:02:52]

https://codereview.chromium.org/2265423005/diff/20001/telemetry/telemetry/int...
File telemetry/telemetry/internal/story_runner.py (right):

https://codereview.chromium.org/2265423005/diff/20001/telemetry/telemetry/int...
telemetry/telemetry/internal/story_runner.py:285: logging.warning('%s is
disabled on the selected browser', benchmark.Name())
On 2016/09/29 12:55:52, nednguyen wrote:
> Can you change this from warning to print since not everyone run benchmark
with
> logging enabled?

Done.

https://codereview.chromium.org/2265423005/diff/20001/telemetry/telemetry/int...
telemetry/telemetry/internal/story_runner.py:288: 'Running benchmark anyway due
to: --also-run-disabled-tests')
On 2016/09/29 12:55:52, nednguyen wrote:
> same here

Done.

https://codereview.chromium.org/2265423005/diff/20001/telemetry/telemetry/int...
telemetry/telemetry/internal/story_runner.py:290: logging.warning(
On 2016/09/29 12:55:52, nednguyen wrote:
> same here

Done.

[nednguyen, 2016-09-30 14:01:46]

lgtm

https://codereview.chromium.org/2265423005/diff/60001/telemetry/telemetry/int...
File telemetry/telemetry/internal/story_runner.py (right):

https://codereview.chromium.org/2265423005/diff/60001/telemetry/telemetry/int...
telemetry/telemetry/internal/story_runner.py:288: print disabled_benchmark
nits: can you just inline the print & remove disabled_benchmark variable?

https://codereview.chromium.org/2265423005/diff/60001/telemetry/telemetry/pag...
File telemetry/telemetry/page/shared_page_state.py (right):

https://codereview.chromium.org/2265423005/diff/60001/telemetry/telemetry/pag...
telemetry/telemetry/page/shared_page_state.py:133: 'This will not generate valid
results.')
I think we can kill this logging. There is a variety of reason why people want
to run a disabled test anyway, and it's not true that doing so will always
generated invalid results

[eyaich1, 2016-09-30 14:30:01]

https://codereview.chromium.org/2265423005/diff/60001/telemetry/telemetry/int...
File telemetry/telemetry/internal/story_runner.py (right):

https://codereview.chromium.org/2265423005/diff/60001/telemetry/telemetry/int...
telemetry/telemetry/internal/story_runner.py:288: print disabled_benchmark
On 2016/09/30 14:01:45, nednguyen wrote:
> nits: can you just inline the print & remove disabled_benchmark variable?

I had it in my head that there was an issue when doing string interpolation
within a print statement, I thought I ran into an error once with that, but it
doesn't seem to be a problem.  Updated.

https://codereview.chromium.org/2265423005/diff/60001/telemetry/telemetry/pag...
File telemetry/telemetry/page/shared_page_state.py (right):

https://codereview.chromium.org/2265423005/diff/60001/telemetry/telemetry/pag...
telemetry/telemetry/page/shared_page_state.py:133: 'This will not generate valid
results.')
On 2016/09/30 14:01:45, nednguyen wrote:
> I think we can kill this logging. There is a variety of reason why people want
> to run a disabled test anyway, and it's not true that doing so will always
> generated invalid results

Done.

[eyaich1, 2016-09-30 14:54:52]

The CQ bit was checked by eyaich@chromium.org

[eyaich1, 2016-09-30 14:54:53]

The patchset sent to the CQ was uploaded after l-g-t-m from nednguyen@google.com
Link to the patchset: https://codereview.chromium.org/2265423005/#ps180001
(title: "Manually replaying changes after merge")

[commit-bot: I haz the power, 2016-09-30 14:55:05]

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, 2016-09-30 15:19:54]

Description was changed from

==========
Updating telemetry logic to return success when a disabled test is run.

BUG=chromium:647714
==========

to

==========
Updating telemetry logic to return success when a disabled test is run.

BUG=chromium:647714

Committed:
https://chromium.googlesource.com/external/github.com/catapult-project/catapu...
==========

[commit-bot: I haz the power, 2016-09-30 15:19:56]

Committed patchset #10 (id:180001) as
https://chromium.googlesource.com/external/github.com/catapult-project/catapu...