create-test-group should allow a different set of repositories to be used in each...
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 23 Mar 2017 20:54:39 +0000 (20:54 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 23 Mar 2017 20:54:39 +0000 (20:54 +0000)
https://bugs.webkit.org/show_bug.cgi?id=169992

Rubber-stamped by Antti Koivisto.

Added the support for new POST parameter, revisionSets, to /privileged-api/create-test-group.
This new parameter now specifies an array of repository id to revision dictionaries, and allows
different set of repositories' revisions to be specified in each dictionary.

We keep the old API for v2 UI and detect-changes.js compatibility for now.

* public/privileged-api/create-test-group.php:
(main):
(commit_sets_from_revision_sets): Added.
(ensure_commit_sets): Only fetch the top-level repository per r213788 and r213976.

* public/v3/models/test-group.js:
(TestGroup.createAndRefetchTestGroups): Use the newly added revisionSets parameter instead of
the now depreacted commitSets parameter.

* public/v3/pages/analysis-task-page.js:
(AnalysisTaskPage.prototype._createTestGroupAfterVerifyingCommitSetList): Simplified this code
by simply verifying the consistency of commit sets now that createAndRefetchTestGroups takes
an array of commit sets instead of a dictionary of repository name to a list of revisions.

* server-tests/privileged-api-create-test-group-tests.js: Added test cases for new parameter.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@214314 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/public/v3/models/test-group.js
Websites/perf.webkit.org/public/v3/pages/analysis-task-page.js
Websites/perf.webkit.org/server-tests/privileged-api-create-test-group-tests.js

index 5d36862..b66ad3e 100644 (file)
@@ -1,3 +1,32 @@
+2017-03-23  Ryosuke Niwa  <rniwa@webkit.org>
+
+        create-test-group should allow a different set of repositories to be used in each configuration
+        https://bugs.webkit.org/show_bug.cgi?id=169992
+
+        Rubber-stamped by Antti Koivisto.
+
+        Added the support for new POST parameter, revisionSets, to /privileged-api/create-test-group.
+        This new parameter now specifies an array of repository id to revision dictionaries, and allows
+        different set of repositories' revisions to be specified in each dictionary.
+
+        We keep the old API for v2 UI and detect-changes.js compatibility for now.
+
+        * public/privileged-api/create-test-group.php:
+        (main):
+        (commit_sets_from_revision_sets): Added.
+        (ensure_commit_sets): Only fetch the top-level repository per r213788 and r213976.
+
+        * public/v3/models/test-group.js:
+        (TestGroup.createAndRefetchTestGroups): Use the newly added revisionSets parameter instead of
+        the now depreacted commitSets parameter.
+
+        * public/v3/pages/analysis-task-page.js:
+        (AnalysisTaskPage.prototype._createTestGroupAfterVerifyingCommitSetList): Simplified this code
+        by simply verifying the consistency of commit sets now that createAndRefetchTestGroups takes
+        an array of commit sets instead of a dictionary of repository name to a list of revisions.
+
+        * server-tests/privileged-api-create-test-group-tests.js: Added test cases for new parameter.
+
 2017-03-22  Ryosuke Niwa  <rniwa@webkit.org>
 
         /api/uploaded-file should return createdAt as a POSIX timestamp
index f68fc2e..56289a5 100644 (file)
@@ -15,10 +15,11 @@ function main() {
     $name = $arguments['name'];
     $task_id = $arguments['task'];
     $repetition_count = $arguments['repetitionCount'];
+    $revision_set_list = array_get($data, 'revisionSets');
     $commit_sets_info = array_get($data, 'commitSets');
 
     require_format('Task', $task_id, '/^\d+$/');
-    if (!$commit_sets_info)
+    if (!$revision_set_list && !$commit_sets_info)
         exit_with_error('InvalidCommitSets');
 
     if ($repetition_count === null)
@@ -33,7 +34,10 @@ function main() {
     if (!$triggerable)
         exit_with_error('TriggerableNotFoundForTask', array('task' => $task_id));
 
-    $commit_sets = ensure_commit_sets($db, $commit_sets_info);
+    if ($revision_set_list)
+        $commit_sets = commit_sets_from_revision_sets($db, $revision_set_list);
+    else // V2 UI compatibility
+        $commit_sets = ensure_commit_sets($db, $commit_sets_info);
 
     $db->begin_transaction();
 
@@ -67,9 +71,36 @@ function main() {
     exit_with_success(array('testGroupId' => $group_id));
 }
 
+function commit_sets_from_revision_sets($db, $revision_set_list)
+{
+    if (count($revision_set_list) < 2)
+        exit_with_error('InvalidRevisionSets', array('revisionSets' => $revision_set_list));
+
+    $commit_set_list = array();
+    foreach ($revision_set_list as $revision_set) {
+        $commit_set = array();
+
+        if (!count($revision_set))
+            exit_with_error('InvalidRevisionSets', array('revisionSets' => $revision_set_list));
+
+        foreach ($revision_set as $repository_id => $revision) {
+            if (!is_numeric($repository_id))
+                exit_with_error('InvalidRepository', array('repository' => $repository_id));
+            $commit = $db->select_first_row('commits', 'commit',
+                array('repository' => intval($repository_id), 'revision' => $revision));
+            if (!$commit)
+                exit_with_error('RevisionNotFound', array('repository' => $repository_id, 'revision' => $revision));
+            array_push($commit_set, $commit['commit_id']);
+        }
+        array_push($commit_set_list, $commit_set);
+    }
+
+    return $commit_set_list;
+}
+
 function ensure_commit_sets($db, $commit_sets_info) {
     $repository_name_to_id = array();
-    foreach ($db->fetch_table('repositories') as $row)
+    foreach ($db->select_rows('repositories', 'repository', array('owner' => NULL)) as $row)
         $repository_name_to_id[$row['repository_name']] = $row['repository_id'];
 
     $commit_sets = array();
index 630d1d2..c4c9446 100644 (file)
@@ -199,11 +199,21 @@ class TestGroup extends LabeledObject {
 
     static createAndRefetchTestGroups(task, name, repetitionCount, commitSets)
     {
+        console.assert(commitSets.length == 2);
+
+        const revisionSets = commitSets.map((commitSet) => {
+            console.assert(commitSet instanceof CustomCommitSet || commitSet instanceof CommitSet);
+            const revisionSet = {};
+            for (let repository of commitSet.repositories())
+                revisionSet[repository.id()] = commitSet.revisionForRepository(repository);
+            return revisionSet;
+        });
+
         return PrivilegedAPI.sendRequest('create-test-group', {
             task: task.id(),
             name: name,
             repetitionCount: repetitionCount,
-            commitSets: commitSets,
+            revisionSets: revisionSets,
         }).then((data) => {
             return this.cachedFetch('/api/test-groups', {task: task.id()}, true).then((data) => this._createModelsFromFetchedTestGroups(data));
         });
index ff5761a..00ec5ec 100644 (file)
@@ -546,40 +546,31 @@ class AnalysisTaskPage extends PageWithHeading {
 
     _createTestGroupAfterVerifyingCommitSetList(testGroupName, repetitionCount, commitSetMap)
     {
-        if (this._hasDuplicateTestGroupName(testGroupName))
+        if (this._hasDuplicateTestGroupName(testGroupName)) {
             alert(`There is already a test group named "${testGroupName}"`);
-
-        const commitSetsByName = {};
-        let firstLabel;
-        for (firstLabel in commitSetMap) {
-            const commitSet = commitSetMap[firstLabel];
-            for (let repository of commitSet.repositories())
-                commitSetsByName[repository.name()] = [];
-            break;
+            return;
         }
 
-        let setIndex = 0;
-        for (let label in commitSetMap) {
-            const commitSet = commitSetMap[label];
+        const firstLabel = Object.keys(commitSetMap)[0];
+        const firstCommitSet = commitSetMap[firstLabel];
+
+        for (let currentLabel in commitSetMap) {
+            const commitSet = commitSetMap[currentLabel];
             for (let repository of commitSet.repositories()) {
-                const list = commitSetsByName[repository.name()];
-                if (!list) {
-                    alert(`Set ${label} specifies ${repository.label()} but set ${firstLabel} does not.`);
-                    return null;
-                }
-                list.push(commitSet.revisionForRepository(repository));
-            }
-            setIndex++;
-            for (let name in commitSetsByName) {
-                const list = commitSetsByName[name];
-                if (list.length < setIndex) {
-                    alert(`Set ${firstLabel} specifies ${name} but set ${label} does not.`);
-                    return null;
-                }
+                if (!firstCommitSet.revisionForRepository(repository))
+                    return alert(`Set ${currentLabel} specifies ${repository.label()} but set ${firstLabel} does not.`);
+            }
+            for (let repository of firstCommitSet.repositories()) {
+                if (!commitSet.revisionForRepository(repository))
+                    return alert(`Set ${firstLabel} specifies ${repository.label()} but set ${currentLabel} does not.`);
             }
         }
 
-        TestGroup.createAndRefetchTestGroups(this._task, testGroupName, repetitionCount, commitSetsByName)
+        const commitSets = [];
+        for (let label in commitSetMap)
+            commitSets.push(commitSetMap[label]);
+
+        TestGroup.createAndRefetchTestGroups(this._task, testGroupName, repetitionCount, commitSets)
             .then(this._didFetchTestGroups.bind(this), function (error) {
             alert('Failed to create a new test group: ' + error);
         });
index 2c3e826..41486f3 100644 (file)
@@ -205,6 +205,28 @@ describe('/privileged-api/create-test-group', function () {
         });
     });
 
+    it('should return "InvalidRevisionSets" when a revision set is empty', () => {
+        return addTriggerableAndCreateTask('some task').then((taskId) => {
+            const webkit = Repository.all().find((repository) => repository.name() == 'WebKit');
+            return PrivilegedAPI.sendRequest('create-test-group', {name: 'test', task: taskId, revisionSets: [{[webkit.id()]: '191622'}, {}]}).then((content) => {
+                assert(false, 'should never be reached');
+            }, (error) => {
+                assert.equal(error, 'InvalidRevisionSets');
+            });
+        });
+    });
+
+    it('should return "InvalidRevisionSets" when the number of revision sets is less than two', () => {
+        return addTriggerableAndCreateTask('some task').then((taskId) => {
+            const webkit = Repository.all().find((repository) => repository.name() == 'WebKit');
+            return PrivilegedAPI.sendRequest('create-test-group', {name: 'test', task: taskId, revisionSets: [{[webkit.id()]: '191622'}]}).then((content) => {
+                assert(false, 'should never be reached');
+            }, (error) => {
+                assert.equal(error, 'InvalidRevisionSets');
+            });
+        });
+    });
+
     it('should return "RepositoryNotFound" when commit sets contains an invalid repository', () => {
         return addTriggerableAndCreateTask('some task').then((taskId) => {
             return PrivilegedAPI.sendRequest('create-test-group', {name: 'test', task: taskId, commitSets: {'Foo': []}}).then((content) => {
@@ -225,6 +247,27 @@ describe('/privileged-api/create-test-group', function () {
         });
     });
 
+    it('should return "RevisionNotFound" when revision sets contains an invalid revision', () => {
+        return addTriggerableAndCreateTask('some task').then((taskId) => {
+            const webkit = Repository.all().find((repository) => repository.name() == 'WebKit');
+            return PrivilegedAPI.sendRequest('create-test-group', {name: 'test', task: taskId, revisionSets: [{[webkit.id()]: '191622'}, {[webkit.id()]: '1'}]}).then((content) => {
+                assert(false, 'should never be reached');
+            }, (error) => {
+                assert.equal(error, 'RevisionNotFound');
+            });
+        });
+    });
+
+    it('should return "InvalidRepository" when a revision set uses a repository name instead of a repository id', () => {
+        return addTriggerableAndCreateTask('some task').then((taskId) => {
+            return PrivilegedAPI.sendRequest('create-test-group', {name: 'test', task: taskId, revisionSets: [{'WebKit': '191622'}, {}]}).then((content) => {
+                assert(false, 'should never be reached');
+            }, (error) => {
+                assert.equal(error, 'InvalidRepository');
+            });
+        });
+    });
+
     it('should return "InvalidCommitSets" when commit sets contains an inconsistent number of revisions', () => {
         return addTriggerableAndCreateTask('some task').then((taskId) => {
             return PrivilegedAPI.sendRequest('create-test-group', {name: 'test', task: taskId, commitSets: {'WebKit': ['191622', '191623'], 'macOS': ['15A284']}}).then((content) => {
@@ -235,7 +278,7 @@ describe('/privileged-api/create-test-group', function () {
         });
     });
 
-    it('should create a test group with the repetition count of one when repetitionCount is omitted', () => {
+    it('should create a test group from commitSets with the repetition count of one when repetitionCount is omitted', () => {
         return addTriggerableAndCreateTask('some task').then((taskId) => {
             let insertedGroupId;
             return PrivilegedAPI.sendRequest('create-test-group', {name: 'test', task: taskId, commitSets: {'WebKit': ['191622', '191623']}}).then((content) => {
@@ -257,6 +300,30 @@ describe('/privileged-api/create-test-group', function () {
         });
     });
 
+    it('should create a test group from revisionSets with the repetition count of one when repetitionCount is omitted', () => {
+        return addTriggerableAndCreateTask('some task').then((taskId) => {
+            const webkit = Repository.all().find((repository) => repository.name() == 'WebKit');
+            const params = {name: 'test', task: taskId, revisionSets: [{[webkit.id()]: '191622'}, {[webkit.id()]: '191623'}]};
+            let insertedGroupId;
+            return PrivilegedAPI.sendRequest('create-test-group', params).then((content) => {
+                insertedGroupId = content['testGroupId'];
+                return TestGroup.fetchByTask(taskId);
+            }).then((testGroups) => {
+                assert.equal(testGroups.length, 1);
+                const group = testGroups[0];
+                assert.equal(group.id(), insertedGroupId);
+                assert.equal(group.repetitionCount(), 1);
+                const requests = group.buildRequests();
+                assert.equal(requests.length, 2);
+                const webkit = Repository.all().filter((repository) => repository.name() == 'WebKit')[0];
+                assert.deepEqual(requests[0].commitSet().repositories(), [webkit]);
+                assert.deepEqual(requests[1].commitSet().repositories(), [webkit]);
+                assert.equal(requests[0].commitSet().revisionForRepository(webkit), '191622');
+                assert.equal(requests[1].commitSet().revisionForRepository(webkit), '191623');
+            });
+        });
+    });
+
     it('should create a test group with the repetition count of two with two repositories', () => {
         return addTriggerableAndCreateTask('some task').then((taskId) => {
             let insertedGroupId;