Add Polymer-Topeka to Key Silk Cases

Add Polymer-Topeka to Key Silk Cases

In the future, we might also want a version of polymer that uses natural animations.

Committed: https://crrev.com/c9abdd09367636244a2444b754fcf77881aee446
Cr-Commit-Position: refs/heads/master@{#320165}

[benjhayden, 2015-03-10 18:52:04]

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

[nednguyen, 2015-03-10 18:54:43]

https://codereview.chromium.org/998693002/diff/1/tools/perf/page_sets/key_sil...
File tools/perf/page_sets/key_silk_cases.py (right):

https://codereview.chromium.org/998693002/diff/1/tools/perf/page_sets/key_sil...
tools/perf/page_sets/key_silk_cases.py:654: """ Why: TODO """
Please fill the TODO.

https://codereview.chromium.org/998693002/diff/1/tools/perf/page_sets/key_sil...
tools/perf/page_sets/key_silk_cases.py:674: is_smooth=True)
FYI, you don't need the is_smooth flag anymore.

https://codereview.chromium.org/998693002/diff/1/tools/perf/page_sets/key_sil...
tools/perf/page_sets/key_silk_cases.py:679: action_runner.Wait(2)
I don't like hardcoded waiting time. What exactly are we waiting for here?

[benjhayden, 2015-03-10 22:47:49]

Patchset #3 (id:40001) has been deleted

[benjhayden, 2015-03-10 22:47:55]

Patchset #2 (id:20001) has been deleted

[benjhayden, 2015-03-10 22:50:01]

https://codereview.chromium.org/998693002/diff/1/tools/perf/page_sets/key_sil...
File tools/perf/page_sets/key_silk_cases.py (right):

https://codereview.chromium.org/998693002/diff/1/tools/perf/page_sets/key_sil...
tools/perf/page_sets/key_silk_cases.py:654: """ Why: TODO """
On 2015/03/10 at 18:54:43, nednguyen wrote:
> Please fill the TODO.

Done.

https://codereview.chromium.org/998693002/diff/1/tools/perf/page_sets/key_sil...
tools/perf/page_sets/key_silk_cases.py:674: is_smooth=True)
On 2015/03/10 at 18:54:43, nednguyen wrote:
> FYI, you don't need the is_smooth flag anymore.

Done.

https://codereview.chromium.org/998693002/diff/1/tools/perf/page_sets/key_sil...
tools/perf/page_sets/key_silk_cases.py:679: action_runner.Wait(2)
On 2015/03/10 at 18:54:43, nednguyen wrote:
> I don't like hardcoded waiting time. What exactly are we waiting for here?

Done.

[nednguyen, 2015-03-10 23:01:58]

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

[nednguyen, 2015-03-10 23:02:36]

On 2015/03/10 23:01:58, nednguyen wrote:

Lemme make a quick hack so you we can try out this benchmark on all CQ bots
before we land this.

[nednguyen, 2015-03-10 23:27:09]

On 2015/03/10 23:02:36, nednguyen wrote:
> On 2015/03/10 23:01:58, nednguyen wrote:
> 
> Lemme make a quick hack so you we can try out this benchmark on all CQ bots
> before we land this.


https://codereview.chromium.org/999583002 is the patch with the "hack". Let's
wait for telemetry_perf_unittest to pass on it.

[nednguyen, 2015-03-11 16:36:17]

On 2015/03/10 23:27:09, nednguyen wrote:
> On 2015/03/10 23:02:36, nednguyen wrote:
> > On 2015/03/10 23:01:58, nednguyen wrote:
> > 
> > Lemme make a quick hack so you we can try out this benchmark on all CQ bots
> > before we land this.
> 
> 
> https://codereview.chromium.org/999583002 is the patch with the "hack". Let's
> wait for telemetry_perf_unittest to pass on it.

All the telemetry test passed on the CQ. However, I realize that the
key_silk_cases is only mobile. When I apply that patch and run
"./tools/perf/run_tests benchmark --browser=android-chrome-beta", I get this
error:

  Traceback (most recent call last):
    Run at tools/telemetry/telemetry/benchmark.py:202
      max_failures=self._max_failures)
    Run at tools/telemetry/telemetry/user_story/user_story_runner.py:208
      user_stories):
    _UpdateAndCheckArchives at
