"/api/report" does not check commit time correctly.
authordewei_zhu@apple.com <dewei_zhu@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 8 Nov 2018 00:39:38 +0000 (00:39 +0000)
committerdewei_zhu@apple.com <dewei_zhu@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 8 Nov 2018 00:39:38 +0000 (00:39 +0000)
https://bugs.webkit.org/show_bug.cgi?id=191351

Reviewed by Ryosuke Niwa.

Test result report API does not convert formated time string to timestamp correctly
which result in not checking commit time correctly.

* public/include/report-processor.php: Use 'strtotime' instead of 'floatval'.
Accepts the time delta within 1 seconds.
* server-tests/api-report-tests.js: Added unit tests.
(reportWitMismatchingCommitTime):
(reportWithOneSecondCommitTimeDifference):

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

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

index 1ad61fb..53835de 100644 (file)
@@ -1,3 +1,19 @@
+2018-11-07  Dewei Zhu  <dewei_zhu@apple.com>
+
+        "/api/report" does not check commit time correctly.
+        https://bugs.webkit.org/show_bug.cgi?id=191351
+
+        Reviewed by Ryosuke Niwa.
+
+        Test result report API does not convert formated time string to timestamp correctly
+        which result in not checking commit time correctly.
+
+        * public/include/report-processor.php: Use 'strtotime' instead of 'floatval'.
+        Accepts the time delta within 1 seconds.
+        * server-tests/api-report-tests.js: Added unit tests.
+        (reportWitMismatchingCommitTime):
+        (reportWithOneSecondCommitTimeDifference):
+
 2018-11-06  Dewei Zhu  <dewei_zhu@apple.com>
 
         Custom test group form should use commit set map before customization as the behavior of radio buttons.
index 30f84b9..d9e8190 100644 (file)
@@ -171,7 +171,8 @@ class ReportProcessor {
                 array('repository' => $repository_id, 'revision' => $revision_data['revision']), $commit_data, '*');
             if (!$commit_row)
                 $this->rollback_with_error('FailedToRecordCommit', $commit_data);
-            if ($commit_data['time'] && abs(floatval($commit_row['commit_time']) - floatval($commit_data['time'])) > 1.0)
+
+            if ($commit_data['time'] && abs(strtotime($commit_row['commit_time']) - strtotime($commit_data['time'])) > 1.0)
                 $this->rollback_with_error('MismatchingCommitTime', array('existing' => $commit_row, 'new' => $commit_data));
 
             if (!$this->db->select_or_insert_row('build_commits', null,
index 8f2ff19..1eb59da 100644 (file)
@@ -33,6 +33,50 @@ describe("/api/report", function () {
         };
     }
 
+    function reportWitMismatchingCommitTime()
+    {
+        return {
+            "buildNumber": "124",
+            "buildTime": "2013-02-28T10:12:03.388304",
+            "builderName": "someBuilder",
+            "slaveName": "someSlave",
+            "builderPassword": "somePassword",
+            "platform": "Mountain Lion",
+            "tests": {},
+            "revisions": {
+                "macOS": {
+                    "revision": "10.8.2 12C60"
+                },
+                "WebKit": {
+                    "revision": "141977",
+                    "timestamp": "2013-02-06T08:55:10.9Z"
+                }
+            }
+        };
+    }
+
+    function reportWithOneSecondCommitTimeDifference()
+    {
+        return {
+            "buildNumber": "125",
+            "buildTime": "2013-02-28T10:12:03.388304",
+            "builderName": "someBuilder",
+            "slaveName": "someSlave",
+            "builderPassword": "somePassword",
+            "platform": "Mountain Lion",
+            "tests": {},
+            "revisions": {
+                "macOS": {
+                    "revision": "10.8.2 12C60"
+                },
+                "WebKit": {
+                    "revision": "141977",
+                    "timestamp": "2013-02-06T08:55:19.9Z"
+                }
+            }
+        };
+    }
+
     function emptySlaveReport()
     {
         return {
@@ -99,6 +143,32 @@ describe("/api/report", function () {
         });
     });
 
+    it('should reject report with "MismatchingCommitTime" if time difference is larger than 1 second', async () => {
+        await addBuilderForReport(emptyReport());
+        let response = await TestServer.remoteAPI().postJSON('/api/report/', [reportWitMismatchingCommitTime()]);
+        assert.equal(response['status'], 'OK');
+        assert.equal(response['failureStored'], false);
+        assert.equal(response['processedRuns'], 1);
+
+        response = await TestServer.remoteAPI().postJSON('/api/report/', [emptyReport()]);
+        assert.equal(response['status'], 'MismatchingCommitTime');
+        assert.equal(response['failureStored'], true);
+        assert.equal(response['processedRuns'], 0);
+    });
+
+    it('should not reject report if the commit time difference is within 1 second"', async () => {
+        await addBuilderForReport(emptyReport());
+        let response = await TestServer.remoteAPI().postJSON('/api/report/', [reportWithOneSecondCommitTimeDifference()]);
+        assert.equal(response['status'], 'OK');
+        assert.equal(response['failureStored'], false);
+        assert.equal(response['processedRuns'], 1);
+
+        response = await TestServer.remoteAPI().postJSON('/api/report/', [emptyReport()]);
+        assert.equal(response['status'], 'OK');
+        assert.equal(response['failureStored'], false);
+        assert.equal(response['processedRuns'], 1);
+    });
+
     it("should store a report from a valid builder", () => {
         return addBuilderForReport(emptyReport()).then(() => {
             return TestServer.remoteAPI().postJSON('/api/report/', [emptyReport()]);