Bug 169701 - Fix unit test and bug fix for 'pull-os-versions.js' script.
Summary: Fix unit test and bug fix for 'pull-os-versions.js' script.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-03-15 15:27 PDT by dewei_zhu
Modified: 2017-03-15 23:29 PDT (History)
3 users (show)

See Also:


Attachments
Patch (4.62 KB, patch)
2017-03-15 15:33 PDT, dewei_zhu
no flags Details | Formatted Diff | Diff
Patch (4.63 KB, patch)
2017-03-15 22:06 PDT, dewei_zhu
no flags Details | Formatted Diff | Diff
Patch for landing (5.15 KB, patch)
2017-03-15 22:48 PDT, dewei_zhu
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description dewei_zhu 2017-03-15 15:27:06 PDT
Fix unit test and bug fix for 'pull-os-versions.js' script.
Comment 1 dewei_zhu 2017-03-15 15:33:34 PDT
Created attachment 304561 [details]
Patch
Comment 2 Ryosuke Niwa 2017-03-15 21:24:14 PDT
Comment on attachment 304561 [details]
Patch

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

> Websites/perf.webkit.org/ChangeLog:9
> +        Fix 'pull-os-versions.js' does not fetch new builds and report.

This sentence has two verbs.
Fix the bug that 'pull-os-versions.js' does not fetch new builds and report?

> Websites/perf.webkit.org/tools/pull-os-versions.js:34
> +        return fetchers.reduce((promise, fetcher) => {
> +            return promise.then(fetcher.fetchAndReportNewBuilds());
> +        }, Promise.resolve());

Are you sure you really meant to call chain the promise returned by fetcher.fetchAndReportNewBuilds()?
If you want to call each promise in sentience, you need to do:
(promise, fetcher) => {
    return promise.then(() => fetcher.fetchAndReportNewBuilds());
}

Perhaps you want to create a new helper function that sequentialize promises?
Comment 3 dewei_zhu 2017-03-15 22:06:46 PDT
Created attachment 304609 [details]
Patch
Comment 4 Ryosuke Niwa 2017-03-15 22:21:06 PDT
Comment on attachment 304609 [details]
Patch

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

> Websites/perf.webkit.org/tools/pull-os-versions.js:33
> +            return promise.then(() => {return fetcher.fetchAndReportNewBuilds();});

I think you can just do () => fetcher.fetchAndReportNewBuilds().
Comment 5 dewei_zhu 2017-03-15 22:48:58 PDT
Created attachment 304613 [details]
Patch for landing
Comment 6 WebKit Commit Bot 2017-03-15 23:29:06 PDT
Comment on attachment 304613 [details]
Patch for landing

Clearing flags on attachment: 304613

Committed r214031: <http://trac.webkit.org/changeset/214031>
Comment 7 WebKit Commit Bot 2017-03-15 23:29:11 PDT
All reviewed patches have been landed.  Closing bug.