tools/telemetry/telemetry/user_story/user_story_runner.py:297
      wpr_archive_info.DownloadArchivesIfNeeded()
    DownloadArchivesIfNeeded at tools/telemetry/telemetry/wpr/archive_info.py:85
      cloud_storage.GetIfChanged(archive_path, self._bucket)
    GetIfChanged at tools/telemetry/telemetry/util/cloud_storage.py:245
      Get(bucket, expected_hash, file_path)
    Get at tools/telemetry/telemetry/util/cloud_storage.py:192
      _RunCommand(['cp', url, local_path])
    _RunCommand at tools/telemetry/telemetry/util/cloud_storage.py:152
      raise NotFoundError(stderr)
  NotFoundError: CommandException: No URLs matched:
gs://chrome-partner-telemetry/8918b8bd8d969d6da52f8c28cd53c4b0755cd699


I think you uploaded the wpr file to the wrong bucket. You need to reupload it
to --bucket=chrome-partner-telemetry, which is the one used by key_silk_cases
pageset.

[nednguyen, 2015-03-11 16:36:28]

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

[nednguyen, 2015-03-11 16:38:52]

On 2015/03/11 16:36:29, nednguyen wrote:

+Kari, our PRESUBMIT fails to check this case because Ben did upload the wpr
file, but to the wrong bucket. I think the archive_data_file key_silk_cases.json
should be the place that contain the info of wpr bucket rather than the
key_silk_cases.py page set. For example, what would happen if the two page set
use a same archive_data_file but they specify the bucket differently?

[aiolos (Not reviewing), 2015-03-11 17:52:10]

On 2015/03/11 16:38:52, nednguyen wrote:
> On 2015/03/11 16:36:29, nednguyen wrote:
> 
> +Kari, our PRESUBMIT fails to check this case because Ben did upload the wpr
> file, but to the wrong bucket. I think the archive_data_file
key_silk_cases.json
> should be the place that contain the info of wpr bucket rather than the
> key_silk_cases.py page set. For example, what would happen if the two page set
> use a same archive_data_file but they specify the bucket differently?

While you can technically do it, telemetry isn't designed with multiple page
sets using the same archive_data_file in mind. The archive_data_file is just a
path that is written to based on information in the page set and the recordings
done of said page set, and it's contents should only be edited by telemetry when
you record a new wpr file. The behavior for the case of two page sets using the
same archive_data_file is undefined since we don't really support it. But if you
had two page sets with different buckets and the same archive_data_file, at
least one of them would throw errors while you were trying to run them since you
wouldn't find the correct files in cloud_storage. I could change that to error
when someone specified the archive_data_file of a page set with a different
permission than expected, but it would require adding a read to disk every time
we instantiated a page set that uses wpr files.

We should keep the bucket in user_story. And it's also used for downloading from
the serving dirs as well as wpr files. We could, however, easily write it out to
the archive_data_file while we're recording it.

I've also been thinking that we should add a --no-upload flag to record_wpr, and
add a prompt for whether the user wants to upload while they are recording. This
would cut down on people still uploading manually and accidentally putting it in
the wrong bucket. And still support our users who asked for the ability to avoid
a prompt due to their automation.

[nednguyen, 2015-03-11 17:58:15]

On 2015/03/11 17:52:10, aiolos wrote:
> On 2015/03/11 16:38:52, nednguyen wrote:
> > On 2015/03/11 16:36:29, nednguyen wrote:
> > 
> > +Kari, our PRESUBMIT fails to check this case because Ben did upload the wpr
> > file, but to the wrong bucket. I think the archive_data_file
> key_silk_cases.json
> > should be the place that contain the info of wpr bucket rather than the
> > key_silk_cases.py page set. For example, what would happen if the two page
set
> > use a same archive_data_file but they specify the bucket differently?
> 
> While you can technically do it, telemetry isn't designed with multiple page
> sets using the same archive_data_file in mind. The archive_data_file is just a
> path that is written to based on information in the page set and the
recordings
> done of said page set, and it's contents should only be edited by telemetry
when
> you record a new wpr file. The behavior for the case of two page sets using
the
> same archive_data_file is undefined since we don't really support it. But if
you
> had two page sets with different buckets and the same archive_data_file, at
> least one of them would throw errors while you were trying to run them since
you
> wouldn't find the correct files in cloud_storage. I could change that to error
> when someone specified the archive_data_file of a page set with a different
> permission than expected, but it would require adding a read to disk every
time
> we instantiated a page set that uses wpr files.

