Commit order should always be returned by api.
authordewei_zhu@apple.com <dewei_zhu@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 17 Apr 2018 17:23:18 +0000 (17:23 +0000)
committerdewei_zhu@apple.com <dewei_zhu@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 17 Apr 2018 17:23:18 +0000 (17:23 +0000)
https://bugs.webkit.org/show_bug.cgi?id=184674

Reviewed by Ryosuke Niwa.

Commit order sometimes missing in CommitLog object before this change.
This makes ordering commits logic become unnecessarily complicate.
This change will ensure commit order is always fetched for a CommitLog object.
Change measurement-set API to contain commit order information.
Change commits API to contain commit order information.

* public/api/measurement-set.php: Includes commit order information.
* public/include/commit-log-fetcher.php:
* public/v3/models/commit-log.js: Added a function to return order information.
(CommitLog.prototype.updateSingleton): This function should update commit order.
(CommitLog.prototype.order): Returns the order of commit.
* public/v3/models/commit-set.js:
(MeasurementCommitSet): Update MeasurementCommitSet to contain commit order information when creating CommitLog object.
* server-tests/api-measurement-set-tests.js: Updated unit tests.
* unit-tests/analysis-task-tests.js: Update unit tests to contain commit order information in mock data.
(measurementCluster):
* unit-tests/commit-log-tests.js: Added unit tests for CommitLog.order.
* unit-tests/commit-set-tests.js: Added commit order in MeasurementCommitSet.
* unit-tests/measurement-adaptor-tests.js: Updated unit tests to contain commit order information in mock data.
* unit-tests/measurement-set-tests.js: Updated unit tests to contain commit order information in mock data.

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

Websites/perf.webkit.org/ChangeLog
Websites/perf.webkit.org/public/api/measurement-set.php
Websites/perf.webkit.org/public/include/commit-log-fetcher.php
Websites/perf.webkit.org/public/v3/models/commit-log.js
Websites/perf.webkit.org/public/v3/models/commit-set.js
Websites/perf.webkit.org/server-tests/api-measurement-set-tests.js
Websites/perf.webkit.org/unit-tests/analysis-task-tests.js
Websites/perf.webkit.org/unit-tests/commit-log-tests.js
Websites/perf.webkit.org/unit-tests/commit-set-tests.js
Websites/perf.webkit.org/unit-tests/measurement-adaptor-tests.js
Websites/perf.webkit.org/unit-tests/measurement-set-tests.js

index 5e58b9c..de2bb9b 100644 (file)
@@ -1,3 +1,31 @@
+2018-04-16  Dewei Zhu  <dewei_zhu@apple.com>
+
+        Commit order should always be returned by api.
+        https://bugs.webkit.org/show_bug.cgi?id=184674
+
+        Reviewed by Ryosuke Niwa.
+
+        Commit order sometimes missing in CommitLog object before this change.
+        This makes ordering commits logic become unnecessarily complicate.
+        This change will ensure commit order is always fetched for a CommitLog object.
+        Change measurement-set API to contain commit order information.
+        Change commits API to contain commit order information.
+
+        * public/api/measurement-set.php: Includes commit order information.
+        * public/include/commit-log-fetcher.php:
+        * public/v3/models/commit-log.js: Added a function to return order information.
+        (CommitLog.prototype.updateSingleton): This function should update commit order.
+        (CommitLog.prototype.order): Returns the order of commit.
+        * public/v3/models/commit-set.js:
+        (MeasurementCommitSet): Update MeasurementCommitSet to contain commit order information when creating CommitLog object.
+        * server-tests/api-measurement-set-tests.js: Updated unit tests.
+        * unit-tests/analysis-task-tests.js: Update unit tests to contain commit order information in mock data.
+        (measurementCluster):
+        * unit-tests/commit-log-tests.js: Added unit tests for CommitLog.order.
+        * unit-tests/commit-set-tests.js: Added commit order in MeasurementCommitSet.
+        * unit-tests/measurement-adaptor-tests.js: Updated unit tests to contain commit order information in mock data.
+        * unit-tests/measurement-set-tests.js: Updated unit tests to contain commit order information in mock data.
+
 2018-04-15  Ryosuke Niwa  <rniwa@webkit.org>
 
         Make it possible to hide some repository groups
