[Telemetry] Add 'check_independent' command to benchmark_runner

[Telemetry] Add 'check_independent' command to benchmark_runner

This CL adds a new 'check_independent' subcommand to benchmark_runner, which returns
0 if stories are independent (can be run alone reliably using --story-filter),
or 1 if not.

It also adds a new strategy for integration testing benchmark_runner.

BUG=catapult:#1811

Committed: https://chromium.googlesource.com/external/github.com/catapult-project/catapult/+/60197f381eba6f956d3b264530925b3b452ab9dc

[eakuefner, 2016-08-30 18:48:18]

eakuefner@chromium.org changed reviewers:
+ dtu@chromium.org, nednguyen@google.com, sullivan@chromium.org

[eakuefner, 2016-08-30 18:48:19]

Folks, PTAL.

[nednguyen, 2016-08-30 18:50:43]

On 2016/08/30 18:48:19, eakuefner wrote:
> Folks, PTAL.

Did we settle down on this approach vs the other one that signal whether the
benchmark is independent through chart json?

[sullivan, 2016-08-30 18:54:36]

On 2016/08/30 18:50:43, nednguyen wrote:
> On 2016/08/30 18:48:19, eakuefner wrote:
> > Folks, PTAL.
> 
> Did we settle down on this approach vs the other one that signal whether the
> benchmark is independent through chart json?

Yeah, we settled on this approach because it is more bisectable (will give the
correct result even if it changes over time)

lgtm

[nednguyen, 2016-08-30 18:57:13]

On 2016/08/30 18:54:36, sullivan wrote:
> On 2016/08/30 18:50:43, nednguyen wrote:
> > On 2016/08/30 18:48:19, eakuefner wrote:
> > > Folks, PTAL.
> > 
> > Did we settle down on this approach vs the other one that signal whether the
> > benchmark is independent through chart json?
> 
> Yeah, we settled on this approach because it is more bisectable (will give the
> correct result even if it changes over time)
> 
> lgtm

I think that issue applies for both approach? If a benchmark changes from
independent to dependent, both approach will discover that start CL & end CL
have different "independent" state?

[nednguyen, 2016-08-30 18:58:31]

On 2016/08/30 18:57:13, nednguyen wrote:
> On 2016/08/30 18:54:36, sullivan wrote:
> > On 2016/08/30 18:50:43, nednguyen wrote:
> > > On 2016/08/30 18:48:19, eakuefner wrote:
> > > > Folks, PTAL.
> > > 
> > > Did we settle down on this approach vs the other one that signal whether
the
> > > benchmark is independent through chart json?
> > 
> > Yeah, we settled on this approach because it is more bisectable (will give
the
> > correct result even if it changes over time)
> > 
> > lgtm
> 
> I think that issue applies for both approach? If a benchmark changes from
> independent to dependent, both approach will discover that start CL & end CL
> have different "independent" state?

The nice thing about plumbing this info to the value system is that we can
easily add "You can reproduce this value by running command .." diagnostic in
our value system.

[sullivan, 2016-08-30 19:11:53]

On 2016/08/30 18:58:31, nednguyen wrote:
> On 2016/08/30 18:57:13, nednguyen wrote:
> > On 2016/08/30 18:54:36, sullivan wrote:
> > > On 2016/08/30 18:50:43, nednguyen wrote:
> > > > On 2016/08/30 18:48:19, eakuefner wrote:
> > > > > Folks, PTAL.
> > > > 
> > > > Did we settle down on this approach vs the other one that signal whether
> the
> > > > benchmark is independent through chart json?
> > > 
> > > Yeah, we settled on this approach because it is more bisectable (will give
> the
> > > correct result even if it changes over time)
> > > 
> > > lgtm
> > 
> > I think that issue applies for both approach? If a benchmark changes from
> > independent to dependent, both approach will discover that start CL & end CL
> > have different "independent" state?
> 
> The nice thing about plumbing this info to the value system is that we can
> easily add "You can reproduce this value by running command .." diagnostic in
> our value system.

We generally plumb bits of data like this per-test, not per-revision--if we
stored all of this at the revision level it would be a LOT of duplicated data.

[nednguyen, 2016-08-30 19:46:18]