Reporting error as early as possible make it's easier to figure out what goes
wrong.
I don't think write to disk is a concern. Telemetry already does a lot of disk
I/O already.

> 
> We should keep the bucket in user_story. And it's also used for downloading
from
> the serving dirs as well as wpr files. We could, however, easily write it out
to
> the archive_data_file while we're recording it.

Write the bucket out to the archive_data_file while we're recording SGTM. 

> 
> I've also been thinking that we should add a --no-upload flag to record_wpr,
and
> add a prompt for whether the user wants to upload while they are recording.
This
> would cut down on people still uploading manually and accidentally putting it
in
> the wrong bucket. And still support our users who asked for the ability to
avoid
> a prompt due to their automation.

[benjhayden, 2015-03-11 18:01:02]

On 2015/03/11 at 16:36:17, nednguyen wrote:
> On 2015/03/10 23:27:09, nednguyen wrote:
> > On 2015/03/10 23:02:36, nednguyen wrote:
> > > On 2015/03/10 23:01:58, nednguyen wrote:
> > > 
> > > Lemme make a quick hack so you we can try out this benchmark on all CQ
bots
> > > before we land this.
> > 
> > 
> > https://codereview.chromium.org/999583002 is the patch with the "hack".
Let's
> > wait for telemetry_perf_unittest to pass on it.
> 
> All the telemetry test passed on the CQ. However, I realize that the
key_silk_cases is only mobile. When I apply that patch and run
"./tools/perf/run_tests benchmark --browser=android-chrome-beta", I get this
error:
> 
>   Traceback (most recent call last):
>     Run at tools/telemetry/telemetry/benchmark.py:202
>       max_failures=self._max_failures)
>     Run at tools/telemetry/telemetry/user_story/user_story_runner.py:208
>       user_stories):
>     _UpdateAndCheckArchives at
tools/telemetry/telemetry/user_story/user_story_runner.py:297
>       wpr_archive_info.DownloadArchivesIfNeeded()
>     DownloadArchivesIfNeeded at
tools/telemetry/telemetry/wpr/archive_info.py:85
>       cloud_storage.GetIfChanged(archive_path, self._bucket)
>     GetIfChanged at tools/telemetry/telemetry/util/cloud_storage.py:245
>       Get(bucket, expected_hash, file_path)
>     Get at tools/telemetry/telemetry/util/cloud_storage.py:192
>       _RunCommand(['cp', url, local_path])
>     _RunCommand at tools/telemetry/telemetry/util/cloud_storage.py:152
>       raise NotFoundError(stderr)
>   NotFoundError: CommandException: No URLs matched:
gs://chrome-partner-telemetry/8918b8bd8d969d6da52f8c28cd53c4b0755cd699
> 
> 
> I think you uploaded the wpr file to the wrong bucket. You need to reupload it
to --bucket=chrome-partner-telemetry, which is the one used by key_silk_cases
pageset.

Done.

[nednguyen, 2015-03-11 20:42:57]

lgtm

[benjhayden, 2015-03-11 21:10:02]

The CQ bit was checked by benjhayden@chromium.org

[commit-bot: I haz the power, 2015-03-11 21:10:53]

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

[commit-bot: I haz the power, 2015-03-11 22:19:20]

Committed patchset #2 (id:60001)

[commit-bot: I haz the power, 2015-03-11 22:20:01]

Patchset 2 (id:??) landed as
https://crrev.com/c9abdd09367636244a2444b754fcf77881aee446
Cr-Commit-Position: refs/heads/master@{#320165}