Attachment 369996 Details for Bug 197928 – Patch

[patch] Patch

bug-197928-20190515150805.patch (text/plain), 10.36 KB, created by dewei_zhu on 2019-05-15 15:08:06 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: dewei_zhu

Created: 2019-05-15 15:08:06 PDT

Size: 10.36 KB

patch

obsolete

>Subversion Revision: 245344
>diff --git a/Websites/perf.webkit.org/ChangeLog b/Websites/perf.webkit.org/ChangeLog
>index 5a7a581f1493a9c6ff9ea0c3cbba6ab92957412e..ce8ea9ab2b1ea534954bcaf7daea7c05b0fb75e5 100644
>--- a/Websites/perf.webkit.org/ChangeLog
>+++ b/Websites/perf.webkit.org/ChangeLog
>@@ -1,3 +1,19 @@
>+2019-05-15  Dewei Zhu  <dewei_zhu@apple.com>
>+
>+        Build type build request builds owned components should work even there is no repository in triggerable repository group accepts patch.
>+        https://bugs.webkit.org/show_bug.cgi?id=197928
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        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.
>+        Add a sanity check to raise an error when build request type is build but there is no repository group template.
>+
>+        * tools/js/buildbot-syncer.js:
>+        (BuildbotSyncer.prototype._propertiesForBuildRequest): Changed sanity check the always requires repository accepts patch when there is a build to make it also works for build request only builds owned components.
>+        (BuildbotSyncer._parseRepositoryGroup): Added check for repository group templates not null  when build requiest type is build.
>+        * unit-tests/buildbot-syncer-tests.js: Added unit tests for this change.
>+        * unit-tests/resources/mock-v3-models.js: Added mock date for unit tests.
>+
> 2019-03-25  Dewei Zhu  <dewei_zhu@apple.com>
> 
>         Primary cluster of measurement set should always contain latest point.
>diff --git a/Websites/perf.webkit.org/tools/js/buildbot-syncer.js b/Websites/perf.webkit.org/tools/js/buildbot-syncer.js
>index 02256b1a5e6376ca1c250921865fbe643f788cab..b912728fac735e22e18c0718d0c3cd34b2ea1e39 100644
>--- a/Websites/perf.webkit.org/tools/js/buildbot-syncer.js
>+++ b/Websites/perf.webkit.org/tools/js/buildbot-syncer.js
>@@ -242,6 +242,7 @@ class BuildbotSyncer {
>             properties[propertyName] = propertiesTemplate[propertyName];
> 
>         const repositoryGroupTemplate = buildRequest.isBuild() ? repositoryGroupConfiguration.buildPropertiesTemplate : repositoryGroupConfiguration.testPropertiesTemplate;
>+        assert(!buildRequest.isBuild() || repositoryGroupTemplate, 'Repository group template cannot be null for build type build request');
>         for (let propertyName in repositoryGroupTemplate) {
>             let value = repositoryGroupTemplate[propertyName];
>             const type = typeof(value) == 'object' ? value.type : 'string';
>@@ -464,10 +465,10 @@ class BuildbotSyncer {
> 
>         let buildPropertiesTemplate = null;
>         if ('buildProperties' in group) {
>-            assert(patchAcceptingRepositoryList.size, `Repository group "${name}" specifies the properties for building but does not accept any patches`);
>             assert(group.acceptsRoots, `Repository group "${name}" specifies the properties for building but does not accept roots in testing`);
>             const revisionRepositories = new Set;
>             const patchRepositories = new Set;
>+            let hasOwnedRevisions = false;
>             buildPropertiesTemplate = this._parseRepositoryGroupPropertyTemplate('build', name, group.buildProperties, (type, value, condition) => {
>                 assert(type != 'roots', `Repository group "${name}" specifies roots in the properties for building`);
>                 let repository = null;
>@@ -482,6 +483,7 @@ class BuildbotSyncer {
>                     revisionRepositories.add(repository);
>                     return {type, repository};
>                 case 'ownedRevisions':
>+                    hasOwnedRevisions = true;
>                     return {type, ownerRepository: resolveRepository(value)};
>                 case 'ifRepositorySet':
>                     assert(condition, 'condition must set if type is "ifRepositorySet"');
>@@ -489,6 +491,7 @@ class BuildbotSyncer {
>                 }
>                 return null;
>             });
>+            assert(patchAcceptingRepositoryList.size || hasOwnedRevisions, `Repository group "${name}" specifies the properties for building but does not accept any patches or need to build owned components`);
>             for (const repository of patchRepositories)
>                 assert(revisionRepositories.has(repository), `Repository group "${name}" specifies a patch for "${repository.name()}" but does not specify a revision`);
>             assert.equal(patchAcceptingRepositoryList.size, patchRepositories.size,
>diff --git a/Websites/perf.webkit.org/unit-tests/buildbot-syncer-tests.js b/Websites/perf.webkit.org/unit-tests/buildbot-syncer-tests.js
>index 66cafc6b45cc06a98dd1cffdb4c326af4ac9ef8a..b6a7f1f098d4a7c0a8743d457dc586b6a23a087c 100644
>--- a/Websites/perf.webkit.org/unit-tests/buildbot-syncer-tests.js
>+++ b/Websites/perf.webkit.org/unit-tests/buildbot-syncer-tests.js
>@@ -1045,6 +1045,58 @@ describe('BuildbotSyncer', () => {
>             assert.deepEqual(JSON.parse(properties['owned-commits']), {'Owner Repository': [{revision: 'owned-002', repository: 'Owned Repository', ownerRevision: 'owner-001'}]});
>         });
> 
>+        it('should allow to build owned component even no repository accepts patch in the triggerable repository group', () => {
>+            const config = sampleiOSConfig();
>+            config.repositoryGroups['owner-repository'] = {
>+                'repositories': {'Owner Repository': {}},
>+                'testProperties': {
>+                    'owner-repo': {'revision': 'Owner Repository'},
>+                    'roots': {'roots': {}},
>+                },
>+                'buildProperties': {
>+                    'owned-commits': {'ownedRevisions': 'Owner Repository'}
>+                },
>+                'acceptsRoots': true,
>+            };
>+            const syncers = BuildbotSyncer._loadConfig(RemoteAPI, config, builderNameToIDMap());
>+            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'});
>+            const commitSet = CommitSet.ensureSingleton('53246486', {customRoots: [], revisionItems: [{commit: owner111289}, {commit: owned111222, commitOwner: owner111289, requiresBuild: true}]});
>+            const request = BuildRequest.ensureSingleton(`123123`, {'triggerable': MockModels.triggerable,
>+                repositoryGroup: MockModels.ownerRepositoryGroup,
>+                'commitSet': commitSet, 'status': 'pending', 'platform': MockModels.iphone, 'test': null, 'order': -1});
>+
>+            const properties = syncers[2]._propertiesForBuildRequest(request, [request]);
>+            assert.deepEqual(JSON.parse(properties['owned-commits']), {'Owner Repository': [{revision: 'owned-002', repository: 'Owned Repository', ownerRevision: 'owner-001'}]});
>+        });
>+
>+        it('should fail if build type build request does not have any build repository group template', () => {
>+            const config = sampleiOSConfig();
>+            config.repositoryGroups['owner-repository'] = {
>+                'repositories': {'Owner Repository': {}},
>+                'testProperties': {
>+                    'owner-repo': {'revision': 'Owner Repository'},
>+                    'roots': {'roots': {}},
>+                },
>+                'acceptsRoots': true,
>+            };
>+            const syncers = BuildbotSyncer._loadConfig(RemoteAPI, config, builderNameToIDMap());
>+            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'});
>+            const commitSet = CommitSet.ensureSingleton('53246486', {customRoots: [], revisionItems: [{commit: owner111289}, {commit: owned111222, commitOwner: owner111289, requiresBuild: true}]});
>+            const request = BuildRequest.ensureSingleton(`123123`, {'triggerable': MockModels.triggerable,
>+                repositoryGroup: MockModels.ownerRepositoryGroup,
>+                'commitSet': commitSet, 'status': 'pending', 'platform': MockModels.iphone, 'test': null, 'order': -1});
>+            let hasException = false;
>+            try {
>+                syncers[2]._propertiesForBuildRequest(request, [request]);
>+            } catch (error) {
>+                assert.equal(error.code, 'ERR_ASSERTION');
>+                hasException = true;
>+            }
>+            assert.ok(hasException);
>+        });
>+
>         it('should set the property for the build request id', () => {
>             const syncers = BuildbotSyncer._loadConfig(RemoteAPI, sampleiOSConfig(), builderNameToIDMap());
>             const request = createSampleBuildRequest(MockModels.iphone, MockModels.speedometer);
>diff --git a/Websites/perf.webkit.org/unit-tests/resources/mock-v3-models.js b/Websites/perf.webkit.org/unit-tests/resources/mock-v3-models.js
>index b7c39a32073f53a6c8d6362497117c1bb2ce690e..1215d562cfec86c7777099ec4cb80ee5b3812a13 100644
>--- a/Websites/perf.webkit.org/unit-tests/resources/mock-v3-models.js
>+++ b/Websites/perf.webkit.org/unit-tests/resources/mock-v3-models.js
>@@ -64,8 +64,13 @@ var MockModels = {
>                 repositories: [{repository: MockModels.ios}, {repository: MockModels.webkit, acceptsPatch: true}, {repository: MockModels.ownerRepository}],
>                 acceptsCustomRoots: true,
>             });
>+            MockModels.ownerRepositoryGroup = new TriggerableRepositoryGroup(35, {
>+                name: 'owner-repository',
>+                repositories: [{repository: MockModels.ownerRepository}],
>+                acceptsCustomRoots: true
>+            });
>             MockModels.triggerable = new Triggerable(3, {name: 'build-webkit',
>-                repositoryGroups: [MockModels.osRepositoryGroup, MockModels.svnRepositoryGroup, MockModels.gitRepositoryGroup, MockModels.svnRepositoryWithOwnedRepositoryGroup],
>+                repositoryGroups: [MockModels.osRepositoryGroup, MockModels.svnRepositoryGroup, MockModels.gitRepositoryGroup, MockModels.svnRepositoryWithOwnedRepositoryGroup, MockModels.ownerRepositoryGroup],
>                 configurations: [{test: MockModels.iPhonePLT, platform: MockModels.iphone}]});
> 
>         });

Flags:

rniwa: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 197928: 369996