On 2016/08/30 19:11:53, sullivan wrote:
> On 2016/08/30 18:58:31, nednguyen wrote:
> > On 2016/08/30 18:57:13, nednguyen wrote:
> > > On 2016/08/30 18:54:36, sullivan wrote:
> > > > On 2016/08/30 18:50:43, nednguyen wrote:
> > > > > On 2016/08/30 18:48:19, eakuefner wrote:
> > > > > > Folks, PTAL.
> > > > > 
> > > > > Did we settle down on this approach vs the other one that signal
whether
> > the
> > > > > benchmark is independent through chart json?
> > > > 
> > > > Yeah, we settled on this approach because it is more bisectable (will
give
> > the
> > > > correct result even if it changes over time)
> > > > 
> > > > lgtm
> > > 
> > > I think that issue applies for both approach? If a benchmark changes from
> > > independent to dependent, both approach will discover that start CL & end
CL
> > > have different "independent" state?
> > 
> > The nice thing about plumbing this info to the value system is that we can
> > easily add "You can reproduce this value by running command .." diagnostic
in
> > our value system.
> 
> We generally plumb bits of data like this per-test, not per-revision--if we
> stored all of this at the revision level it would be a LOT of duplicated data.

How does adding one extra bit of info to the chart json is a lot of duplicated
data given that we already plumb benchmark name, story name, & iteration info
for every single value?

[sullivan, 2016-08-30 20:31:56]

On 2016/08/30 19:46:18, nednguyen wrote:
> On 2016/08/30 19:11:53, sullivan wrote:
> > On 2016/08/30 18:58:31, nednguyen wrote:
> > > On 2016/08/30 18:57:13, nednguyen wrote:
> > > > On 2016/08/30 18:54:36, sullivan wrote:
> > > > > On 2016/08/30 18:50:43, nednguyen wrote:
> > > > > > On 2016/08/30 18:48:19, eakuefner wrote:
> > > > > > > Folks, PTAL.
> > > > > > 
> > > > > > Did we settle down on this approach vs the other one that signal
> whether
> > > the
> > > > > > benchmark is independent through chart json?
> > > > > 
> > > > > Yeah, we settled on this approach because it is more bisectable (will
> give
> > > the
> > > > > correct result even if it changes over time)
> > > > > 
> > > > > lgtm
> > > > 
> > > > I think that issue applies for both approach? If a benchmark changes
from
> > > > independent to dependent, both approach will discover that start CL &
end
> CL
> > > > have different "independent" state?
> > > 
> > > The nice thing about plumbing this info to the value system is that we can
> > > easily add "You can reproduce this value by running command .." diagnostic
> in
> > > our value system.
> > 
> > We generally plumb bits of data like this per-test, not per-revision--if we
> > stored all of this at the revision level it would be a LOT of duplicated
data.
> 
> How does adding one extra bit of info to the chart json is a lot of duplicated
> data given that we already plumb benchmark name, story name, & iteration info
> for every single value?

We plumb it through, but we do not store it per-value. If we plumbed this
through, we'd want to store it per-value to get the same functionality as this
CL.

[nednguyen, 2016-08-30 20:42:32]

On 2016/08/30 20:31:56, sullivan wrote:
> On 2016/08/30 19:46:18, nednguyen wrote:
> > On 2016/08/30 19:11:53, sullivan wrote:
> > > On 2016/08/30 18:58:31, nednguyen wrote:
> > > > On 2016/08/30 18:57:13, nednguyen wrote:
> > > > > On 2016/08/30 18:54:36, sullivan wrote:
> > > > > > On 2016/08/30 18:50:43, nednguyen wrote:
> > > > > > > On 2016/08/30 18:48:19, eakuefner wrote:
> > > > > > > > Folks, PTAL.
> > > > > > > 
> > > > > > > Did we settle down on this approach vs the other one that signal
> > whether
> > > > the
> > > > > > > benchmark is independent through chart json?
> > > > > > 
> > > > > > Yeah, we settled on this approach because it is more bisectable
(will
> > give
> > > > the
> > > > > > correct result even if it changes over time)
> > > > > > 
> > > > > > lgtm
> > > > > 
> > > > > I think that issue applies for both approach? If a benchmark changes
> from
> > > > > independent to dependent, both approach will discover that start CL &
> end
> > CL
> > > > > have different "independent" state?
> > > > 
> > > > The nice thing about plumbing this info to the value system is that we
can
> > > > easily add "You can reproduce this value by running command .."
diagnostic
> > in
> > > > our value system.
> > > 
> > > We generally plumb bits of data like this per-test, not per-revision--if
we
> > > stored all of this at the revision level it would be a LOT of duplicated
> data.
> > 
> > How does adding one extra bit of info to the chart json is a lot of
duplicated
> > data given that we already plumb benchmark name, story name, & iteration
info
> > for every single value?
> 
> We plumb it through, but we do not store it per-value. If we plumbed this
> through, we'd want to store it per-value to get the same functionality as this
> CL.

