[webkit-reviews] review granted: [Bug 179743] Add support for fetching recent builds in Buildbot 0.9 format in BuildbotSyncer : [Attachment 332983] Updated patch with unit-tests

[bugzilla-daemon at webkit.org]

Ryosuke Niwa <rniwa at webkit.org> has granted Aakash Jain
<aakash_jain at apple.com>'s request for review:
Bug 179743: Add support for fetching recent builds in Buildbot 0.9 format in
BuildbotSyncer
https://bugs.webkit.org/show_bug.cgi?id=179743

Attachment 332983: Updated patch with unit-tests

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




--- Comment #10 from Ryosuke Niwa <rniwa at webkit.org> ---
Comment on attachment 332983
  --> https://bugs.webkit.org/attachment.cgi?id=332983
Updated patch with unit-tests

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

> Websites/perf.webkit.org/unit-tests/buildbot-syncer-tests.js:1356
> +	   it('should not fetch recent builds when count is zero', () => {

Use async "() => {" here

> Websites/perf.webkit.org/unit-tests/buildbot-syncer-tests.js:1357
> +	       let syncer = BuildbotSyncer._loadConfig(MockRemoteAPI,
sampleiOSConfig(), builderNameToIDMap())[1];

Use const.

> Websites/perf.webkit.org/unit-tests/buildbot-syncer-tests.js:1362
> +	       return promise.then((content) => {
> +		   assert.deepEqual(content, []);
> +	       });

and do:
await promise;
assert.deepEqual(content, []);

i.e.
async () => {
    const syncer = BuildbotSyncer._loadConfig(MockRemoteAPI, sampleiOSConfig(),
builderNameToIDMap())[1];
    const promise = syncer._pullRecentBuilds(0);
    assert.equal(requests.length, 0);
    await promise;
    assert.deepEqual(content, []);
}

> Websites/perf.webkit.org/unit-tests/buildbot-syncer-tests.js:1366
> +	       let syncer = BuildbotSyncer._loadConfig(MockRemoteAPI,
sampleiOSConfig(), builderNameToIDMap())[1];

Use const.

> Websites/perf.webkit.org/unit-tests/buildbot-syncer-tests.js:1374
> +	       let syncer = BuildbotSyncer._loadConfig(MockRemoteAPI,
sampleiOSConfig(), builderNameToIDMap())[1];
> +	       let promise = syncer._pullRecentBuilds(2);

Use const.

> Websites/perf.webkit.org/unit-tests/buildbot-syncer-tests.js:1380
> +	       return promise.then((content) => {
> +		   assert.deepEqual(content, []);
> +	       });

Ditto about using async await.

> Websites/perf.webkit.org/unit-tests/buildbot-syncer-tests.js:1383
> +	   it('should create BuildbotBuildEntry after fetching recent builds',
() => {

Use async

> Websites/perf.webkit.org/unit-tests/buildbot-syncer-tests.js:1385
> +	       let syncer = BuildbotSyncer._loadConfig(MockRemoteAPI,
sampleiOSConfig(), builderNameToIDMap())[1];
> +	       let promise = syncer._pullRecentBuilds(2);

use const.

> Websites/perf.webkit.org/unit-tests/buildbot-syncer-tests.js:1391
> +	       return promise.then((entries) => {
> +		   assert.deepEqual(entries.length, 2);

Use await.