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

[bugzilla-daemon at webkit.org]

Ojan Vafai <ojan at chromium.org> has granted 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 158925: Patch
https://bugs.webkit.org/attachment.cgi?id=158925&action=review

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


Looks good. Please address the last couple comments and post a new patch with
cq? marking it as requesting the commit queue. Any committer can change that to
cq+ once you've gotten an r+.

> Tools/ChangeLog:8
> +	   Additional information of the change such as approach, rationale.
Please add per-function descriptions below (OOPS!).

This needs a bit more of a detailed description. Specifically, you could
mention that you're also adding the missing mappings to
LEGACY_BUILDER_MASTERS_TO_GROUPS and that the builder name filter doesn't work
for the current builder names, but that they're getting renamed.

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

Need to change the BuilderGroup to DEPS_WEBKIT. Now that I look at it, that
argument is plumbed through a bunch of calls, but not actually used. :( I
suppose it doesn't matter, but may as well change it for consistency sake.