New issue
Advanced search Search tips

Issue 799988 link

Starred by 1 user
Status: Assigned
Owner:
charliea@chromium.org
Cc:
perezju@chromium.org
nedngu...@google.com
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Feature



Sign in to add a comment

Telemetry should have a presubmit that blocks entries in the expectations file that lack a crbug

Project Member Reported by charliea@google.com, Jan 8 2018 
We've seen a few times where folks try to disable a failing story without linking to the bug pertaining to why the story's being disabled. Technically, the format allows this (and therefore the parser does as well), but it probably makes sense to add a presubmit that blocks these entries.

Before doing this, we need to decide what to do with the existing entries where there's no reason given now (https://cs.chromium.org/search/?q=f:tools/perf/expectations.config+%5E%5C%5B&sq=package:chromium&type=cs). There are only 10 or so of these entries at the moment. It looks like some of them should probably be migrated to CanRunBenchmark (chrome:newtab not running on Webview, for example, which accounts for 3/10). My proposal is that we do the "right thing" (find the corresponding bug, migrate to CanRunBenchmark) for each of these given the relatively low number of them.

Comment 1 by charliea@google.com, Jan 8 2018 

On second thought, with the "Android Webview" chrome:newtab problem detailed above, I made a few mistakes.


First, CanRunBenchmark operates at the benchmark level, whereas we only want to disable a particular story, so it's not really applicable. 

Second, CanRunBenchmark isn't how we do things anymore - we use the SUPPORTED_PLATFORMS class field instead.

Third, if we wanted to use a similar concept as SUPPORTED_PLATFORMS for a Story, no such concept really exists. Perhaps we could add a SUPPORTED_PLATFORMS at the story level? I'm not sure.

Anyhow, my point is that my previous comment about just doing the "right" think for each of these 10 bugless disables doesn't really make sense. For at least some of these, Telemetry needs some work so that there even is a "right" thing to do.

Comment 2 by charliea@google.com, Jan 8 2018

Cc: -nednguyen@chromium.org nedngu...@google.com

Comment 3 by nedngu...@google.com, Jan 8 2018

Labels: Hotlist-GoodFirstBug

Comment 4 by perezju@chromium.org, Jan 9 2018 

> Telemetry needs some work so that there even is a "right" thing to do.

Yes, that's the right thing to do. :D

We talked about that a lot, but nothing ever got implemented.

I suggest:
- file a new bug on implementing a proper way for stories to specify whether they should be "skipped" because they do not apply to a particular browser or platform (pretty much any arbitrary code should be allowed there).
- For now, give that bug as the reason why those stories are disabled on the expectations file.
- Implement then the check so no new reason-less disables are added to the expectations file.
►

Sign in to add a comment