Bug 169701 – Fix unit test and bug fix for 'pull-os-versions.js' script.

• Component: New Bugs
• Status: RESOLVED FIXED

Fix unit test and bug fix for 'pull-os-versions.js' script.

Created attachment 304561 [details]
Patch

[Ryosuke Niwa]

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?

Created attachment 304609 [details]
Patch

[Ryosuke Niwa]

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().

Created attachment 304613 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 304613 [details]
Patch for landing

Clearing flags on attachment: 304613

Committed r214031: <http://trac.webkit.org/changeset/214031>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.