Extend 'ifBuilt' config key to set property based on whether certain repositories...
authordewei_zhu@apple.com <dewei_zhu@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 22 Jan 2018 22:08:03 +0000 (22:08 +0000)
committerdewei_zhu@apple.com <dewei_zhu@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 22 Jan 2018 22:08:03 +0000 (22:08 +0000)
https://bugs.webkit.org/show_bug.cgi?id=181906

Reviewed by Ryosuke Niwa.

Before this change, 'ifBuilt' will always set specified property for test properties as long as there
is a build type build in the same build request group. However, this is no longer valid as we don't
want to set specified property for testing when only owned commit is built in previous build.
'ifBuilt' needs to conditionally set property based on whether certain required repositories are built.
Empty required repository list means no requirement on repository to set property.

* tools/js/buildbot-syncer.js:
(BuildbotSyncer.prototype._propertiesForBuildRequest):In the case of 'built', only set property when
repository requirment is meet and there is a 'build' root request in the same build request group.
(BuildbotSyncer._parseRepositoryGroup): Extend 'ifBuild' to pass information based on contition.
* unit-tests/buildbot-syncer-tests.js: Added unit tests.

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@227356 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Websites/perf.webkit.org/ChangeLog
Websites/perf.webkit.org/tools/js/buildbot-syncer.js
Websites/perf.webkit.org/unit-tests/buildbot-syncer-tests.js

index 5b887ea874fcb1dcb069ee5de7753d47e7bff777..46fc77cfc5944cc1078603a41f80dc0a17e85aa8 100644 (file)
@@ -1,3 +1,22 @@
+2018-01-20  Dewei Zhu  <dewei_zhu@apple.com>
+
+        Extend 'ifBuilt' config key to set property based on whether certain repositories are built or not.
+        https://bugs.webkit.org/show_bug.cgi?id=181906
+
+        Reviewed by Ryosuke Niwa.
+
+        Before this change, 'ifBuilt' will always set specified property for test properties as long as there
+        is a build type build in the same build request group. However, this is no longer valid as we don't
+        want to set specified property for testing when only owned commit is built in previous build.
+        'ifBuilt' needs to conditionally set property based on whether certain required repositories are built.
+        Empty required repository list means no requirement on repository to set property.
+
+        * tools/js/buildbot-syncer.js:
+        (BuildbotSyncer.prototype._propertiesForBuildRequest):In the case of 'built', only set property when
+        repository requirment is meet and there is a 'build' root request in the same build request group.
+        (BuildbotSyncer._parseRepositoryGroup): Extend 'ifBuild' to pass information based on contition.
+        * unit-tests/buildbot-syncer-tests.js: Added unit tests.
+
 2018-01-19  Dewei Zhu  <dewei_zhu@apple.com>
 
         Should reject updating a build request which has an associated build.