Ah right. Alternatively, when once click "bisect", the server can make an extra
query to get the original chart json & pick "--per-revision" out. Though in term
of use cases for dashboard UI & feature about adding direction for user to rerun
benchmark, I think it's beneficial to store this "independent" for per value
basis. 

Given that each value these day is a histogram with other diagnostics like
benchmark name, story name,etc.., I think adding this extra field will only
change the total size of each value slightly?

[eakuefner, 2016-08-30 21:05:31]

On 2016/08/30 at 20:42:32, nednguyen wrote:
> On 2016/08/30 20:31:56, sullivan wrote:
> > On 2016/08/30 19:46:18, nednguyen wrote:
> > > On 2016/08/30 19:11:53, sullivan wrote:
> > > > On 2016/08/30 18:58:31, nednguyen wrote:
> > > > > On 2016/08/30 18:57:13, nednguyen wrote:
> > > > > > On 2016/08/30 18:54:36, sullivan wrote:
> > > > > > > On 2016/08/30 18:50:43, nednguyen wrote:
> > > > > > > > On 2016/08/30 18:48:19, eakuefner wrote:
> > > > > > > > > Folks, PTAL.
> > > > > > > > 
> > > > > > > > Did we settle down on this approach vs the other one that signal
> > > whether
> > > > > the
> > > > > > > > benchmark is independent through chart json?
> > > > > > > 
> > > > > > > Yeah, we settled on this approach because it is more bisectable
(will
> > > give
> > > > > the
> > > > > > > correct result even if it changes over time)
> > > > > > > 
> > > > > > > lgtm
> > > > > > 
> > > > > > I think that issue applies for both approach? If a benchmark changes
> > from
> > > > > > independent to dependent, both approach will discover that start CL
&
> > end
> > > CL
> > > > > > have different "independent" state?
> > > > > 
> > > > > The nice thing about plumbing this info to the value system is that we
can
> > > > > easily add "You can reproduce this value by running command .."
diagnostic
> > > in
> > > > > our value system.
> > > > 
> > > > We generally plumb bits of data like this per-test, not per-revision--if
we
> > > > stored all of this at the revision level it would be a LOT of duplicated
> > data.
> > > 
> > > How does adding one extra bit of info to the chart json is a lot of
duplicated
> > > data given that we already plumb benchmark name, story name, & iteration
info
> > > for every single value?
> > 
> > We plumb it through, but we do not store it per-value. If we plumbed this
> > through, we'd want to store it per-value to get the same functionality as
this
> > CL.
> 
> Ah right. Alternatively, when once click "bisect", the server can make an
extra query to get the original chart json & pick "--per-revision" out. Though
in term of use cases for dashboard UI & feature about adding direction for user
to rerun benchmark, I think it's beneficial to store this "independent" for per
value basis. 
> 
> Given that each value these day is a histogram with other diagnostics like
benchmark name, story name,etc.., I think adding this extra field will only
change the total size of each value slightly?

Stepping back from these questions about storage, the approach of pulling it out
of chart JSON cannot work unless you have an existing chart JSON result for
_every_ revision in the range on which you are about to bisect. We don't test or
upload per-revision, and the whole point of this scheme is to avoid running the
benchmarks unnecessarily, so I don't see how the approach of pulling "is this
benchmark independent for this exact revision" from chart JSON can work.

[nednguyen(REVIEW IN OTHER ACC), 2016-08-30 21:23:46]

