[webkit-reviews] review denied: [Bug 94255] Add AV perf layout tests to webkit flakiness dashboard : [Attachment 158911] Patch

[bugzilla-daemon at webkit.org]

Ojan Vafai <ojan at chromium.org> has denied Shadi <shadi at chromium.org>'s request
for review:
Bug 94255: Add AV perf layout tests to webkit flakiness dashboard
https://bugs.webkit.org/show_bug.cgi?id=94255

Attachment 158911: Patch
https://bugs.webkit.org/attachment.cgi?id=158911&action=review

------- Additional Comments from Ojan Vafai <ojan at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=158911&action=review


As far as the naming issue we discussed offline, I see two options:
1. Rename the builders so they say AV in the name.
2. Add a new group "@DEPS AV - chromium.org".

I have a slight preference for option 1, but option 2 is totally fine if you'd
prefer that.

My intuition is that showing a different name than the builder name would be
far too complicated as that assumption is woven throughout the dashboard code.
You're welcome to try making such a patch, but I doubt it'd be a good use of
your time. :)

> Tools/TestResultServer/static-dashboards/builders.js:59
>  var LEGACY_BUILDER_MASTERS_TO_GROUPS = {

You need to update this as well. Actually, now that I look at it, ChromiumWin,
ChromiumMac and ChromiumLinux are missing from here as well. Mind adding them
while you're modifying this?

> Tools/TestResultServer/static-dashboards/builders.js:197
> +    return builder.indexOf('Builder') == -1

Won't this exclude "Win7 BuilderTester"? Also, this line needs a semicolon at
the end.

> Tools/TestResultServer/static-dashboards/builders.js:252
> +	       requestBuilderList(LAYOUT_TESTS_BUILDER_GROUPS,
isChromiumTipOfTreeAVTestRunner, CHROMIUM_PERF_AV_BUILDER_MASTER, groupName,
BuilderGroup.TOT_WEBKIT, builderGroup);

These bots currently seem to be syncing to Chromium DEPS. Is that intended? If
so, then they should be added to the "@Deps - chromium.org" group.