Telemetry should have a way to disable stories that shouldn't run on a certain platform |
||||||
Issue descriptionIn the system health benchmarks, there are several stories that shouldn't run on certain platforms (e.g. webview) because the story doesn't make sense there (e.g. browse:chrome:newtab). The expectations.config file isn't the right place to disable these because that file is for short-term disables, not permanent disables that exist for a real and functional reason. My proposal is that we add a SUPPORTED_PLATFORMS field to stories that's checked just like the existing one at the Benchmark level.
,
Jul 25
,
Jul 25
,
Jul 25
We can use page.CanRunOnBrowser() for this type of disabling. Example usage: https://cs.chromium.org/chromium/src/tools/perf/page_sets/rendering/rendering_shared_state.py?rcl=5fabb2c2ad0bcfce811c93db532d3f97d9ffef51&l=11
,
Jul 26
Looks like it's SharedPageState.CanRunOnBrowser, right? I'm trying to remember, are SharedPageStates shared across stories? Either way, It seems very weird that we'd specify what platforms a story can run on through the SharedPageState rather than the story.
,
Jul 26
#5: that's because each share state holds a collection of commonly shared states that are needed for disabling. If you want to make it less weird, you can make a default implementation SharedPageState.CanRunOnBrowser of that invokes Page.CanRunOnBrowser(..)
,
Aug 15
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/676d68809d3c70bdde04c81d9c87342fb93e38d8 commit 676d68809d3c70bdde04c81d9c87342fb93e38d8 Author: Annie Sullivan <sullivan@chromium.org> Date: Wed Aug 15 17:15:05 2018 Purposefully disable system health stories that shouldn't run on WebView. These stories depend on tabs and/or omnibox. The fact that they don't run on WebView is intentional, not a bug. So remove them from expectations and clarify that they shouldn't run on WebView in the code. Bug: 865464 , 865466 , 865471 , 867568 Change-Id: I9bbd6a4a87aaa423bdf8d106d3de2d64cf6c8d52 Reviewed-on: https://chromium-review.googlesource.com/1174700 Commit-Queue: Annie Sullivan <sullivan@chromium.org> Commit-Queue: Juan Antonio Navarro Pérez <perezju@chromium.org> Reviewed-by: Juan Antonio Navarro Pérez <perezju@chromium.org> Reviewed-by: Ned Nguyen <nednguyen@google.com> Cr-Commit-Position: refs/heads/master@{#583292} [modify] https://crrev.com/676d68809d3c70bdde04c81d9c87342fb93e38d8/tools/perf/expectations.config [modify] https://crrev.com/676d68809d3c70bdde04c81d9c87342fb93e38d8/tools/perf/page_sets/system_health/chrome_stories.py [modify] https://crrev.com/676d68809d3c70bdde04c81d9c87342fb93e38d8/tools/perf/page_sets/system_health/long_running_stories.py [modify] https://crrev.com/676d68809d3c70bdde04c81d9c87342fb93e38d8/tools/perf/page_sets/system_health/system_health_story.py
,
Aug 15
Marking Fixed as the submitted CL shows the current recommended way to do this.
,
Jan 3
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/95e86e4632c0f6dbab0a603778bf5034ae86b911 commit 95e86e4632c0f6dbab0a603778bf5034ae86b911 Author: Tom McKee <tommckee@chromium.org> Date: Thu Jan 03 19:04:39 2019 [tools/perf] Dropping some redundant expectations.config entries. The 'newtab' and 'omnibox' stories aren't run against Android_Webview because they don't make sense in that context. Since those stories don't run against Android_Webview we don't need entries in expectations.config to disable those stories on that platform. Bug: 867568 Change-Id: Ifcd38f0bc5ace5fddc0d883e24b89b06d1f84ef6 Reviewed-on: https://chromium-review.googlesource.com/c/1392734 Commit-Queue: Tom McKee <tommckee@chromium.org> Reviewed-by: Juan Antonio Navarro Pérez <perezju@chromium.org> Cr-Commit-Position: refs/heads/master@{#619706} [modify] https://crrev.com/95e86e4632c0f6dbab0a603778bf5034ae86b911/tools/perf/expectations.config
,
Jan 16
(6 days ago)
,
Jan 16
(6 days ago)
|
||||||
►
Sign in to add a comment |
||||||
Comment 1 by tdres...@chromium.org
, Jul 25