On 2016/08/30 21:05:31, eakuefner wrote:
> On 2016/08/30 at 20:42:32, nednguyen wrote:
> > On 2016/08/30 20:31:56, sullivan wrote:
> > > On 2016/08/30 19:46:18, nednguyen wrote:
> > > > On 2016/08/30 19:11:53, sullivan wrote:
> > > > > On 2016/08/30 18:58:31, nednguyen wrote:
> > > > > > On 2016/08/30 18:57:13, nednguyen wrote:
> > > > > > > On 2016/08/30 18:54:36, sullivan wrote:
> > > > > > > > On 2016/08/30 18:50:43, nednguyen wrote:
> > > > > > > > > On 2016/08/30 18:48:19, eakuefner wrote:
> > > > > > > > > > Folks, PTAL.
> > > > > > > > > 
> > > > > > > > > Did we settle down on this approach vs the other one that
signal
> > > > whether
> > > > > > the
> > > > > > > > > benchmark is independent through chart json?
> > > > > > > > 
> > > > > > > > Yeah, we settled on this approach because it is more bisectable
> (will
> > > > give
> > > > > > the
> > > > > > > > correct result even if it changes over time)
> > > > > > > > 
> > > > > > > > lgtm
> > > > > > > 
> > > > > > > I think that issue applies for both approach? If a benchmark
changes
> > > from
> > > > > > > independent to dependent, both approach will discover that start
CL
> &
> > > end
> > > > CL
> > > > > > > have different "independent" state?
> > > > > > 
> > > > > > The nice thing about plumbing this info to the value system is that
we
> can
> > > > > > easily add "You can reproduce this value by running command .."
> diagnostic
> > > > in
> > > > > > our value system.
> > > > > 
> > > > > We generally plumb bits of data like this per-test, not
per-revision--if
> we
> > > > > stored all of this at the revision level it would be a LOT of
duplicated
> > > data.
> > > > 
> > > > How does adding one extra bit of info to the chart json is a lot of
> duplicated
> > > > data given that we already plumb benchmark name, story name, & iteration
> info
> > > > for every single value?
> > > 
> > > We plumb it through, but we do not store it per-value. If we plumbed this
> > > through, we'd want to store it per-value to get the same functionality as
> this
> > > CL.
> > 
> > Ah right. Alternatively, when once click "bisect", the server can make an
> extra query to get the original chart json & pick "--per-revision" out. Though
> in term of use cases for dashboard UI & feature about adding direction for
user
> to rerun benchmark, I think it's beneficial to store this "independent" for
per
> value basis. 
> > 
> > Given that each value these day is a histogram with other diagnostics like
> benchmark name, story name,etc.., I think adding this extra field will only
> change the total size of each value slightly?
> 
> Stepping back from these questions about storage, the approach of pulling it
out
> of chart JSON cannot work unless you have an existing chart JSON result for
> _every_ revision in the range on which you are about to bisect. We don't test
or
> upload per-revision, and the whole point of this scheme is to avoid running
the
> benchmarks unnecessarily, so I don't see how the approach of pulling "is this
> benchmark independent for this exact revision" from chart JSON can work.

Ah, that's a good point that we may bisect on revision that we didn't run on
waterfall. Seems like this is the only approach that work then.

Lgtm

Can you add some test coverage?

[eakuefner, 2016-08-30 21:45:03]

