Bug 197928 - Build type build request should work even there is no repository in triggerable repository group accepts patch.
Summary: Build type build request should work even there is no repository in triggerab...
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: dewei_zhu
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-05-15 15:02 PDT by dewei_zhu
Modified: 2022-02-09 10:14 PST (History)
3 users (show)

See Also:


Attachments
Patch (10.36 KB, patch)
2019-05-15 15:08 PDT, dewei_zhu
rniwa: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description dewei_zhu 2019-05-15 15:02:28 PDT
Build type build request should work even there is no repository in triggerable repository group accepts patch.
Comment 1 dewei_zhu 2019-05-15 15:08:06 PDT
Created attachment 369996 [details]
Patch
Comment 2 dewei_zhu 2019-05-15 15:11:01 PDT
<rdar://problem/45558814>
Comment 3 Ryosuke Niwa 2019-05-21 15:16:00 PDT
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