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: https://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 5b887ea..46fc77c 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 135e0a9..5e1938d 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 0bd0da5..705be81 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'] = {