index 6c4d083..712aae7 100644 (file)
@@ -170,7 +170,7 @@ class MeasurementSetFetcher {
     function execute_query($config_id) {
         return $this->db->query('
             SELECT test_runs.*, build_id, build_number, build_builder, build_time,
-            array_agg((commit_id, commit_repository, commit_revision, extract(epoch from commit_time at time zone \'utc\') * 1000)) AS revisions,
+            array_agg((commit_id, commit_repository, commit_revision, commit_order, extract(epoch from commit_time at time zone \'utc\') * 1000)) AS revisions,
             extract(epoch from max(commit_time at time zone \'utc\')) * 1000 AS revision_time
                 FROM builds
                     LEFT OUTER JOIN build_commits ON commit_build = build_id
@@ -208,7 +208,8 @@ class MeasurementSetFetcher {
     }
 
     private static function parse_revisions_array(&$postgres_array) {
-        // e.g. {"(<commit-id>,<repository-id>,<revision>,\"2012-10-16 14:53:00\")","(<commit-id>,<repository-id>,<revision>,)"}
+        // e.g. {"(<commit-id>,<repository-id>,<revision>,<order>,\"2012-10-16 14:53:00\")","(<commit-id>,<repository-id>,<revision>,<order>,)",
+        // "(<commit-id>,<repository-id>,<revision>,,)", "(<commit-id>,<repository-id>,<revision>,,\"2012-10-16 14:53:00\")"}
         $outer_array = json_decode('[' . trim($postgres_array, '{}') . ']');
         $revisions = array();
         foreach ($outer_array as $item) {
@@ -218,8 +219,10 @@ class MeasurementSetFetcher {
             $commit_id = intval(trim($name_and_revision[0], '"'));
             $repository_id = intval(trim($name_and_revision[1], '"'));
             $revision = trim($name_and_revision[2], '"');
-            $time = intval(trim($name_and_revision[3], '"'));
-            array_push($revisions, array($commit_id, $repository_id, $revision, $time));
+            $trimmed_order = trim($name_and_revision[3], '"');
+            $order = strlen($trimmed_order) ? intval($trimmed_order) : NULL;
+            $time = intval(trim($name_and_revision[4], '"'));
+            array_push($revisions, array($commit_id, $repository_id, $revision, $order, $time));
         }
         return $revisions;
     }
index 709cf2d..9c76263 100644 (file)
@@ -61,6 +61,7 @@ class CommitLogFetcher {
             committer_account as "authorEmail",
             commit_repository as "repository",
             commit_message as "message",
+            commit_order as "order",
             EXISTS(SELECT * FROM commit_ownerships WHERE commit_owner = commit_id) as "ownsCommits"
             FROM commits LEFT OUTER JOIN committers ON commit_committer = committer_id
             WHERE commit_repository = $1 AND commit_reported = true';
index 2c4b49f..39b3260 100644 (file)
@@ -29,6 +29,8 @@ class CommitLog extends DataModelObject {
             this._rawData.message = rawData.message;
         if (rawData.ownsCommits)
             this._rawData.ownsCommits = rawData.ownsCommits;
+        if (rawData.order)
+            this._rawData.order = rawData.order;
     }
 
     repository() { return this._repository; }
@@ -40,7 +42,7 @@ class CommitLog extends DataModelObject {
     ownsCommits() { return this._rawData['ownsCommits']; }
     ownedCommits() { return this._ownedCommits; }
     ownerCommit() { return this._ownerCommit; }
-
+    order() { return this._rawData['order']; }
     setOwnerCommits(ownerCommit) { this._ownerCommit = ownerCommit; }
 
     label()
index e1bbcfd..f179511 100644 (file)
@@ -228,18 +228,19 @@ class MeasurementCommitSet extends CommitSet {
     constructor(id, revisionList)
     {
         super(id, null);
-        for (var values of revisionList) {
-            // [<commit-id>, <repository-id>, <revision>, <time>]
-            var commitId = values[0];
-            var repositoryId = values[1];
-            var revision = values[2];
-            var time = values[3];
-            var repository = Repository.findById(repositoryId);
+        for (const values of revisionList) {
+            // [<commit-id>, <repository-id>, <revision>, <order>, <time>]
+            const commitId = values[0];
+            const repositoryId = values[1];
+            const revision = values[2];
+            const order = values[3];
+            const time = values[4];
+            const repository = Repository.findById(repositoryId);
             if (!repository)
                 continue;
 
             // FIXME: Add a flag to remember the fact this commit log is incomplete.
-            const commit = CommitLog.ensureSingleton(commitId, {repository: repository, revision: revision, time: time});
+            const commit = CommitLog.ensureSingleton(commitId, {repository, revision, order, time});
             this._repositoryToCommitMap.set(repository, commit);
             this._repositories.push(repository);
         }
index 7ab4089..84713a8 100644 (file)
@@ -325,7 +325,7 @@ describe("/api/measurement-set", function () {
                sum: 65,
                squareSum: 855,
                markedOutlier: false,
-               revisions: [[1, repositoryId, '144000', revisionTime]],
+               revisions: [[1, repositoryId, '144000', null, revisionTime]],
                commitTime: revisionTime,
                buildTime: revisionBuildTime,
                buildNumber: '124' });
@@ -393,7 +393,7 @@ describe("/api/measurement-set", function () {
                sum: 15,
                squareSum: 55,
                markedOutlier: false,
-               revisions: [[3, 1, 'macOS 16C68', 0]],
+               revisions: [[3, 1, 'macOS 16C68', 1, 0]],
                commitTime: +Date.UTC(2017, 0, 19, 15, 28, 1),
                buildTime: +Date.UTC(2017, 0, 19, 15, 28, 1),
                buildNumber: '1001' });
@@ -403,7 +403,7 @@ describe("/api/measurement-set", function () {
                 sum: 35,
                 squareSum: 255,
                 markedOutlier: false,
-                revisions: [[2, 1, 'macOS 16A323', 0]],
+                revisions: [[2, 1, 'macOS 16A323', 0, 0]],
                 commitTime: +Date.UTC(2017, 0, 19, 19, 46, 37),
                 buildTime: +Date.UTC(2017, 0, 19, 19, 46, 37),
                 buildNumber: '1002' });
index 415ea01..d696f68 100644 (file)
@@ -72,18 +72,21 @@ function measurementCluster()
                             105978,
                             10,
                             '791451',
+                            null,
                             1454481204649
                         ],
                         [
                             105975,
                             11,
                             '196051',
+                            null,
                             1454481246108
                         ],
                         [
                             105502,
                             9,
                             '10.11 15D21',
+                            1504021,
                             0
                         ]
                     ],
index d9263b0..849b036 100644 (file)
@@ -48,6 +48,7 @@ function osxCommit()
         repository: MockModels.osx,
         revision: '10.11.4 15E65',
         time: null,
+        order: 1504065
     });
 }
 
@@ -57,6 +58,7 @@ function oldOSXCommit()
         repository: MockModels.osx,
         revision: '10.11.3 15D21',
         time: null,
+        order: 1503021
     });
 }
 
@@ -67,6 +69,7 @@ function commitWithoutOwnedCommits()
         revision: '10.11.4 15E66',
         ownsCommits: false,
         time: null,
+        order: 1504065
     });
 }
 
@@ -77,6 +80,7 @@ function ownerCommit()
         revision: '10.11.4 15E65',
         ownsCommits: true,
         time: null,
+        order: 1504065
     });
 }
 
@@ -87,6 +91,7 @@ function otherOwnerCommit()
         revision: '10.11.4 15E66',
         ownsCommits: true,
         time: null,
+        order: 1504066
     });
 }
 
