Bug 197928 – Build type build request should work even there is no repository in triggerable repository group accepts patch.

• Status: NEW
• Component: Tools / Tests

Build type build request should work even there is no repository in triggerable repository group accepts patch.

Created attachment 369996 [details]
Patch

<rdar://problem/45558814>

[Ryosuke Niwa]

Comment on attachment 369996 [details]
Patch

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

> Websites/perf.webkit.org/ChangeLog:3
> +        Build type build request builds owned components should work even there is no repository in triggerable repository group accepts patch.

This title is misleading. What we want to do is to allow build request when a revision when there are no patches.
How about this: perf dashboard erroneously rejects a build request to build owned components when there are no patches

> Websites/perf.webkit.org/ChangeLog:8
> +        Fix a bug that build type build request that only builds owned components failed to pass sanity check when there is no repository accepts patch in triggerable repository group.

Please wrap this line at some point. It's too long.

> Websites/perf.webkit.org/ChangeLog:9
> +        Add a sanity check to raise an error when build request type is build but there is no repository group template.

Nit: throw an error.

> Websites/perf.webkit.org/unit-tests/buildbot-syncer-tests.js:1048
> +        it('should allow to build owned component even no repository accepts patch in the triggerable repository group', () => {

Nit: build *with a* owned component even *when/if* no repository accepts *a* patch

> Websites/perf.webkit.org/unit-tests/buildbot-syncer-tests.js:1063
> +            const owner111289 = CommitLog.ensureSingleton('111289', {'id': '111289', 'time': 1456931874000, 'repository': MockModels.ownerRepository, 'revision': 'owner-001'});
> +            const owned111222 = CommitLog.ensureSingleton('111222', {'id': '111222', 'time': 1456932774000, 'repository': MockModels.ownedRepository, 'revision': 'owned-002'});

Can we just call these owner 1 and owner 2?

> Websites/perf.webkit.org/unit-tests/buildbot-syncer-tests.js:1065
> +            const request = BuildRequest.ensureSingleton(`123123`, {'triggerable': MockModels.triggerable,

Seems like can just use regular single quotation marks here.

> Websites/perf.webkit.org/unit-tests/buildbot-syncer-tests.js:1085
> +            const owner111289 = CommitLog.ensureSingleton('111289', {'id': '111289', 'time': 1456931874000, 'repository': MockModels.ownerRepository, 'revision': 'owner-001'});
> +            const owned111222 = CommitLog.ensureSingleton('111222', {'id': '111222', 'time': 1456932774000, 'repository': MockModels.ownedRepository, 'revision': 'owned-002'});

Ditto.

> Websites/perf.webkit.org/unit-tests/buildbot-syncer-tests.js:1087
> +            const request = BuildRequest.ensureSingleton(`123123`, {'triggerable': MockModels.triggerable,

Ditto.

> Websites/perf.webkit.org/unit-tests/buildbot-syncer-tests.js:1091
> +            try {

Use assert.throws:
https://nodejs.org/api/assert.html#assert_assert_throws_fn_error_message