On 2016/08/30 at 21:23:46, nednguyen wrote:
> On 2016/08/30 21:05:31, eakuefner wrote:
> > On 2016/08/30 at 20:42:32, nednguyen wrote:
> > > On 2016/08/30 20:31:56, sullivan wrote:
> > > > On 2016/08/30 19:46:18, nednguyen wrote:
> > > > > On 2016/08/30 19:11:53, sullivan wrote:
> > > > > > On 2016/08/30 18:58:31, nednguyen wrote:
> > > > > > > On 2016/08/30 18:57:13, nednguyen wrote:
> > > > > > > > On 2016/08/30 18:54:36, sullivan wrote:
> > > > > > > > > On 2016/08/30 18:50:43, nednguyen wrote:
> > > > > > > > > > On 2016/08/30 18:48:19, eakuefner wrote:
> > > > > > > > > > > Folks, PTAL.
> > > > > > > > > > 
> > > > > > > > > > Did we settle down on this approach vs the other one that
signal
> > > > > whether
> > > > > > > the
> > > > > > > > > > benchmark is independent through chart json?
> > > > > > > > > 
> > > > > > > > > Yeah, we settled on this approach because it is more
bisectable
> > (will
> > > > > give
> > > > > > > the
> > > > > > > > > correct result even if it changes over time)
> > > > > > > > > 
> > > > > > > > > lgtm
> > > > > > > > 
> > > > > > > > I think that issue applies for both approach? If a benchmark
changes
> > > > from
> > > > > > > > independent to dependent, both approach will discover that start
CL
> > &
> > > > end
> > > > > CL
> > > > > > > > have different "independent" state?
> > > > > > > 
> > > > > > > The nice thing about plumbing this info to the value system is
that we
> > can
> > > > > > > easily add "You can reproduce this value by running command .."
> > diagnostic
> > > > > in
> > > > > > > our value system.
> > > > > > 
> > > > > > We generally plumb bits of data like this per-test, not
per-revision--if
> > we
> > > > > > stored all of this at the revision level it would be a LOT of
duplicated
> > > > data.
> > > > > 
> > > > > How does adding one extra bit of info to the chart json is a lot of
> > duplicated
> > > > > data given that we already plumb benchmark name, story name, &
iteration
> > info
> > > > > for every single value?
> > > > 
> > > > We plumb it through, but we do not store it per-value. If we plumbed
this
> > > > through, we'd want to store it per-value to get the same functionality
as
> > this
> > > > CL.
> > > 
> > > Ah right. Alternatively, when once click "bisect", the server can make an
> > extra query to get the original chart json & pick "--per-revision" out.
Though
> > in term of use cases for dashboard UI & feature about adding direction for
user
> > to rerun benchmark, I think it's beneficial to store this "independent" for
per
> > value basis. 
> > > 
> > > Given that each value these day is a histogram with other diagnostics like
> > benchmark name, story name,etc.., I think adding this extra field will only
> > change the total size of each value slightly?
> > 
> > Stepping back from these questions about storage, the approach of pulling it
out
> > of chart JSON cannot work unless you have an existing chart JSON result for
> > _every_ revision in the range on which you are about to bisect. We don't
test or
> > upload per-revision, and the whole point of this scheme is to avoid running
the
> > benchmarks unnecessarily, so I don't see how the approach of pulling "is
this
> > benchmark independent for this exact revision" from chart JSON can work.
> 
> Ah, that's a good point that we may bisect on revision that we didn't run on
waterfall. Seems like this is the only approach that work then.
> 
> Lgtm
> 
> Can you add some test coverage?

Yes, I will. Also, just worth pointing out: while it's urgent that we allow this
now, overall, neither Dave nor I really have confidence that this is the right
long-term approach, because:

1. If we will allow benchmarks that do not tear down the story after every run,
it should be up to bisect to tell Telemetry that it _intends_ to bisect on a
given story, and up to Telemetry to decide whether it can run only that story or
not (i.e. --story-filter might be the wrong abstraction), but even more broadly:
2. If it's only a tracing problem that means that long-running benchmarks cannot
be implemented using a single story (thus making the choice of whether to tear
down the state after each story obsolete), we should make it easier to start and
stop tracing during the course of a story (this might need the public browser
API that I stopped working on, but could start working on if we wanted to do
this).

I'm going to send an email at some point, hopefully this week, that works
through all of these points.

[eakuefner, 2016-08-31 02:45:55]

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

[commit-bot: I haz the power, 2016-08-31 02:46:01]

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

[eakuefner, 2016-08-31 02:51:27]

The CQ bit was unchecked by eakuefner@chromium.org

[eakuefner, 2016-08-31 17:21:28]

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

[commit-bot: I haz the power, 2016-08-31 17:21:31]

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-31 17:42:42]

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

[commit-bot: I haz the power, 2016-08-31 17:42:43]

Dry run: This issue passed the CQ dry run.

[nednguyen, 2016-09-08 22:44:21]

https://codereview.chromium.org/2291313002/diff/1/telemetry/telemetry/benchma...
File telemetry/telemetry/benchmark_runner.py (right):

https://codereview.chromium.org/2291313002/diff/1/telemetry/telemetry/benchma...
telemetry/telemetry/benchmark_runner.py:425: all_commands = [Help, Independent,
List, Run] + extra_commands
rename Independent to CheckIndependent since all commands should be verb

[nednguyen, 2016-09-08 22:44:40]

