Don't shouldn't create a request to build a patch if there is no patch to build
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 1 Jun 2017 01:55:52 +0000 (01:55 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 1 Jun 2017 01:55:52 +0000 (01:55 +0000)
https://bugs.webkit.org/show_bug.cgi?id=172791

Reviewed by Chris Dumez.

When a commit set doesn't have a patch specified, don't create a request to build. For example, when we're comparing
WebKit in the system to WebKit with a patch, there is nothing to build for the first commit set.

However, when conducting an A/B testing, it's advisible to compare WebKit built with and without a patch on a single
machine with the same version of Xcode, etc... For this reason, we still create a request to build for a commit set
if there is another commit set with a patch which uses the same repository group.

* public/privileged-api/create-test-group.php:
(main): Fixed the bug. Only create a build request to build if there is a matching repository group with a patch.
* server-tests/privileged-api-create-test-group-tests.js: Added a test case.

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

Websites/perf.webkit.org/ChangeLog
Websites/perf.webkit.org/public/privileged-api/create-test-group.php
Websites/perf.webkit.org/server-tests/privileged-api-create-test-group-tests.js

index 061f395..9dd8f23 100644 (file)
@@ -1,5 +1,23 @@
 2017-05-31  Ryosuke Niwa  <rniwa@webkit.org>
 
+        Don't shouldn't create a request to build a patch if there is no patch to build
+        https://bugs.webkit.org/show_bug.cgi?id=172791
+
+        Reviewed by Chris Dumez.
+
+        When a commit set doesn't have a patch specified, don't create a request to build. For example, when we're comparing
+        WebKit in the system to WebKit with a patch, there is nothing to build for the first commit set.
+
+        However, when conducting an A/B testing, it's advisible to compare WebKit built with and without a patch on a single
+        machine with the same version of Xcode, etc... For this reason, we still create a request to build for a commit set
+        if there is another commit set with a patch which uses the same repository group.
+
+        * public/privileged-api/create-test-group.php:
+        (main): Fixed the bug. Only create a build request to build if there is a matching repository group with a patch.
+        * server-tests/privileged-api-create-test-group-tests.js: Added a test case.
+
+2017-05-31  Ryosuke Niwa  <rniwa@webkit.org>
+
         Allow sync-buildbot.js to set a buildbot property only when patches are built
         https://bugs.webkit.org/show_bug.cgi?id=172743
 
index f92d288..4e57ad7 100644 (file)
@@ -87,23 +87,37 @@ function main()
         $task_id = $db->insert_row('analysis_tasks', 'task', array('name' => $task_name, 'author' => $author));
 
     $configuration_list = array();
-    $needs_to_build = FALSE;
+    $repository_group_with_builds = array();
     foreach ($commit_sets as $commit_list) {
         $commit_set_id = $db->insert_row('commit_sets', 'commitset', array());
+        $need_to_build = FALSE;
         foreach ($commit_list['set'] as $commit_row) {
             $commit_row['set'] = $commit_set_id;
-            $needs_to_build = $needs_to_build || $commit_row['patch_file'];
+            $need_to_build = $need_to_build || $commit_row['patch_file'];
             $db->insert_row('commit_set_items', 'commitset', $commit_row, 'commit');
         }
-        array_push($configuration_list, array('commit_set' => $commit_set_id, 'repository_group' => $commit_list['repository_group']));
+        $repository_group = $commit_list['repository_group'];
+        if ($need_to_build)
+            $repository_group_with_builds[$repository_group] = TRUE;
+        array_push($configuration_list, array('commit_set' => $commit_set_id, 'repository_group' => $repository_group));
+    }
+
+    $build_count = 0;
+    foreach ($configuration_list as &$config_item) {
+        if (array_get($repository_group_with_builds, $config_item['repository_group'])) {
+            $config_item['need_to_build'] = TRUE;
+            $build_count++;
+        }
     }
 
     $group_id = $db->insert_row('analysis_test_groups', 'testgroup',
         array('task' => $task_id, 'name' => $name, 'author' => $author));
 
-    if ($needs_to_build) {
-        $order = -count($configuration_list);
+    if ($build_count) {
+        $order = -$build_count;
         foreach ($configuration_list as $config) {
+            if (!array_get($config, 'need_to_build'))
+                continue;
             assert($order < 0);
             $db->insert_row('build_requests', 'request', array(
                 'triggerable' => $triggerable_id,
index 9833579..82eadca 100644 (file)
@@ -103,10 +103,13 @@ function addTriggerableAndCreateTask(name)
             {test: MockData.someTestId(), platform: MockData.otherPlatformId()},
         ],
         'repositoryGroups': [
-            {name: 'webkit-only', repositories: [
-                {repository: MockData.webkitRepositoryId(), acceptsPatch: true}
+            {name: 'os-only', acceptsRoot: true, repositories: [
+                {repository: MockData.macosRepositoryId(), acceptsPatch: false},
             ]},
-            {name: 'system-and-webkit', repositories: [
+            {name: 'webkit-only', acceptsRoot: true, repositories: [
+                {repository: MockData.webkitRepositoryId(), acceptsPatch: true},
+            ]},
+            {name: 'system-and-webkit', acceptsRoot: true, repositories: [
                 {repository: MockData.macosRepositoryId(), acceptsPatch: false},
                 {repository: MockData.webkitRepositoryId(), acceptsPatch: true}
             ]},
@@ -583,6 +586,70 @@ describe('/privileged-api/create-test-group', function () {
         });
     });
 
+    it('should not create a build request to build a patch when the commit set does not have a patch', () => {
+        let taskId;
+        let webkit;
+        let macos;
+        let insertedGroupId;
+        let uploadedFile;
+        return addTriggerableAndCreateTask('some task').then((id) => taskId = id).then(() => {
+            webkit = Repository.all().filter((repository) => repository.name() == 'WebKit')[0];
+            macos = Repository.all().filter((repository) => repository.name() == 'macOS')[0];
+            return TemporaryFile.makeTemporaryFile('some.dat', 'some content');
+        }).then((stream) => {
+            return PrivilegedAPI.sendRequest('upload-file', {newFile: stream}, {useFormData: true});
+        }).then((response) => {
+            const rawFile = response['uploadedFile'];
+            uploadedFile = UploadedFile.ensureSingleton(rawFile.id, rawFile);
+            const revisionSets = [{[macos.id()]: {revision: '15A284'}},
+                {[webkit.id()]: {revision: '191622', patch: uploadedFile.id()}, [macos.id()]: {revision: '15A284'}}];
+            return PrivilegedAPI.sendRequest('create-test-group', {name: 'test', task: taskId, repetitionCount: 2, revisionSets});
+        }).then((content) => {
+            insertedGroupId = content['testGroupId'];
+            return TestGroup.fetchForTask(taskId, true);
+        }).then((testGroups) => {
+            assert.equal(testGroups.length, 1);
+            const group = testGroups[0];
+            assert.equal(group.id(), insertedGroupId);
+            assert.equal(group.repetitionCount(), 2);
+            assert.equal(group.test(), Test.findById(MockData.someTestId()));
+            assert.equal(group.platform(), Platform.findById(MockData.somePlatformId()));
+            const requests = group.buildRequests();
+            assert.equal(requests.length, 5);
+
+            assert.equal(requests[0].isBuild(), true);
+            assert.equal(requests[1].isBuild(), false);
+            assert.equal(requests[2].isBuild(), false);
+            assert.equal(requests[3].isBuild(), false);
+            assert.equal(requests[4].isBuild(), false);
+
+            assert.equal(requests[0].isTest(), false);
+            assert.equal(requests[1].isTest(), true);
+            assert.equal(requests[2].isTest(), true);
+            assert.equal(requests[3].isTest(), true);
+            assert.equal(requests[4].isTest(), true);
+
+            const set0 = requests[0].commitSet();
+            const set1 = requests[1].commitSet();
+            assert.equal(requests[2].commitSet(), set0);
+            assert.equal(requests[3].commitSet(), set1);
+            assert.equal(requests[4].commitSet(), set0);
+            assert.deepEqual(Repository.sortByNamePreferringOnesWithURL(set0.repositories()), [webkit, macos]);
+            assert.deepEqual(set0.customRoots(), []);
+            assert.deepEqual(Repository.sortByNamePreferringOnesWithURL(set1.repositories()), [macos]);
+            assert.deepEqual(set1.customRoots(), []);
+            assert.equal(set0.revisionForRepository(webkit), '191622');
+            assert.equal(set1.revisionForRepository(webkit), null);
+            assert.equal(set0.patchForRepository(webkit), uploadedFile);
+            assert.equal(set0.revisionForRepository(macos), '15A284');
+            assert.equal(set0.revisionForRepository(macos), set1.revisionForRepository(macos));
+            assert.equal(set0.commitForRepository(macos), set1.commitForRepository(macos));
+            assert.equal(set0.patchForRepository(macos), null);
+            assert.equal(set1.patchForRepository(macos), null);
+            assert(!set0.equals(set1));
+        });
+    });
+
     it('should return "PatchNotAccepted" when a patch is specified for a repository that does not accept a patch', () => {
         let taskId;
         let webkit;