index 135e0a933b50d5b36e171a2123bb43d87d9c4668..5e1938d4060e7c85bcba2351ff6dc8c8a7fc63d3 100644 (file)
@@ -283,7 +283,9 @@ class BuildbotSyncer {
             case 'conditional':
                 switch (value.condition) {
                 case 'built':
-                    if (!requestsInGroup.some((otherRequest) => otherRequest.isBuild() && otherRequest.commitSet() == buildRequest.commitSet()))
+                    const repositoryRequirement = value.repositoryRequirement;
+                    const meetRepositoryRequirement = !repositoryRequirement.length || repositoryRequirement.some((repository) => commitSet.requiresBuildForRepository(repository));
+                    if (!meetRepositoryRequirement || !requestsInGroup.some((otherRequest) => otherRequest.isBuild() && otherRequest.commitSet() == buildRequest.commitSet()))
                         continue;
                     break;
                 case 'requiresBuild':
@@ -440,7 +442,7 @@ class BuildbotSyncer {
 
         const testRepositories = new Set;
         let specifiesRoots = false;
-        const testPropertiesTemplate = this._parseRepositoryGroupPropertyTemplate('test', name, group.testProperties, (type, value) => {
+        const testPropertiesTemplate = this._parseRepositoryGroupPropertyTemplate('test', name, group.testProperties, (type, value, condition) => {
             assert(type != 'patch', `Repository group "${name}" specifies a patch for "${value}" in the properties for testing`);
             switch (type) {
             case 'revision':
@@ -452,7 +454,8 @@ class BuildbotSyncer {
                 specifiesRoots = true;
                 return {type};
             case 'ifBuilt':
-                return {type: 'conditional', condition: 'built', value};
+                assert('condition', 'condition must set if type is "ifBuilt"');
+                return {type: 'conditional', condition: 'built', value, repositoryRequirement: condition.map(resolveRepository)};
             }
             return null;
         });
index 0bd0da51cc082ae7b9b6e657325a24f17e60959e..705be81ed9dc267e769c451a60c0b819b07be563 100644 (file)
@@ -265,7 +265,10 @@ function createSampleBuildRequestWithOwnedCommit(platform, test, order)
     const owned111222 = CommitLog.ensureSingleton('111222', {'id': '111222', 'time': 1456932774000, 'repository': MockModels.ownedRepository, 'revision': 'owned-002'});
     const ios13A452 = CommitLog.ensureSingleton('88930', {'id': '88930', 'time': 0, 'repository': MockModels.ios, 'revision': '13A452'});
 
-    const commitSet = CommitSet.ensureSingleton('53246486', {customRoots: [], revisionItems: [{commit: webkit197463}, {commit: owner111289}, {commit: owned111222, commitOwner: owner111289, requiresBuild: true}, {commit: ios13A452}]});
+    const root = new UploadedFile(456, {'createdAt': new Date('2017-05-01T21:03:27Z'), 'filename': 'root.dat', 'extension': '.dat', 'author': 'some user',
+        size: 16452234, sha256: '03eed7a8494ab8794c44b7d4308e55448fc56f4d6c175809ba968f78f656d58d'});
+
+    const commitSet = CommitSet.ensureSingleton('53246486', {customRoots: [root], revisionItems: [{commit: webkit197463}, {commit: owner111289}, {commit: owned111222, commitOwner: owner111289, requiresBuild: true}, {commit: ios13A452}]});
 
     return BuildRequest.ensureSingleton(`6345645370-${order}`, {'triggerable': MockModels.triggerable,
         repositoryGroup: MockModels.svnRepositoryWithOwnedRepositoryGroup,
@@ -1039,8 +1042,8 @@ describe('BuildbotSyncer', () => {
                     'webkit': {'revision': 'WebKit'},
                     'shared': {'revision': 'Shared'},
                     'roots': {'roots': {}},
-                    'test-custom-build': {'ifBuilt': ''},
-                    'has-built-patch': {'ifBuilt': 'true'},
+                    'test-custom-build': {'ifBuilt': [], 'value': ''},
+                    'has-built-patch': {'ifBuilt': [], 'value': 'true'},
                 },
                 'acceptsRoots': true,
             };
@@ -1069,6 +1072,64 @@ describe('BuildbotSyncer', () => {
 
         });
 
+        it('should set the value for "ifBuilt" if the repository in the list appears', () => {
+            const config = sampleiOSConfig();
+            config.repositoryGroups['ios-svn-webkit'] = {
+                'repositories': {'WebKit': {'acceptsPatch': true}, 'Shared': {}, 'iOS': {}},
+                'testProperties': {
+                    'os': {'revision': 'iOS'},
+                    'webkit': {'revision': 'WebKit'},
+                    'shared': {'revision': 'Shared'},
+                    'roots': {'roots': {}},
+                    'checkbox': {'ifBuilt': ['WebKit'], 'value': 'test-webkit'}
+                },
+                'buildProperties': {
+                    'webkit': {'revision': 'WebKit'},
+                    'webkit-patch': {'patch': 'WebKit'},
+                    'checkbox': {'ifRepositorySet': ['WebKit'], 'value': 'build-webkit'},
+                    'shared': {'revision': 'Shared'},
+                },
+                'acceptsRoots': true,
+            };
+            const syncers = BuildbotSyncer._loadConfig(RemoteAPI, config, builderNameToIDMap());
+            const requestToBuild = createSampleBuildRequestWithPatch(MockModels.iphone, null, -1);
+            const requestToTest = createSampleBuildRequestWithPatch(MockModels.iphone, MockModels.speedometer, 0);
+            const properties = syncers[0]._propertiesForBuildRequest(requestToTest, [requestToBuild, requestToTest]);
+            assert.equal(properties['webkit'], '197463');
+            assert.equal(properties['roots'], '[{"url":"http://build.webkit.org/api/uploaded-file/456.dat"}]');
+            assert.equal(properties['checkbox'], 'test-webkit');
+        });
+
+        it('should not set the value for "ifBuilt" if no build for the repository in the list appears', () => {
+            const config = sampleiOSConfig();
+            config.repositoryGroups['ios-svn-webkit-with-owned-commit'] = {
+                'repositories': {'WebKit': {'acceptsPatch': true}, 'Owner Repository': {}, 'iOS': {}},
+                'testProperties': {
+                    'os': {'revision': 'iOS'},
+                    'webkit': {'revision': 'WebKit'},
+                    'owner-repo': {'revision': 'Owner Repository'},
+                    'roots': {'roots': {}},
+                    'checkbox': {'ifBuilt': ['WebKit'], 'value': 'test-webkit'}
+                },
+                'buildProperties': {
+                    'webkit': {'revision': 'WebKit'},
+                    'webkit-patch': {'patch': 'WebKit'},
+                    'owner-repo': {'revision': 'Owner Repository'},
+                    'checkbox': {'ifRepositorySet': ['WebKit'], 'value': 'build-webkit'},
+                    'owned-commits': {'ownedRevisions': 'Owner Repository'}
+                },
+                'acceptsRoots': true,
+            };
+            const syncers = BuildbotSyncer._loadConfig(RemoteAPI, config, builderNameToIDMap());
+            const requestToBuild =  createSampleBuildRequestWithOwnedCommit(MockModels.iphone, null, -1);
+            const requestToTest = createSampleBuildRequestWithOwnedCommit(MockModels.iphone, MockModels.speedometer, 0);
+            const properties = syncers[0]._propertiesForBuildRequest(requestToTest, [requestToBuild, requestToTest]);
+
+            assert.equal(properties['webkit'], '197463');
+            assert.equal(properties['roots'], '[{"url":"http://build.webkit.org/api/uploaded-file/456.dat"}]');
+            assert.equal(properties['checkbox'], undefined);
+        });
+
         it('should resolve "ifRepositorySet" and "requiresBuild"', () => {
             const config = sampleiOSConfig();
             config.repositoryGroups['ios-svn-webkit-with-owned-commit'] = {