Issue 2890283002: Migrate infinite scrolls to system health stories

nednguyen

The CQ bit was checked by nednguyen@google.com to run a CQ dry run

3 years, 7 months ago (2017-05-19 00:02:44 UTC) #1

commit-bot: I haz the power

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

3 years, 7 months ago (2017-05-19 00:05:25 UTC) #2

nednguyen

The CQ bit was checked by nednguyen@google.com to run a CQ dry run

3 years, 7 months ago (2017-05-19 00:08:14 UTC) #4

commit-bot: I haz the power

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

3 years, 7 months ago (2017-05-19 00:09:19 UTC) #5

nednguyen

The CQ bit was checked by nednguyen@google.com to run a CQ dry run

3 years, 7 months ago (2017-05-19 00:10:15 UTC) #6

commit-bot: I haz the power

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

3 years, 7 months ago (2017-05-19 00:10:39 UTC) #7

nednguyen

nednguyen@google.com changed reviewers: + perezju@chromium.org, ulan@chromium.org

3 years, 7 months ago (2017-05-19 00:11:06 UTC) #8

nednguyen

This add 5 new stories for desktop, 5 new stories for mobile, I am not ...

3 years, 7 months ago (2017-05-19 00:11:06 UTC) #9

commit-bot: I haz the power

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

3 years, 7 months ago (2017-05-19 01:57:23 UTC) #10

commit-bot: I haz the power

Dry run: Try jobs failed on following builders: android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_swarming_rel/builds/181955)

3 years, 7 months ago (2017-05-19 01:57:24 UTC) #11

perezju

