Bug 193563

Summary: Analyzing a chart that does not exist should not halt whole run-analysis script.
Product: WebKit Reporter: dewei_zhu
Component: New BugsAssignee: dewei_zhu
Status: RESOLVED FIXED    
Severity: Normal CC: dewei_zhu, rniwa
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch rniwa: review+

Description dewei_zhu 2019-01-17 22:00:38 PST
Analyzing a chart that does not exist should not halt whole run-analysis script.
Comment 1 dewei_zhu 2019-01-17 22:05:29 PST
Created attachment 359449 [details]
Patch
Comment 2 Ryosuke Niwa 2019-01-18 20:31:02 PST
Comment on attachment 359449 [details]
Patch

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

> Websites/perf.webkit.org/ChangeLog:17
> +        (async.set requests):
> +        (async):

Remove these?

> Websites/perf.webkit.org/tools/js/measurement-set-analyzer.js:55
> +        } catch (error) {
> +            this._logger.warn(`Skipping analysis for "${metric.fullName()}" on "${platform.name()}" as time series does not exit.`);

Please check/assert for a specific error instead of swallowing any exception.
Comment 3 dewei_zhu 2019-01-19 00:58:26 PST
Created attachment 359598 [details]
Patch
Comment 4 Ryosuke Niwa 2019-01-22 19:24:25 PST
Comment on attachment 359598 [details]
Patch

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

> Websites/perf.webkit.org/public/v3/models/measurement-set.js:86
> +        if (promise && callback) {
>              promise.then(callback, callback);
> -        else {
> +        } else if (!promise) {

It would be cleaner to nest if's as in:
if (promise) {
    if (callback)
        ~
} else {
    ~
}
Comment 5 dewei_zhu 2019-01-22 20:30:13 PST
Landed in r240319.