[webkit-reviews] review granted: [Bug 180126] Add a test health page. : [Attachment 327826] Patch

[bugzilla-daemon at webkit.org]

Alexey Proskuryakov <ap at webkit.org> has granted dewei_zhu at apple.com's request
for review:
Bug 180126: Add a test health page.
https://bugs.webkit.org/show_bug.cgi?id=180126

Attachment 327826: Patch

https://bugs.webkit.org/attachment.cgi?id=327826&action=review




--- Comment #3 from Alexey Proskuryakov <ap at webkit.org> ---
Comment on attachment 327826
  --> https://bugs.webkit.org/attachment.cgi?id=327826
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=327826&action=review

> Websites/perf.webkit.org/ChangeLog:8
> +	   Added a tet health page which shows freshness of a test.

s/tet/test/

> Websites/perf.webkit.org/public/v3/components/test-health-cell.js:42
> +	   return `/v3/#/charts?since=${Date.now() - 7 * 24 * 3600 *
1000}&paneList=((${this._platform.id()}-${this._metric.id()}))`;

Does performance dashboard code have a centralized place where URLs are built?
Doing it in every view controller seems less than ideal.

> Websites/perf.webkit.org/public/v3/pages/test-health-page.js:18
> +	   for(const config of configs) {

Missing space after "for".

> Websites/perf.webkit.org/public/v3/pages/test-health-page.js:19
> +	       const platforms =
Array.prototype.concat(...config.platformGroups.map((platformGroup) =>
platformGroup.platforms));

Another way to write this:

const platforms = config.platformGroups.reduce((result, group) =>
result.concat(group.platforms), []);

Not sure which one is easier to read.

> Websites/perf.webkit.org/public/v3/pages/test-health-page.js:28
> +	       const metrics =
Array.prototype.concat(...config.metricGroups.map((metricGroup) =>
> +		  
Array.prototype.concat(...metricGroup.subgroups.map((subgroup) =>
subgroup.metrics))));

Similar to the above, but this chain definitely appears tricky to read.

Given that you have to use an explicit iteration just below anyway, does the
map/concat chain actually improve the code?