commit time returned by '/api/measurement-set' should match the one returned by ...
authordewei_zhu@apple.com <dewei_zhu@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 14 Nov 2018 05:31:45 +0000 (05:31 +0000)
committerdewei_zhu@apple.com <dewei_zhu@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 14 Nov 2018 05:31:45 +0000 (05:31 +0000)
https://bugs.webkit.org/show_bug.cgi?id=191457

Reviewed by Dean Jackson and Ryosuke Niwa.

Commit time returned by '/api/measurement-set' sometimes is calculated by 'epoch from ..'.
This function will return a floating number with 5 or 6 decimal digits due to double precision limitations.
However, some commits may be reported with 6 decimal decimal.
So the commit time for those commits will be rounded to 5 decimal digits.
In order to avoid front end assertion failure in CommitLog, Database::to_js_time need to
match this behavior.

* public/include/db.php: Change the behavior to match that of postgres.
Added logic to avoid losing precision in php.
* server-tests/api-measurement-set-tests.js: Added unit tests for this bug.
(queryPlatformAndMetric): Fix a bug that arguments are not used at all.

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

Websites/perf.webkit.org/ChangeLog
Websites/perf.webkit.org/public/include/db.php
Websites/perf.webkit.org/public/include/report-processor.php
Websites/perf.webkit.org/server-tests/api-measurement-set-tests.js

index 53835de..40519bd 100644 (file)
@@ -1,3 +1,21 @@
+2018-11-08  Dewei Zhu  <dewei_zhu@apple.com>
+
+        commit time returned by '/api/measurement-set' should match the one returned by '/api/commits'.
+        https://bugs.webkit.org/show_bug.cgi?id=191457
+
+        Reviewed by Dean Jackson and Ryosuke Niwa.
+
+        Commit time returned by '/api/measurement-set' sometimes is calculated by 'epoch from ..'.
+        This function may return a floating number with 5 or 6 decimal digits due to double precision limitations.
+        However, some commits may be reported with 6 decimal decimal.
+        So the commit time for those commits will sometime be rounded to 5 decimal digits.
+        In order to avoid front end assertion failure in CommitLog, Database::to_js_time need to round to 5 digits.
+
+        * public/include/db.php: Change the behavior to match that of postgres.
+        Added logic to avoid losing precision in php.
+        * server-tests/api-measurement-set-tests.js: Added unit tests for this bug.
+        (queryPlatformAndMetric): Fix a bug that arguments are not used at all.
+
 2018-11-07  Dewei Zhu  <dewei_zhu@apple.com>
 
         "/api/report" does not check commit time correctly.