My suggestion would be to add just 3 new browse stories (maybe discourse, flickr, and ...

3 years, 7 months ago (2017-05-19 09:40:36 UTC) #12

hjd

https://codereview.chromium.org/2890283002/diff/20001/tools/perf/page_sets/system_health/browsing_stories.py File tools/perf/page_sets/system_health/browsing_stories.py (right): https://codereview.chromium.org/2890283002/diff/20001/tools/perf/page_sets/system_health/browsing_stories.py#newcode815 tools/perf/page_sets/system_health/browsing_stories.py:815: def _Scroll(self, action_runner, distance, step_size): On 2017/05/19 09:40:35, perezju ...

3 years, 7 months ago (2017-05-19 10:42:21 UTC) #14

ulan

ulan@chromium.org changed reviewers: + hpayer@chromium.org, mlippautz@chromium.org

3 years, 7 months ago (2017-05-19 10:50:30 UTC) #15

ulan

+GC to reviewers. Hannes, Michael, wdyt about replacing some infinite scroll stories with browsing stories?

3 years, 7 months ago (2017-05-19 10:50:30 UTC) #16

ulan

Talked with GC team offline. It would be great to keep all the stories. https://codereview.chromium.org/2890283002/diff/20001/tools/perf/page_sets/system_health/browsing_stories.py ...

3 years, 7 months ago (2017-05-19 12:59:00 UTC) #17

Talked with GC team offline. It would be great to keep all the stories.

https://codereview.chromium.org/2890283002/diff/20001/tools/perf/page_sets/sy...
File tools/perf/page_sets/system_health/browsing_stories.py (right):

https://codereview.chromium.org/2890283002/diff/20001/tools/perf/page_sets/sy...
tools/perf/page_sets/system_health/browsing_stories.py:842: NAME =
'browse_infinite_scroll:forum:discourse'
On 2017/05/19 09:40:35, perezju wrote:
> I would call this: browse:social:discourse
> And add some 'infinite_scroll' tag instead.

How about browse:infinite_scroll:discourse? I would like to keep all these
stories in the same group and avoid confusion with other browse stories.

https://codereview.chromium.org/2890283002/diff/20001/tools/perf/page_sets/sy...
tools/perf/page_sets/system_health/browsing_stories.py:857: NAME =
'browse_infinite_scroll:social:facebook'
On 2017/05/19 09:40:35, perezju wrote:
> Could this replace our existing browse:social:facebook?

They execute different work loads and should be considered as different stories.

https://codereview.chromium.org/2890283002/diff/20001/tools/perf/page_sets/sy...
tools/perf/page_sets/system_health/browsing_stories.py:874: NAME =
'browse_infinite_scroll:media:flickr'
On 2017/05/19 09:40:35, perezju wrote:
> Call this just browse:media:flickr

How about browse:infinite_scroll:flickr?

https://codereview.chromium.org/2890283002/diff/20001/tools/perf/page_sets/sy...
tools/perf/page_sets/system_health/browsing_stories.py:887: NAME =
'browse_infinite_scroll:social:pinterest'
On 2017/05/19 09:40:36, perezju wrote:
> browse:social:pinterest

How about browse:infinite_scroll:pinterest?

https://codereview.chromium.org/2890283002/diff/20001/tools/perf/page_sets/sy...
tools/perf/page_sets/system_health/browsing_stories.py:893: NAME =
'browse_infinite_scroll:social:tumblr'
On 2017/05/19 09:40:36, perezju wrote:
> browse:social:tumblr

How about browse:infinite_scroll:pinterest?

perezju

https://codereview.chromium.org/2890283002/diff/20001/tools/perf/page_sets/system_health/browsing_stories.py File tools/perf/page_sets/system_health/browsing_stories.py (right): https://codereview.chromium.org/2890283002/diff/20001/tools/perf/page_sets/system_health/browsing_stories.py#newcode842 tools/perf/page_sets/system_health/browsing_stories.py:842: NAME = 'browse_infinite_scroll:forum:discourse' On 2017/05/19 12:59:00, ulan wrote: > ...

3 years, 7 months ago (2017-05-19 14:34:41 UTC) #18

https://codereview.chromium.org/2890283002/diff/20001/tools/perf/page_sets/sy...
File tools/perf/page_sets/system_health/browsing_stories.py (right):

https://codereview.chromium.org/2890283002/diff/20001/tools/perf/page_sets/sy...
tools/perf/page_sets/system_health/browsing_stories.py:842: NAME =
'browse_infinite_scroll:forum:discourse'
On 2017/05/19 12:59:00, ulan wrote:
> On 2017/05/19 09:40:35, perezju wrote:
> > I would call this: browse:social:discourse
> > And add some 'infinite_scroll' tag instead.
> 
> How about browse:infinite_scroll:discourse? I would like to keep all these
> stories in the same group and avoid confusion with other browse stories.

I think "tags" should be the correct way to group together stories in system
health. (also see my other reply below.)

https://codereview.chromium.org/2890283002/diff/20001/tools/perf/page_sets/sy...
tools/perf/page_sets/system_health/browsing_stories.py:857: NAME =
'browse_infinite_scroll:social:facebook'
On 2017/05/19 12:59:00, ulan wrote:
> On 2017/05/19 09:40:35, perezju wrote:
> > Could this replace our existing browse:social:facebook?
> 
> They execute different work loads and should be considered as different
stories.

For system health, IMO, we should pick representative websites, and then pick a
representative workflow on that website.

There might be cases where we want to provide coverage for several workflows on
the same property, but that should be rare.

The driving principle should be to test a diverse set of representative websites
on representative workflows; but without doing the full Cartesian product of
both.

In my ideal world all stories should be "browse", and the rest of the name
should be "site_category:site_name".

Tags are then used to group stories by e.g. workflow or feature; providing a
convenient way for developers e.g. to run only a set of stories; or filter-by
when looking at dashboards.

Ned, what are your thoughts here?

Hannes Payer (out of office)

On 2017/05/19 14:34:41, perezju wrote: > https://codereview.chromium.org/2890283002/diff/20001/tools/perf/page_sets/system_health/browsing_stories.py > File tools/perf/page_sets/system_health/browsing_stories.py (right): > > https://codereview.chromium.org/2890283002/diff/20001/tools/perf/page_sets/system_health/browsing_stories.py#newcode842 > ...

3 years, 7 months ago (2017-05-19 14:42:27 UTC) #19

On 2017/05/19 14:34:41, perezju wrote:
>
https://codereview.chromium.org/2890283002/diff/20001/tools/perf/page_sets/sy...
> File tools/perf/page_sets/system_health/browsing_stories.py (right):
> 
>
https://codereview.chromium.org/2890283002/diff/20001/tools/perf/page_sets/sy...
> tools/perf/page_sets/system_health/browsing_stories.py:842: NAME =
> 'browse_infinite_scroll:forum:discourse'
> On 2017/05/19 12:59:00, ulan wrote:
> > On 2017/05/19 09:40:35, perezju wrote:
> > > I would call this: browse:social:discourse
> > > And add some 'infinite_scroll' tag instead.
> > 
> > How about browse:infinite_scroll:discourse? I would like to keep all these
> > stories in the same group and avoid confusion with other browse stories.
> 
> I think "tags" should be the correct way to group together stories in system
> health. (also see my other reply below.)
> 
>
https://codereview.chromium.org/2890283002/diff/20001/tools/perf/page_sets/sy...
> tools/perf/page_sets/system_health/browsing_stories.py:857: NAME =
> 'browse_infinite_scroll:social:facebook'
> On 2017/05/19 12:59:00, ulan wrote:
> > On 2017/05/19 09:40:35, perezju wrote:
> > > Could this replace our existing browse:social:facebook?
> > 
> > They execute different work loads and should be considered as different
> stories.
> 
> For system health, IMO, we should pick representative websites, and then pick
a
> representative workflow on that website.
> 
> There might be cases where we want to provide coverage for several workflows
on
> the same property, but that should be rare.
> 
> The driving principle should be to test a diverse set of representative
websites
> on representative workflows; but without doing the full Cartesian product of
> both.
> 
> In my ideal world all stories should be "browse", and the rest of the name
> should be "site_category:site_name".
> 
> Tags are then used to group stories by e.g. workflow or feature; providing a
> convenient way for developers e.g. to run only a set of stories; or filter-by
> when looking at dashboards.
> 
> Ned, what are your thoughts here?

The selection of system health and browsing stories was the same, i.e. we picked
popular websites that triggered interesting performance patterns. We should keep
them in one category infinite_scroll because this action is particularly
interesting
for animation. Moreover, we should keep all the stories because they are all
still
useful.

nednguyen

On 2017/05/19 14:42:27, Hannes Payer wrote: > On 2017/05/19 14:34:41, perezju wrote: > > > ...

3 years, 7 months ago (2017-05-19 14:54:25 UTC) #20

On 2017/05/19 14:42:27, Hannes Payer wrote:
> On 2017/05/19 14:34:41, perezju wrote:
> >
>
https://codereview.chromium.org/2890283002/diff/20001/tools/perf/page_sets/sy...
> > File tools/perf/page_sets/system_health/browsing_stories.py (right):
> > 
> >
>
https://codereview.chromium.org/2890283002/diff/20001/tools/perf/page_sets/sy...
> > tools/perf/page_sets/system_health/browsing_stories.py:842: NAME =
> > 'browse_infinite_scroll:forum:discourse'
> > On 2017/05/19 12:59:00, ulan wrote:
> > > On 2017/05/19 09:40:35, perezju wrote:
> > > > I would call this: browse:social:discourse
> > > > And add some 'infinite_scroll' tag instead.
> > > 
> > > How about browse:infinite_scroll:discourse? I would like to keep all these
> > > stories in the same group and avoid confusion with other browse stories.
> > 
> > I think "tags" should be the correct way to group together stories in system
> > health. (also see my other reply below.)
> > 
> >
>
https://codereview.chromium.org/2890283002/diff/20001/tools/perf/page_sets/sy...
> > tools/perf/page_sets/system_health/browsing_stories.py:857: NAME =
> > 'browse_infinite_scroll:social:facebook'
> > On 2017/05/19 12:59:00, ulan wrote:
> > > On 2017/05/19 09:40:35, perezju wrote:
> > > > Could this replace our existing browse:social:facebook?
> > > 
> > > They execute different work loads and should be considered as different
> > stories.
> > 
> > For system health, IMO, we should pick representative websites, and then
pick
> a
> > representative workflow on that website.
> > 
> > There might be cases where we want to provide coverage for several workflows
> on
> > the same property, but that should be rare.
> > 
> > The driving principle should be to test a diverse set of representative
> websites
> > on representative workflows; but without doing the full Cartesian product of
> > both.
> > 
> > In my ideal world all stories should be "browse", and the rest of the name
> > should be "site_category:site_name".
> > 
> > Tags are then used to group stories by e.g. workflow or feature; providing a
> > convenient way for developers e.g. to run only a set of stories; or
filter-by
> > when looking at dashboards.
> > 
> > Ned, what are your thoughts here?
> 
> The selection of system health and browsing stories was the same, i.e. we
picked
> popular websites that triggered interesting performance patterns. We should
keep
> them in one category infinite_scroll because this action is particularly
> interesting
> for animation. Moreover, we should keep all the stories because they are all
> still
> useful.

Hanne & Ulan: the existing naming convention in browsing_stories.py is "<user
action type>:<web category>:<domain name>". It's worth noting that the dasbhoard
team is working on surfacing the story's tags as a UI feature on the dashboard,
so assuming we add "infinite_scroll" as tags to these stories, we should be able
to select the group of all browsing stories that match such tag on the dashboard
& results.html (tracking bug: 
https://github.com/catapult-project/catapult/issues/3554)

nednguyen

On 2017/05/19 14:54:25, nednguyen wrote: > On 2017/05/19 14:42:27, Hannes Payer wrote: > > On ...

3 years, 7 months ago (2017-05-19 14:56:11 UTC) #21

On 2017/05/19 14:54:25, nednguyen wrote:
> On 2017/05/19 14:42:27, Hannes Payer wrote:
> > On 2017/05/19 14:34:41, perezju wrote:
> > >
> >
>
https://codereview.chromium.org/2890283002/diff/20001/tools/perf/page_sets/sy...
> > > File tools/perf/page_sets/system_health/browsing_stories.py (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/2890283002/diff/20001/tools/perf/page_sets/sy...
> > > tools/perf/page_sets/system_health/browsing_stories.py:842: NAME =
> > > 'browse_infinite_scroll:forum:discourse'
> > > On 2017/05/19 12:59:00, ulan wrote:
> > > > On 2017/05/19 09:40:35, perezju wrote:
> > > > > I would call this: browse:social:discourse
> > > > > And add some 'infinite_scroll' tag instead.
> > > > 
> > > > How about browse:infinite_scroll:discourse? I would like to keep all
these
> > > > stories in the same group and avoid confusion with other browse stories.
> > > 
> > > I think "tags" should be the correct way to group together stories in
system
> > > health. (also see my other reply below.)
> > > 
> > >
> >
>
https://codereview.chromium.org/2890283002/diff/20001/tools/perf/page_sets/sy...
> > > tools/perf/page_sets/system_health/browsing_stories.py:857: NAME =
> > > 'browse_infinite_scroll:social:facebook'
> > > On 2017/05/19 12:59:00, ulan wrote:
> > > > On 2017/05/19 09:40:35, perezju wrote:
> > > > > Could this replace our existing browse:social:facebook?
> > > > 
> > > > They execute different work loads and should be considered as different
> > > stories.
> > > 
> > > For system health, IMO, we should pick representative websites, and then
> pick
> > a
> > > representative workflow on that website.
> > > 
> > > There might be cases where we want to provide coverage for several
workflows
> > on
> > > the same property, but that should be rare.
> > > 
> > > The driving principle should be to test a diverse set of representative
> > websites
> > > on representative workflows; but without doing the full Cartesian product
of
> > > both.
> > > 
> > > In my ideal world all stories should be "browse", and the rest of the name
> > > should be "site_category:site_name".
> > > 
> > > Tags are then used to group stories by e.g. workflow or feature; providing
a
> > > convenient way for developers e.g. to run only a set of stories; or
> filter-by
> > > when looking at dashboards.
> > > 
> > > Ned, what are your thoughts here?
> > 
> > The selection of system health and browsing stories was the same, i.e. we
> picked
> > popular websites that triggered interesting performance patterns. We should
> keep
> > them in one category infinite_scroll because this action is particularly
> > interesting
> > for animation. Moreover, we should keep all the stories because they are all
> > still
> > useful.
> 
> Hanne & Ulan: the existing naming convention in browsing_stories.py is "<user
> action type>:<web category>:<domain name>". It's worth noting that the
dasbhoard
> team is working on surfacing the story's tags as a UI feature on the
dashboard,
> so assuming we add "infinite_scroll" as tags to these stories, we should be
able
> to select the group of all browsing stories that match such tag on the
dashboard
> & results.html (tracking bug: 
> https://github.com/catapult-project/catapult/issues/3554)

On the topic of adding too many user stories, I am thinking that we can remove
some of the old loading stories in system_health & add these new infinite scroll
stories so that we don't increase too much of bot cycle times & number of
stories overall. I will write a doc so folks can easily chime in.

nednguyen

On 2017/05/19 14:56:11, nednguyen wrote: > On 2017/05/19 14:54:25, nednguyen wrote: > > On 2017/05/19 ...

3 years, 7 months ago (2017-05-23 15:14:50 UTC) #22

On 2017/05/19 14:56:11, nednguyen wrote:
> On 2017/05/19 14:54:25, nednguyen wrote:
> > On 2017/05/19 14:42:27, Hannes Payer wrote:
> > > On 2017/05/19 14:34:41, perezju wrote:
> > > >
> > >
> >
>
https://codereview.chromium.org/2890283002/diff/20001/tools/perf/page_sets/sy...
> > > > File tools/perf/page_sets/system_health/browsing_stories.py (right):
> > > > 
> > > >
> > >
> >
>
https://codereview.chromium.org/2890283002/diff/20001/tools/perf/page_sets/sy...
> > > > tools/perf/page_sets/system_health/browsing_stories.py:842: NAME =
> > > > 'browse_infinite_scroll:forum:discourse'
> > > > On 2017/05/19 12:59:00, ulan wrote:
> > > > > On 2017/05/19 09:40:35, perezju wrote:
> > > > > > I would call this: browse:social:discourse
> > > > > > And add some 'infinite_scroll' tag instead.
> > > > > 
> > > > > How about browse:infinite_scroll:discourse? I would like to keep all
> these
> > > > > stories in the same group and avoid confusion with other browse
stories.
> > > > 
> > > > I think "tags" should be the correct way to group together stories in
> system
> > > > health. (also see my other reply below.)
> > > > 
> > > >
> > >
> >
>
https://codereview.chromium.org/2890283002/diff/20001/tools/perf/page_sets/sy...
> > > > tools/perf/page_sets/system_health/browsing_stories.py:857: NAME =
> > > > 'browse_infinite_scroll:social:facebook'
> > > > On 2017/05/19 12:59:00, ulan wrote:
> > > > > On 2017/05/19 09:40:35, perezju wrote:
> > > > > > Could this replace our existing browse:social:facebook?
> > > > > 
> > > > > They execute different work loads and should be considered as
different
> > > > stories.
> > > > 
> > > > For system health, IMO, we should pick representative websites, and then
> > pick
> > > a
> > > > representative workflow on that website.
> > > > 
> > > > There might be cases where we want to provide coverage for several
> workflows
> > > on
> > > > the same property, but that should be rare.
> > > > 
> > > > The driving principle should be to test a diverse set of representative
> > > websites
> > > > on representative workflows; but without doing the full Cartesian
product
> of
> > > > both.
> > > > 
> > > > In my ideal world all stories should be "browse", and the rest of the
name
> > > > should be "site_category:site_name".
> > > > 
> > > > Tags are then used to group stories by e.g. workflow or feature;
providing
> a
> > > > convenient way for developers e.g. to run only a set of stories; or
> > filter-by
> > > > when looking at dashboards.
> > > > 
> > > > Ned, what are your thoughts here?
> > > 
> > > The selection of system health and browsing stories was the same, i.e. we
> > picked
> > > popular websites that triggered interesting performance patterns. We
should
> > keep
> > > them in one category infinite_scroll because this action is particularly
> > > interesting
> > > for animation. Moreover, we should keep all the stories because they are
all
> > > still
> > > useful.
> > 
> > Hanne & Ulan: the existing naming convention in browsing_stories.py is
"<user
> > action type>:<web category>:<domain name>". It's worth noting that the
> dasbhoard
> > team is working on surfacing the story's tags as a UI feature on the
> dashboard,
> > so assuming we add "infinite_scroll" as tags to these stories, we should be
> able
> > to select the group of all browsing stories that match such tag on the
> dashboard
> > & results.html (tracking bug: 
> > https://github.com/catapult-project/catapult/issues/3554)
> 
> On the topic of adding too many user stories, I am thinking that we can remove
> some of the old loading stories in system_health & add these new infinite
scroll
> stories so that we don't increase too much of bot cycle times & number of
> stories overall. I will write a doc so folks can easily chime in.

I whip up a doc so it's easier for us to iterate on the discussion in
https://docs.google.com/document/d/1m5QbbXfmd2P_yrdEDROCaiEzJymAd3dhHIDoX_Okk...
PTAL the doc.

nednguyen

The CQ bit was checked by nednguyen@google.com to run a CQ dry run

3 years, 7 months ago (2017-05-24 15:17:31 UTC) #23

commit-bot: I haz the power

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

3 years, 7 months ago (2017-05-24 15:18:03 UTC) #24

nednguyen

On 2017/05/23 15:14:50, nednguyen wrote: > On 2017/05/19 14:56:11, nednguyen wrote: > > On 2017/05/19 ...

3 years, 7 months ago (2017-05-24 15:30:16 UTC) #25

On 2017/05/23 15:14:50, nednguyen wrote:
> On 2017/05/19 14:56:11, nednguyen wrote:
> > On 2017/05/19 14:54:25, nednguyen wrote:
> > > On 2017/05/19 14:42:27, Hannes Payer wrote:
> > > > On 2017/05/19 14:34:41, perezju wrote:
> > > > >
> > > >
> > >
> >
>
https://codereview.chromium.org/2890283002/diff/20001/tools/perf/page_sets/sy...
> > > > > File tools/perf/page_sets/system_health/browsing_stories.py (right):
> > > > > 
> > > > >
> > > >
> > >
> >
>
https://codereview.chromium.org/2890283002/diff/20001/tools/perf/page_sets/sy...
> > > > > tools/perf/page_sets/system_health/browsing_stories.py:842: NAME =
> > > > > 'browse_infinite_scroll:forum:discourse'
> > > > > On 2017/05/19 12:59:00, ulan wrote:
> > > > > > On 2017/05/19 09:40:35, perezju wrote:
> > > > > > > I would call this: browse:social:discourse
> > > > > > > And add some 'infinite_scroll' tag instead.
> > > > > > 
> > > > > > How about browse:infinite_scroll:discourse? I would like to keep all
> > these
> > > > > > stories in the same group and avoid confusion with other browse
> stories.
> > > > > 
> > > > > I think "tags" should be the correct way to group together stories in
> > system
> > > > > health. (also see my other reply below.)
> > > > > 
> > > > >
> > > >
> > >
> >
>
https://codereview.chromium.org/2890283002/diff/20001/tools/perf/page_sets/sy...
> > > > > tools/perf/page_sets/system_health/browsing_stories.py:857: NAME =
> > > > > 'browse_infinite_scroll:social:facebook'
> > > > > On 2017/05/19 12:59:00, ulan wrote:
> > > > > > On 2017/05/19 09:40:35, perezju wrote:
> > > > > > > Could this replace our existing browse:social:facebook?
> > > > > > 
> > > > > > They execute different work loads and should be considered as
> different
> > > > > stories.
> > > > > 
> > > > > For system health, IMO, we should pick representative websites, and
then
> > > pick
> > > > a
> > > > > representative workflow on that website.
> > > > > 
> > > > > There might be cases where we want to provide coverage for several
> > workflows
> > > > on
> > > > > the same property, but that should be rare.
> > > > > 
> > > > > The driving principle should be to test a diverse set of
representative
> > > > websites
> > > > > on representative workflows; but without doing the full Cartesian
> product
> > of
> > > > > both.
> > > > > 
> > > > > In my ideal world all stories should be "browse", and the rest of the
> name
> > > > > should be "site_category:site_name".
> > > > > 
> > > > > Tags are then used to group stories by e.g. workflow or feature;
> providing
> > a
> > > > > convenient way for developers e.g. to run only a set of stories; or
> > > filter-by
> > > > > when looking at dashboards.
> > > > > 
> > > > > Ned, what are your thoughts here?
> > > > 
> > > > The selection of system health and browsing stories was the same, i.e.
we
> > > picked
> > > > popular websites that triggered interesting performance patterns. We
> should
> > > keep
> > > > them in one category infinite_scroll because this action is particularly
> > > > interesting
> > > > for animation. Moreover, we should keep all the stories because they are
> all
> > > > still
> > > > useful.
> > > 
> > > Hanne & Ulan: the existing naming convention in browsing_stories.py is
> "<user
> > > action type>:<web category>:<domain name>". It's worth noting that the
> > dasbhoard
> > > team is working on surfacing the story's tags as a UI feature on the
> > dashboard,
> > > so assuming we add "infinite_scroll" as tags to these stories, we should
be
> > able
> > > to select the group of all browsing stories that match such tag on the
> > dashboard
> > > & results.html (tracking bug: 
> > > https://github.com/catapult-project/catapult/issues/3554)
> > 
> > On the topic of adding too many user stories, I am thinking that we can
remove
> > some of the old loading stories in system_health & add these new infinite
> scroll
> > stories so that we don't increase too much of bot cycle times & number of
> > stories overall. I will write a doc so folks can easily chime in.
> 
> I whip up a doc so it's easier for us to iterate on the discussion in
>
https://docs.google.com/document/d/1m5QbbXfmd2P_yrdEDROCaiEzJymAd3dhHIDoX_Okk...
> PTAL the doc.

CL updated after discussion, PTAL

perezju

I had left several comments in patch set #1 (other than story names) that I ...

3 years, 7 months ago (2017-05-24 15:39:41 UTC) #26

commit-bot: I haz the power

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

3 years, 7 months ago (2017-05-24 17:36:35 UTC) #27

commit-bot: I haz the power

Dry run: Try jobs failed on following builders: win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_rel_ng/builds/435321)

3 years, 7 months ago (2017-05-24 17:36:36 UTC) #28

nednguyen

The CQ bit was checked by nednguyen@google.com to run a CQ dry run

3 years, 7 months ago (2017-05-24 21:08:44 UTC) #29

nednguyen

https://codereview.chromium.org/2890283002/diff/20001/tools/perf/page_sets/system_health/browsing_stories.py File tools/perf/page_sets/system_health/browsing_stories.py (right): https://codereview.chromium.org/2890283002/diff/20001/tools/perf/page_sets/system_health/browsing_stories.py#newcode798 tools/perf/page_sets/system_health/browsing_stories.py:798: def RunPageInteractions(self, action_runner): On 2017/05/19 09:40:35, perezju wrote: > ...

3 years, 7 months ago (2017-05-24 21:08:49 UTC) #30

https://codereview.chromium.org/2890283002/diff/20001/tools/perf/page_sets/sy...
File tools/perf/page_sets/system_health/browsing_stories.py (right):

https://codereview.chromium.org/2890283002/diff/20001/tools/perf/page_sets/sy...
tools/perf/page_sets/system_health/browsing_stories.py:798: def
RunPageInteractions(self, action_runner):
On 2017/05/19 09:40:35, perezju wrote:
> I don't thing we should override this method.
> 
> Could we do it by overriding _DidLoadDocument instead, like all other system
> health stories do?
> 
> My worry is that this bypasses the system health mechanism for taking
> measurements (e.g. cache flushes, measure memory but only in the corresponding
> benchmark, etc.)

Oh, it's just me copy & paste the stories from v8 benchmarks without much
thinking. This is why we have code review :-)

Done.

https://codereview.chromium.org/2890283002/diff/20001/tools/perf/page_sets/sy...
tools/perf/page_sets/system_health/browsing_stories.py:799: with
action_runner.CreateInteraction('Load'):
On 2017/05/19 09:40:36, perezju wrote:
> Do we still want to create all of these interaction records?

Nope.

https://codereview.chromium.org/2890283002/diff/20001/tools/perf/page_sets/sy...
tools/perf/page_sets/system_health/browsing_stories.py:822: remaining = distance
- action_runner.EvaluateJavaScript('window.scrollY')
On 2017/05/19 10:42:21, hjd wrote:
> I think scrollY can be overwritten in JavaScript to be (for example) a string
> although this should be rare in practice.

Acknowledged.

https://codereview.chromium.org/2890283002/diff/20001/tools/perf/page_sets/sy...
tools/perf/page_sets/system_health/browsing_stories.py:829: if remaining ==
new_remaining:
On 2017/05/19 10:42:21, hjd wrote:
> This could be remaining >= new_remaining to guarantee we're making forward
> progress. Otherwise if the page js is trying to scroll us up at the same time
> we're scrolling down we could end up stuck in a loop.

Done.

https://codereview.chromium.org/2890283002/diff/20001/tools/perf/page_sets/sy...
tools/perf/page_sets/system_health/browsing_stories.py:867: def
RunNavigateSteps(self, action_runner):
On 2017/05/19 09:40:36, perezju wrote:
> We should be overriding _Login instead.

Done.

https://codereview.chromium.org/2890283002/diff/20001/tools/perf/page_sets/sy...
tools/perf/page_sets/system_health/browsing_stories.py:889: SUPPORTED_PLATFORMS
= platforms.MOBILE_ONLY
On 2017/05/19 09:40:35, perezju wrote:
> No desktop version?

Nope

https://codereview.chromium.org/2890283002/diff/20001/tools/perf/page_sets/sy...
tools/perf/page_sets/system_health/browsing_stories.py:894: URL =
'http://techcrunch.tumblr.com/'
On 2017/05/19 10:42:21, hjd wrote:
> nit: maybe a comment that http://techcrunch.tumblr.com doesn't have https
right now, so
> later we don't think it was because of a wpr bug or something.

Done.

https://codereview.chromium.org/2890283002/diff/40001/tools/perf/page_sets/sy...
File tools/perf/page_sets/system_health/browsing_stories.py (right):

https://codereview.chromium.org/2890283002/diff/40001/tools/perf/page_sets/sy...
tools/perf/page_sets/system_health/browsing_stories.py:860: URL =
('https://meta.discourse.org/t/the-official-discourse-tags-plugin' +
On 2017/05/24 15:39:41, perezju wrote:
> nit: for urls it's fine to let them go over 80 chars (so they can be easily
> clicked or copy-pasted from code search)

Done.

https://codereview.chromium.org/2890283002/diff/40001/tools/perf/page_sets/sy...
tools/perf/page_sets/system_health/browsing_stories.py:868: URL =
('https://meta.discourse.org/t/the-official-discourse-tags-plugin' +
On 2017/05/24 15:39:41, perezju wrote:
> nit: ditto

Done.

commit-bot: I haz the power

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

3 years, 7 months ago (2017-05-24 21:09:18 UTC) #31

commit-bot: I haz the power

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

3 years, 7 months ago (2017-05-24 22:01:43 UTC) #32

commit-bot: I haz the power

Dry run: Try jobs failed on following builders: win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_rel_ng/builds/435644)

3 years, 7 months ago (2017-05-24 22:01:43 UTC) #33

nednguyen

The CQ bit was checked by nednguyen@google.com to run a CQ dry run

3 years, 7 months ago (2017-05-24 23:03:53 UTC) #34

commit-bot: I haz the power

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

3 years, 7 months ago (2017-05-24 23:04:57 UTC) #35

commit-bot: I haz the power

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

3 years, 7 months ago (2017-05-25 00:22:45 UTC) #36

commit-bot: I haz the power

Dry run: Try jobs failed on following builders: android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_swarming_rel/builds/186123)

3 years, 7 months ago (2017-05-25 00:22:46 UTC) #37

perezju

lgtm w/nits https://codereview.chromium.org/2890283002/diff/100001/tools/perf/page_sets/system_health/browsing_stories.py File tools/perf/page_sets/system_health/browsing_stories.py (right): https://codereview.chromium.org/2890283002/diff/100001/tools/perf/page_sets/system_health/browsing_stories.py#newcode852 tools/perf/page_sets/system_health/browsing_stories.py:852: URL = ('https://meta.discourse.org/t/the-official-discourse-tags-plugin-discourse-tagging/26482') nit: no need for ...

3 years, 7 months ago (2017-05-25 08:27:43 UTC) #38

nednguyen

On 2017/05/25 08:27:43, perezju wrote: > lgtm w/nits > > https://codereview.chromium.org/2890283002/diff/100001/tools/perf/page_sets/system_health/browsing_stories.py > File tools/perf/page_sets/system_health/browsing_stories.py (right): ...

3 years, 7 months ago (2017-05-25 10:11:31 UTC) #39

ulan

On 2017/05/25 10:11:31, nednguyen wrote: > On 2017/05/25 08:27:43, perezju wrote: > > lgtm w/nits ...

3 years, 6 months ago (2017-05-29 06:39:17 UTC) #40

nednguyen

The CQ bit was checked by nednguyen@google.com to run a CQ dry run

3 years, 6 months ago (2017-05-30 20:03:12 UTC) #41

commit-bot: I haz the power

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

3 years, 6 months ago (2017-05-30 20:03:56 UTC) #42

commit-bot: I haz the power

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

3 years, 6 months ago (2017-05-30 21:15:29 UTC) #43

commit-bot: I haz the power

Dry run: Try jobs failed on following builders: win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_rel_ng/builds/439151)

3 years, 6 months ago (2017-05-30 21:15:30 UTC) #44

nednguyen

On 2017/05/29 06:39:17, ulan wrote: > On 2017/05/25 10:11:31, nednguyen wrote: > > On 2017/05/25 ...

3 years, 6 months ago (2017-05-31 14:33:41 UTC) #45

nednguyen

The patchset sent to the CQ was uploaded after l-g-t-m from perezju@chromium.org, ulan@chromium.org Link to ...

3 years, 6 months ago (2017-05-31 14:42:20 UTC) #47

commit-bot: I haz the power

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

3 years, 6 months ago (2017-05-31 14:42:37 UTC) #48

commit-bot: I haz the power

CQ is committing da patch. Bot data: {"patchset_id": 140001, "attempt_start_ts": 1496241740012550, "parent_rev": "4deb94b53a551930b3db578c4c339ec0ef95bc6b", "commit_rev": "3487cdad7d1ea3c63ad671a8ac0c0d45f1fb4d46"}

3 years, 6 months ago (2017-05-31 16:30:58 UTC) #49

commit-bot: I haz the power

Description was changed from ========== Migrate infinite scrolls to system health stories BUG=724262 ========== to ...

3 years, 6 months ago (2017-05-31 16:31:08 UTC) #50

commit-bot: I haz the power

3 years, 6 months ago (2017-05-31 16:31:09 UTC) #51

Message was sent while issue was closed.

Committed patchset #7 (id:140001) as
https://chromium.googlesource.com/chromium/src/+/3487cdad7d1ea3c63ad671a8ac0c...

Issue 2890283002: Migrate infinite scrolls to system health stories (Closed)

Description

Patch Set 1 #

Patch Set 2 : Rename stories #

Patch Set 3 : Address review comments #

Patch Set 4 : update #

Patch Set 5 : Fix naming #

Patch Set 6 : Fix scrolling stuck check & disable mobile facebook scroll #

Patch Set 7 : Disable browse:social:facebook_infinite_scroll & browse:tech:discourse_infinite_scroll #

Messages