+2017-01-19 Ryosuke Niwa <rniwa@webkit.org>
+
+ measurement-sets API can incorrectly order points with OS version without commit time
+ https://bugs.webkit.org/show_bug.cgi?id=167227
+
+ Reviewed by Chris Dumez.
+
+ Ignore revision_order for the purpose of ordering data points in /api/measurement-sets.
+
+ These orderings are used in some UI (e.g A/B testing) to order OS build numbers which do not have a timestamp
+ associated with each "revision".
+
+ The baseline measurements made in our internal dashboard were using these ordering numbers before ordering
+ results with build time. Because those data points don't have an associated Webkit revisions, all data points
+ were ordered first by macOS's revision_order, then build time. Because v3 UI completely ignores revision_order
+ for the purpose of plotting data points, this resulted in some data points being plotted in a wrong order
+ with some lines going backwards in time.
+
+ This patch addresses this discrepancy by stop ordering data points with revision_order in the JSON API.
+
+ * public/api/measurement-set.php:
+ (MeasurementSetFetcher::execute_query): Fixed the bug.
+ * server-tests/api-measurement-set-tests.js: Added a test.
+
2017-01-19 Ryosuke Niwa <rniwa@webkit.org>
Build fix after r210626. We need to clear Triggerable's static map in each iteration.
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,
- extract(epoch from max(commit_time at time zone \'utc\')) * 1000 AS revision_time, max(commit_order) AS revision_order
+ 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
LEFT OUTER JOIN commits ON build_commit = commit_id, test_runs
WHERE run_build = build_id AND run_config = $1 AND NOT EXISTS (SELECT * FROM build_requests WHERE request_build = build_id)
GROUP BY build_id, build_builder, build_number, build_time, build_latest_revision, build_slave,
run_id, run_config, run_build, run_iteration_count_cache, run_mean_cache, run_sum_cache, run_square_sum_cache, run_marked_outlier
- ORDER BY revision_time, revision_order, build_time', array($config_id));
+ ORDER BY revision_time, build_time', array($config_id));
}
static function format_map()
}).catch(done);
});
+ it("should order results by build time when commit times are missing", function (done) {
+ const remote = TestServer.remoteAPI();
+ let repositoryId;
+ addBuilderForReport(reportWithBuildTime[0]).then(() => {
+ const db = TestServer.database();
+ return Promise.all([
+ db.insert('repositories', {'id': 1, 'name': 'macOS'}),
+ db.insert('commits', {'id': 2, 'repository': 1, 'revision': 'macOS 16A323', 'order': 0}),
+ db.insert('commits', {'id': 3, 'repository': 1, 'revision': 'macOS 16C68', 'order': 1}),
+ ]);
+ }).then(() => {
+ return remote.postJSON('/api/report/', [{
+ "buildNumber": "1001",
+ "buildTime": '2017-01-19 15:28:01',
+ "revisions": {
+ "macOS": {
+ "revision": "macOS 16C68",
+ },
+ },
+ "builderName": "someBuilder",
+ "builderPassword": "somePassword",
+ "platform": "Sierra",
+ "tests": { "Test": {"metrics": {"Time": { "baseline": [1, 2, 3, 4, 5] } } } },
+ }]);
+ }).then(function () {
+ return remote.postJSON('/api/report/', [{
+ "buildNumber": "1002",
+ "buildTime": '2017-01-19 19:46:37',
+ "revisions": {
+ "macOS": {
+ "revision": "macOS 16A323",
+ },
+ },
+ "builderName": "someBuilder",
+ "builderPassword": "somePassword",
+ "platform": "Sierra",
+ "tests": { "Test": {"metrics": {"Time": { "baseline": [5, 6, 7, 8, 9] } } } },
+ }]);
+ }).then(function () {
+ return queryPlatformAndMetricWithRepository('Sierra', 'Time', 'macOS');
+ }).then(function (result) {
+ return remote.getJSONWithStatus(`/api/measurement-set/?platform=${result.platformId}&metric=${result.metricId}`);
+ }).then(function (response) {
+ const currentRows = response['configurations']['baseline'];
+ assert.equal(currentRows.length, 2);
+ assert.deepEqual(format(response['formatMap'], currentRows[0]), {
+ mean: 3,
+ iterationCount: 5,
+ sum: 15,
+ squareSum: 55,
+ markedOutlier: false,
+ revisions: [[3, 1, 'macOS 16C68', 0]],
+ commitTime: +Date.UTC(2017, 0, 19, 15, 28, 1),
+ buildTime: +Date.UTC(2017, 0, 19, 15, 28, 1),
+ buildNumber: '1001' });
+ assert.deepEqual(format(response['formatMap'], currentRows[1]), {
+ mean: 7,
+ iterationCount: 5,
+ sum: 35,
+ squareSum: 255,
+ markedOutlier: false,
+ revisions: [[2, 1, 'macOS 16A323', 0]],
+ commitTime: +Date.UTC(2017, 0, 19, 19, 46, 37),
+ buildTime: +Date.UTC(2017, 0, 19, 19, 46, 37),
+ buildNumber: '1002' });
+ done();
+ }).catch(done);
+ });
+
function buildNumbers(parsedResult, config)
{
return parsedResult['configurations'][config].map(function (row) {
}).catch(done);
});
-
it("should create cached results", function (done) {
const remote = TestServer.remoteAPI();
let cachePrefix;
git://git.webkit.org / WebKit.git / commitdiff