On 2016/09/08 22:44:21, nednguyen wrote:
>
https://codereview.chromium.org/2291313002/diff/1/telemetry/telemetry/benchma...
> File telemetry/telemetry/benchmark_runner.py (right):
> 
>
https://codereview.chromium.org/2291313002/diff/1/telemetry/telemetry/benchma...
> telemetry/telemetry/benchmark_runner.py:425: all_commands = [Help,
Independent,
> List, Run] + extra_commands
> rename Independent to CheckIndependent since all commands should be verb

err, CheckIndependence

[eakuefner, 2016-09-08 22:46:36]

Description was changed from

==========
[Telemetry] Add 'independent' command to benchmark_runner

This CL adds a new 'independent' subcommand to benchmark_runner, which returns
0 if stories are independent (can be run alone reliably using --story-filter),
or 1 if not.

BUG=catapult:#1811
==========

to

==========
[Telemetry] Add 'check_independent' command to benchmark_runner

This CL adds a new 'check_independent' subcommand to benchmark_runner, which
returns
0 if stories are independent (can be run alone reliably using --story-filter),
or 1 if not.

BUG=catapult:#1811
==========

[eakuefner, 2016-09-08 22:46:36]

eakuefner@chromium.org changed reviewers:
+ nednguyen@chromium.org

[eakuefner, 2016-09-08 22:46:55]

On 2016/09/08 at 22:44:40, nednguyen wrote:
> On 2016/09/08 22:44:21, nednguyen wrote:
> >
https://codereview.chromium.org/2291313002/diff/1/telemetry/telemetry/benchma...
> > File telemetry/telemetry/benchmark_runner.py (right):
> > 
> >
https://codereview.chromium.org/2291313002/diff/1/telemetry/telemetry/benchma...
> > telemetry/telemetry/benchmark_runner.py:425: all_commands = [Help,
Independent,
> > List, Run] + extra_commands
> > rename Independent to CheckIndependent since all commands should be verb
> 
> err, CheckIndependence

Renamed to CheckIndependent.

[eakuefner, 2016-09-08 22:48:58]

The CQ bit was checked by eakuefner@chromium.org

[eakuefner, 2016-09-08 22:48:58]

The patchset sent to the CQ was uploaded after l-g-t-m from
sullivan@chromium.org, nednguyen@chromium.org
Link to the patchset: https://codereview.chromium.org/2291313002/#ps20001
(title: "Address Ned's comments")

[eakuefner, 2016-09-08 22:49:03]

The CQ bit was unchecked by eakuefner@chromium.org

[eakuefner, 2016-09-08 22:49:20]

Description was changed from

==========
[Telemetry] Add 'check_independent' command to benchmark_runner

This CL adds a new 'check_independent' subcommand to benchmark_runner, which
returns
0 if stories are independent (can be run alone reliably using --story-filter),
or 1 if not.

BUG=catapult:#1811
==========

to

==========
[Telemetry] Add 'check_independent' command to benchmark_runner

This CL adds a new 'check_independent' subcommand to benchmark_runner, which
returns
0 if stories are independent (can be run alone reliably using --story-filter),
or 1 if not.

It also adds a new strategy for integration testing benchmark_runner.

BUG=catapult:#1811
==========

[eakuefner, 2016-09-08 22:49:24]

The CQ bit was checked by eakuefner@chromium.org

[commit-bot: I haz the power, 2016-09-08 22:49:27]

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

[nednguyen, 2016-09-08 22:49:37]

lgtm

https://codereview.chromium.org/2291313002/diff/20001/telemetry/telemetry/tes...
File telemetry/telemetry/testdata/run_benchmark_integration_test_benchmarks.py
(right):

https://codereview.chromium.org/2291313002/diff/20001/telemetry/telemetry/tes...
telemetry/telemetry/testdata/run_benchmark_integration_test_benchmarks.py:5: 
nits: add space

https://codereview.chromium.org/2291313002/diff/20001/telemetry/telemetry/tes...
telemetry/telemetry/testdata/run_benchmark_integration_test_benchmarks.py:24: 
nits: remove extra space

[eakuefner, 2016-09-08 22:49:58]

The CQ bit was unchecked by eakuefner@chromium.org

[eakuefner, 2016-09-08 22:51:38]

The CQ bit was checked by eakuefner@chromium.org

[eakuefner, 2016-09-08 22:51:38]

The patchset sent to the CQ was uploaded after l-g-t-m from
nednguyen@chromium.org, nednguyen@google.com, sullivan@chromium.org
Link to the patchset: https://codereview.chromium.org/2291313002/#ps40001
(title: "address ned's nits")

