Fix a bug that 'test_metrics' and 'tests' tables are not joined correctly in CommitLo...
authordewei_zhu@apple.com <dewei_zhu@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 20 Jun 2019 23:53:22 +0000 (23:53 +0000)
committerdewei_zhu@apple.com <dewei_zhu@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 20 Jun 2019 23:53:22 +0000 (23:53 +0000)
https://bugs.webkit.org/show_bug.cgi?id=199062

Reviewed by Ryosuke Niwa.

'test_metrics' and 'tests' should be joined based on 'metric_test' and 'test_id'.

* public/include/commit-log-fetcher.php: Fix the typo in the query.
* server-tests/api-commits-tests.js: Added a unit test for this change.

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

Websites/perf.webkit.org/ChangeLog
Websites/perf.webkit.org/public/include/commit-log-fetcher.php
Websites/perf.webkit.org/server-tests/api-commits-tests.js

index 85ef411..625a3fd 100644 (file)
@@ -1,3 +1,15 @@
+2019-06-20  Dewei Zhu  <dewei_zhu@apple.com>
+
+        Fix a bug that 'test_metrics' and 'tests' tables are not joined correctly in CommitLogFetcher.fetch_latest_for_platform
+        https://bugs.webkit.org/show_bug.cgi?id=199062
+
+        Reviewed by Ryosuke Niwa.
+
+        'test_metrics' and 'tests' should be joined based on 'metric_test' and 'test_id'.
+
+        * public/include/commit-log-fetcher.php: Fix the typo in the query.
+        * server-tests/api-commits-tests.js: Added a unit test for this change.
+
 2019-06-17  Dewei Zhu  <dewei_zhu@apple.com>
 
         Customizable test group form should allow user to supply a revision prefix of a commit and revision starts with 'r'.
index 540f1de..0fd6df8 100644 (file)
@@ -155,10 +155,10 @@ class CommitLogFetcher {
     function fetch_latest_for_platform($repository_id, $platform_id)
     {
         $query_result = $this->db->query_and_fetch_all("SELECT commits.* FROM test_runs, builds, build_commits, commits
-            WHERE run_build = build_id AND NOT EXISTS (SELECT * FROM build_requests WHERE request_build = build_id)
+            WHERE run_build = build_id AND NOT EXISTS (SELECT * FROM build_requests WHERE request_build = build_id LIMIT 1)
                 AND run_config IN (SELECT config_id FROM test_configurations
                     WHERE config_type = 'current' AND config_platform = $2 AND config_metric
-                        IN (SELECT metric_id FROM test_metrics, tests WHERE metric_id = test_id and test_parent IS NULL))
+                        IN (SELECT metric_id FROM test_metrics, tests WHERE metric_test = test_id and test_parent IS NULL))
                 AND run_build = build_id AND commit_build = build_id AND build_commit = commit_id AND commit_repository = $1
             ORDER BY build_time DESC LIMIT 1;", array($repository_id, $platform_id));
         /* This query is approximation. It finds the commit of the last build instead of the last commit.
index 98b02ef..8bdade2 100644 (file)
@@ -5,6 +5,7 @@ const assert = require('assert');
 const TestServer = require('./resources/test-server.js');
 const addSlaveForReport = require('./resources/common-operations.js').addSlaveForReport;
 const prepareServerTest = require('./resources/common-operations.js').prepareServerTest;
+const submitReport = require('./resources/common-operations.js').submitReport;
 
 describe("/api/commits/", function () {
     prepareServerTest(this);
@@ -80,6 +81,21 @@ describe("/api/commits/", function () {
         time: '2017-01-20T03:56:20.045Z'
     }
 
+    const report = [{
+        "buildNumber": "124",
+        "buildTime": "2015-10-27T15:34:51",
+        "builderName": "someBuilder",
+        "builderPassword": "somePassword",
+        "platform": "some platform",
+        "tests": {"Speedometer-2": {"metrics": {"Score": {"current": [[100]]}}}},
+        "revisions": {
+            "WebKit": {
+                "timestamp": "2017-01-20T02:52:34.577Z",
+                "revision": "210948"
+            }
+        }
+    }];
+
     function assertCommitIsSameAsOneSubmitted(commit, submitted)
     {
         assert.equal(commit['revision'], submitted['revision']);
@@ -238,6 +254,28 @@ describe("/api/commits/", function () {
                 assert.equal(result['commits'][0]['revision'], systemVersionCommits['commits'][0]['revision']);
             });
         });
+
+        it("should always return a commit as long as there is an existing 'current' type test run for a given platform", async () => {
+            const remote = TestServer.remoteAPI();
+            const db = TestServer.database();
+            await db.insert('tests', {name: 'A-Test'});
+            await submitReport(report);
+            await db.query(`DELETE FROM tests WHERE test_name = 'A-Test'`);
+
+            const platforms = await db.selectAll('platforms');
+            assert.equal(platforms.length, 1);
+
+            const test_metrics = await db.selectAll('test_metrics');
+            assert.equal(test_metrics.length, 1);
+
+            const tests = await db.selectAll('tests');
+            assert.equal(tests.length, 1);
+
+            assert(test_metrics[0].id != tests[0].id);
+
+            const response = await remote.getJSON(`/api/commits/WebKit/latest?platform=${platforms[0].id}`);
+            assert(response.commits.length);
+        });
     });
 
     describe('/api/commits/<repository>/last-reported', () => {