index ada95ed..8a2d582 100644 (file)
@@ -100,11 +100,14 @@ class Database
     }
 
     static function to_js_time($time_str) {
-        $timestamp_in_s = strtotime($time_str);
+        $timestamp_in_ms = strtotime($time_str) * 1000;
         $dot_index = strrpos($time_str, '.');
-        if ($dot_index !== FALSE)
-            $timestamp_in_s += floatval(substr($time_str, $dot_index));
-        return intval($timestamp_in_s * 1000);
+        if ($dot_index !== FALSE) {
+            // Keep 5 decimal digits as postgres timestamp may only have 5 decimal digits.
+            // Multiply by 1000 ahead to avoid losing precision. 1538041792.670479 will become 1538041792.6705 on php.
+            $timestamp_in_ms += round(floatval(substr($time_str, $dot_index)), 5) * 1000;
+        }
+        return intval($timestamp_in_ms);
     }
 
     static function escape_for_like($string) {
index d9e8190..1bd964f 100644 (file)
@@ -172,7 +172,7 @@ class ReportProcessor {
             if (!$commit_row)
                 $this->rollback_with_error('FailedToRecordCommit', $commit_data);
 
-            if ($commit_data['time'] && abs(strtotime($commit_row['commit_time']) - strtotime($commit_data['time'])) > 1.0)
+            if ($commit_data['time'] && abs(Database::to_js_time($commit_row['commit_time']) - Database::to_js_time($commit_data['time'])) > 1000.0)
                 $this->rollback_with_error('MismatchingCommitTime', array('existing' => $commit_row, 'new' => $commit_data));
 
             if (!$this->db->select_or_insert_row('build_commits', null,
index e2aa2c4..6c594c1 100644 (file)
@@ -16,8 +16,8 @@ describe("/api/measurement-set", function () {
     {
         const db = TestServer.database();
         return Promise.all([
-            db.selectFirstRow('platforms', {name: 'Mountain Lion'}),
-            db.selectFirstRow('test_metrics', {name: 'Time'}),
+            db.selectFirstRow('platforms', {name: platformName}),
+            db.selectFirstRow('test_metrics', {name: metricName}),
         ]).then(function (result) {
             return {platformId: result[0]['id'], metricId: result[1]['id']};
         });
@@ -545,4 +545,91 @@ describe("/api/measurement-set", function () {
             1, 1362046323388, '123', 1, 1, 'current']]);
     });
 
+    const reportWithCommitsNeedsRoundCommitTimeAndBuildRequest = {
+        "buildNumber": "123",
+        "buildTime": "2013-02-28T10:12:03.388304",
+        "builderName": "someBuilder",
+        "builderPassword": "somePassword",
+        "platform": "Mountain Lion",
+        "buildRequest": "700",
+        "tests": {
+            "test": {
+                "metrics": {"FrameRate": { "current": [[[0, 4], [100, 5], [205, 3]]] }}
+            },
+        },
+        "revisions": {
+            "macOS": {
+                "revision": "10.8.2 12C60",
+                "timestamp": '2018-09-27T09:49:52.670499Z',
+            },
+            "WebKit": {
+                "revision": "141977",
+                "timestamp": '2018-09-27T09:49:52.670999Z',
+            }
+        }
+    };
+
+    it("commit time of commits from measurement set queried by analysis task should match the one from `/api/commits`", async () => {
+        const expectedWebKitCommitTime = 1538041792671;
+        const expectedMacOSCommitTime = 1538041792670;
+        const remote = TestServer.remoteAPI();
+        await MockData.addMockData(TestServer.database());
+        let response = await reportAfterAddingBuilderAndAggregatorsWithResponse(reportWithCommitsNeedsRoundCommitTimeAndBuildRequest);
+        assert.equal(response['status'], 'OK');
+
+        const rawWebKitCommit = await remote.getJSONWithStatus(`/api/commits/WebKit/141977`);
+        assert.equal(rawWebKitCommit.commits[0].time, expectedWebKitCommitTime);
+
+        const rawMacOSCommit = await remote.getJSONWithStatus(`/api/commits/macOS/10.8.2%2012C60`);
+        assert.equal(rawMacOSCommit.commits[0].time, expectedMacOSCommitTime);
+
+        response = await TestServer.remoteAPI().getJSONWithStatus('/api/measurement-set/?analysisTask=500');
+        assert.equal(response['status'], 'OK');
+        assert.deepEqual(response['measurements'], [[1, 4, 3, 12, 50, [
+            ['1', '9', '10.8.2 12C60', null, expectedMacOSCommitTime], ['2', '11', '141977', null, expectedWebKitCommitTime]],
+            1, 1362046323388, '123', 1, 1, 'current']]);
+    });
+
+    const reportWithCommitsNeedsRoundCommitTime = {
+        "buildNumber": "123",
+        "buildTime": "2013-02-28T10:12:03.388304",
+        "builderName": "someBuilder",
+        "builderPassword": "somePassword",
+        "platform": "Mountain Lion",
+        "tests": {
+            "test": {
+                "metrics": {"FrameRate": { "current": [[[0, 4], [100, 5], [205, 3]]] }}
+            },
+        },
+        "revisions": {
+            "macOS": {
+                "revision": "10.8.2 12C60",
+                "timestamp": '2018-09-27T09:49:52.670499Z',
+            },
+            "WebKit": {
+                "revision": "141977",
+                "timestamp": '2018-09-27T09:49:52.670999Z',
+            }
+        }
+    };
+
+    it("commit time of commits from measurement set queried by platform and metric should match the one from `/api/commits`", async () => {
+        const expectedWebKitCommitTime = 1538041792671;
+        const expectedMacOSCommitTime = 1538041792670;
+        const remote = TestServer.remoteAPI();
+        await addBuilderForReport(reportWithCommitsNeedsRoundCommitTime);
+        await remote.postJSON('/api/report/', [reportWithCommitsNeedsRoundCommitTime]);
+
+        const rawWebKitCommit = await remote.getJSONWithStatus(`/api/commits/WebKit/141977`);
+        assert.equal(rawWebKitCommit.commits[0].time, expectedWebKitCommitTime);
+
+        const rawMacOSCommit = await remote.getJSONWithStatus(`/api/commits/macOS/10.8.2%2012C60`);
+        assert.equal(rawMacOSCommit.commits[0].time, expectedMacOSCommitTime);
+
+        const result = await queryPlatformAndMetric('Mountain Lion', 'FrameRate');
+        const primaryCluster = await remote.getJSONWithStatus(`/api/measurement-set/?platform=${result.platformId}&metric=${result.metricId}`);
+        assert.deepEqual(primaryCluster.configurations.current, [[1, 4, 3, 12, 50, false, [
+            [1, 1, '10.8.2 12C60', null, expectedMacOSCommitTime], [2, 2, '141977', null, expectedWebKitCommitTime]],
+            expectedWebKitCommitTime, 1, 1362046323388, '123', 1]]);
+    });
 });