[commit-bot: I haz the power, 2016-09-08 22:51:47]

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-08 23:39:24]

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

[commit-bot: I haz the power, 2016-09-08 23:39:25]

Try jobs failed on following builders:
  Catapult Mac Tryserver on master.tryserver.client.catapult (JOB_FAILED,
https://build.chromium.org/p/tryserver.client.catapult/builders/Catapult%20Ma...)

[eakuefner, 2016-09-21 20:57:42]

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

[commit-bot: I haz the power, 2016-09-21 20:57:49]

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-09-21 21:10:13]

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

[commit-bot: I haz the power, 2016-09-21 21:10:14]

Dry run: Try jobs failed on following builders:
  Catapult Linux Tryserver on master.tryserver.client.catapult (JOB_FAILED,
https://build.chromium.org/p/tryserver.client.catapult/builders/Catapult%20Li...)
  Catapult Windows Tryserver on master.tryserver.client.catapult (JOB_FAILED,
https://build.chromium.org/p/tryserver.client.catapult/builders/Catapult%20Wi...)

[eakuefner, 2016-09-21 23:17:26]

Had to fix a unit test; landing this now.

Dave, feel free to begin using this in the bisect recipe to ensure that
--story-filter is passed.

https://codereview.chromium.org/2291313002/diff/20001/telemetry/telemetry/tes...
File telemetry/telemetry/testdata/run_benchmark_integration_test_benchmarks.py
(right):

https://codereview.chromium.org/2291313002/diff/20001/telemetry/telemetry/tes...
telemetry/telemetry/testdata/run_benchmark_integration_test_benchmarks.py:5: 
On 2016/09/08 at 22:49:37, nednguyen wrote:
> nits: add space

Done.

https://codereview.chromium.org/2291313002/diff/20001/telemetry/telemetry/tes...
telemetry/telemetry/testdata/run_benchmark_integration_test_benchmarks.py:24: 
On 2016/09/08 at 22:49:37, nednguyen wrote:
> nits: remove extra space

Done.

[eakuefner, 2016-09-21 23:17:28]

The CQ bit was checked by eakuefner@chromium.org

[eakuefner, 2016-09-21 23:17:29]

The patchset sent to the CQ was uploaded after l-g-t-m from
nednguyen@chromium.org, nednguyen@google.com, sullivan@chromium.org
Link to the patchset: https://codereview.chromium.org/2291313002/#ps60001
(title: "add testdata __init__.py")

[commit-bot: I haz the power, 2016-09-21 23:17:35]

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-21 23:51:51]

Description was changed from

==========
[Telemetry] Add 'check_independent' command to benchmark_runner

This CL adds a new 'check_independent' subcommand to benchmark_runner, which
returns
0 if stories are independent (can be run alone reliably using --story-filter),
or 1 if not.

It also adds a new strategy for integration testing benchmark_runner.

BUG=catapult:#1811
==========

to

==========
[Telemetry] Add 'check_independent' command to benchmark_runner

This CL adds a new 'check_independent' subcommand to benchmark_runner, which
returns
0 if stories are independent (can be run alone reliably using --story-filter),
or 1 if not.

It also adds a new strategy for integration testing benchmark_runner.

BUG=catapult:#1811

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

[commit-bot: I haz the power, 2016-09-21 23:51:52]

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

[nednguyen, 2016-10-06 15:17:31]

On 2016/09/21 23:51:52, commit-bot: I haz the power wrote:
> Committed patchset #4 (id:60001) as
>
https://chromium.googlesource.com/external/github.com/catapult-project/catapu...

Can we revert this CL now?

[eakuefner, 2016-10-07 16:18:26]

On 2016/10/06 at 15:17:31, nednguyen wrote:
> On 2016/09/21 23:51:52, commit-bot: I haz the power wrote:
> > Committed patchset #4 (id:60001) as
> >
https://chromium.googlesource.com/external/github.com/catapult-project/catapu...
> 
> Can we revert this CL now?

Yes.

[eakuefner, 2016-10-07 16:18:36]

A revert of this CL (patchset #4 id:60001) has been created in
https://codereview.chromium.org/2395233004/ by eakuefner@chromium.org.

The reason for reverting is: No longer needed..