@@ -121,6 +126,15 @@ describe('CommitLog', function () {
         });
     });
 
+    describe('order', () => {
+        it('should return null if no commit order', () => {
+            assert.equal(webkitCommit().order(), null);
+        });
+        it('should return commit order if order exists', () => {
+            assert.equal(osxCommit().order(), 1504065);
+        });
+    });
+
     describe('diff', function () {
         it('should use label() as the label the previous commit is missing', function () {
             assert.deepEqual(webkitCommit().diff(), {
index e3bfb60..3af3579 100644 (file)
@@ -287,7 +287,7 @@ describe('CommitSet', () => {
     function oneMeasurementCommitSet()
     {
         return MeasurementCommitSet.ensureSingleton(1, [
-            [2017, 11, 'webkit-commit-0', 1456932773000]
+            [2017, 11, 'webkit-commit-0', null, 1456932773000]
         ]);
     }
 
index 24a4907..113eaa0 100644 (file)
@@ -10,8 +10,8 @@ const sampleCluster = {
     'clusterSize': 5184000000,
     'configurations': {
         'current': [
-            [28954983, 217.94607142857, 20, 4358.9214285714, 950303.02365434, false, [[111, 9, '10.11 15D21', 0], [222, 11, '192483', 1447707055576], [333, 999, 'some unknown revision', 0]], 1447707055576, 184629, 1447762266153, '178', 176],
-            [28952257, 220.11455357143, 20, 4402.2910714286, 969099.67509885, false, [[111, 9, '10.11 15D21', 0], [444, 11, '192486', 1447713500460]], 1447713500460, 184614, 1447760255683, '177', 176]
+            [28954983, 217.94607142857, 20, 4358.9214285714, 950303.02365434, false, [[111, 9, '10.11 15D21', 1504021, 0], [222, 11, '192483', null, 1447707055576], [333, 999, 'some unknown revision', null, 0]], 1447707055576, 184629, 1447762266153, '178', 176],
+            [28952257, 220.11455357143, 20, 4402.2910714286, 969099.67509885, false, [[111, 9, '10.11 15D21', 1504021, 0], [444, 11, '192486', null, 1447713500460]], 1447713500460, 184614, 1447760255683, '177', 176]
         ],
         'baseline': [
             [10548956, 312.59, 1, 0, 0, false, [], 1420070400000, 67724, 1420070400000, "0", 0]
index 1b2fab1..47393e0 100644 (file)
@@ -716,37 +716,37 @@ describe('MeasurementSet', () => {
                 "current": [
                     [
                         26530031, 135.26375, 80, 10821.1, 1481628.13, false,
-                        [ [27173, 1, "210096", 1482398562950] ],
+                        [ [27173, 1, "210096", null, 1482398562950] ],
                         1482398562950, 52999, 1482413222311, "10877", 7
                     ],
                     [
                         26530779, 153.2675, 80, 12261.4, 1991987.4, true, // changed to true.
-                        [ [27174,1,"210097",1482424870729] ],
+                        [ [27174,1,"210097", null, 1482424870729] ],
                         1482424870729, 53000, 1482424992735, "10878", 7
                     ],
                     [
                         26532275, 134.2725, 80, 10741.8, 1458311.88, false,
-                        [ [ 27176, 1, "210102", 1482431464371 ] ],
+                        [ [ 27176, 1, "210102", null, 1482431464371 ] ],
                         1482431464371, 53002, 1482436041865, "10879", 7
                     ],
                     [
                         26547226, 150.9625, 80, 12077, 1908614.94, false,
-                        [ [ 27195, 1, "210168", 1482852412735 ] ],
+                        [ [ 27195, 1, "210168", null, 1482852412735 ] ],
                         1482852412735, 53022, 1482852452143, "10902", 7
                     ],
                     [
                         26559915, 141.72, 80, 11337.6, 1633126.8, false,
-                        [ [ 27211, 1, "210222", 1483347732051 ] ],
+                        [ [ 27211, 1, "210222", null, 1483347732051 ] ],
                         1483347732051, 53039, 1483347926429, "10924", 7
                     ],
                     [
                         26564388, 138.13125, 80, 11050.5, 1551157.93, false,
-                        [ [ 27217, 1, "210231", 1483412171531 ] ],
+                        [ [ 27217, 1, "210231", null, 1483412171531 ] ],
                         1483412171531, 53045, 1483415426049, "10930", 7
                     ],
                     [
                         26568867, 144.16, 80, 11532.8, 1694941.1, false,
-                        [ [ 27222, 1, "210240", 1483469584347 ] ],
+                        [ [ 27222, 1, "210240", null, 1483469584347 ] ],
                         1483469584347, 53051, 1483469642993, "10935", 7
                     ]
                 ]