Skip to content

Commit

Permalink
Commit order should always be returned by api.
Browse files Browse the repository at this point in the history
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: http://svn.webkit.org/repository/webkit/trunk@230719 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
[email protected] committed Apr 17, 2018
1 parent da08682 commit c340b15
Show file tree
Hide file tree
Showing 11 changed files with 78 additions and 26 deletions.
28 changes: 28 additions & 0 deletions Websites/perf.webkit.org/ChangeLog
Original file line number Diff line number Diff line change
@@ -1,3 +1,31 @@
2018-04-16 Dewei Zhu <[email protected]>

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 <[email protected]>

Make it possible to hide some repository groups
Expand Down
11 changes: 7 additions & 4 deletions Websites/perf.webkit.org/public/api/measurement-set.php
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ function fetch_next_cluster() {
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
Expand Down Expand Up @@ -208,7 +208,8 @@ private static function format_run(&$run, &$commit_time) {
}

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) {
Expand All @@ -218,8 +219,10 @@ private static function parse_revisions_array(&$postgres_array) {
$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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ function fetch_between($repository_id, $before_first_revision, $last_revision, $
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';
Expand Down
4 changes: 3 additions & 1 deletion Websites/perf.webkit.org/public/v3/models/commit-log.js
Original file line number Diff line number Diff line change
Expand Up @@ -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; }
Expand All @@ -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()
Expand Down
17 changes: 9 additions & 8 deletions Websites/perf.webkit.org/public/v3/models/commit-set.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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' });
Expand Down Expand Up @@ -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' });
Expand All @@ -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' });
Expand Down
3 changes: 3 additions & 0 deletions Websites/perf.webkit.org/unit-tests/analysis-task-tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,18 +72,21 @@ function measurementCluster()
105978,
10,
'791451',
null,
1454481204649
],
[
105975,
11,
'196051',
null,
1454481246108
],
[
105502,
9,
'10.11 15D21',
1504021,
0
]
],
Expand Down
14 changes: 14 additions & 0 deletions Websites/perf.webkit.org/unit-tests/commit-log-tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ function osxCommit()
repository: MockModels.osx,
revision: '10.11.4 15E65',
time: null,
order: 1504065
});
}

Expand All @@ -57,6 +58,7 @@ function oldOSXCommit()
repository: MockModels.osx,
revision: '10.11.3 15D21',
time: null,
order: 1503021
});
}

Expand All @@ -67,6 +69,7 @@ function commitWithoutOwnedCommits()
revision: '10.11.4 15E66',
ownsCommits: false,
time: null,
order: 1504065
});
}

Expand All @@ -77,6 +80,7 @@ function ownerCommit()
revision: '10.11.4 15E65',
ownsCommits: true,
time: null,
order: 1504065
});
}

Expand All @@ -87,6 +91,7 @@ function otherOwnerCommit()
revision: '10.11.4 15E66',
ownsCommits: true,
time: null,
order: 1504066
});
}

Expand Down Expand Up @@ -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(), {
Expand Down
2 changes: 1 addition & 1 deletion Websites/perf.webkit.org/unit-tests/commit-set-tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -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]
]);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
14 changes: 7 additions & 7 deletions Websites/perf.webkit.org/unit-tests/measurement-set-tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
]
]
Expand Down

0 comments on commit c340b15

Please sign in to comment.