Perf dashboard can't merge when the destination platform is missing baseline/target

https://bugs.webkit.org/show_bug.cgi?id=152286

Reviewed by Stephanie Lewis.

The bug was caused by the query to migrate test configurations to new platform checking
configuration type and metric separately; that is, it assumes the configuration exists
only if either the same type or the same metric exists in the destination.

Fixed the bug by checking both conditions simultaneously for each configuration.

* public/admin/platforms.php:
* tests/admin-platforms.js: Added a test.

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

diff --git a/Websites/perf.webkit.org/ChangeLog b/Websites/perf.webkit.org/ChangeLog
index 3192896..c446245 100644 (file)
--- a/Websites/perf.webkit.org/ChangeLog
+++ b/Websites/perf.webkit.org/ChangeLog
@@ -1,3 +1,19 @@
+2015-12-14  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Perf dashboard can't merge when the destination platform is missing baseline/target
+        https://bugs.webkit.org/show_bug.cgi?id=152286
+
+        Reviewed by Stephanie Lewis.
+
+        The bug was caused by the query to migrate test configurations to new platform checking
+        configuration type and metric separately; that is, it assumes the configuration exists
+        only if either the same type or the same metric exists in the destination.
+
+        Fixed the bug by checking both conditions simultaneously for each configuration.
+
+        * public/admin/platforms.php:
+        * tests/admin-platforms.js: Added a test.
+
 2015-12-11  Ryosuke Niwa  <rniwa@webkit.org>
 
         Perf dashboard's buildbot sync config JSON duplicates too much information

diff --git a/Websites/perf.webkit.org/public/admin/platforms.php b/Websites/perf.webkit.org/public/admin/platforms.php
index 1bfc19d..5c0b9bf 100644 (file)
--- a/Websites/perf.webkit.org/public/admin/platforms.php
+++ b/Websites/perf.webkit.org/public/admin/platforms.php
@@ -21,9 +21,11 @@ function merge_platforms($platform_to_merge, $destination_platform) {
 
     // Then migrate test configurations that don't exist in the destination platform to the new platform
     // so that test runs associated with those configurations are moved to the destination.
-    if ($db->query_and_get_affected_rows('UPDATE test_configurations SET config_platform = $2
-        WHERE config_platform = $1 AND (config_metric NOT IN (SELECT config_metric FROM test_configurations WHERE config_platform = $2)
-            OR config_type NOT IN (SELECT config_type FROM test_configurations WHERE config_platform = $2))',
+    if ($db->query_and_get_affected_rows('UPDATE test_configurations as merged SET config_platform = $2
+        WHERE config_platform = $1 AND NOT EXISTS (SELECT * FROM test_configurations as destination WHERE
+            merged.config_platform = $1 AND destination.config_platform = $2
+                AND destination.config_type = merged.config_type
+                AND destination.config_metric = merged.config_metric)',
         array($platform_to_merge, $destination_platform)) === FALSE) {
         $db->rollback_transaction();
         return notice("Failed to migrate test configurations for $platform_to_merge.");

diff --git a/Websites/perf.webkit.org/tests/admin-platforms.js b/Websites/perf.webkit.org/tests/admin-platforms.js
index e321b48..8d0ba2c 100644 (file)
--- a/Websites/perf.webkit.org/tests/admin-platforms.js
+++ b/Websites/perf.webkit.org/tests/admin-platforms.js
@@ -89,4 +89,34 @@ describe("/admin/platforms", function () {
         });
     });
 
+    it("should move test configurations from the merged platform to the destination platform", function () {
+        reportsForDifferentPlatforms[0]['tests'] = {"test": { "metrics": {"FrameRate": { "baseline": [[1, 1, 1], [1, 1, 1]] } } } };
+        submitReport(reportsForDifferentPlatforms, function () {
+            var queryForConfig = 'SELECT * from test_configurations, platforms, test_metrics'
+                + ' where config_platform = platform_id and config_metric = metric_id and platform_name in ($1, $2) order by config_id';
+            queryAndFetchAll(queryForConfig, [reportsForDifferentPlatforms[0]['platform'], reportsForDifferentPlatforms[2]['platform']], function (configs) {
+                assert.equal(configs.length, 2);
+                assert.equal(configs[0]['platform_name'], reportsForDifferentPlatforms[0]['platform']);
+                assert.equal(configs[0]['metric_name'], 'FrameRate');
+                assert.equal(configs[0]['config_type'], 'baseline');
+                assert.equal(configs[1]['platform_name'], reportsForDifferentPlatforms[2]['platform']);
+                assert.equal(configs[1]['metric_name'], 'FrameRate');
+                assert.equal(configs[1]['config_type'], 'current');
+                httpPost('/admin/platforms.php', {'action': 'merge', 'id': configs[0]['platform_id'], 'destination': configs[1]['platform_id']}, function (response) {
+                    assert.equal(response.statusCode, 200);
+                    queryAndFetchAll(queryForConfig, [reportsForDifferentPlatforms[0]['platform'], reportsForDifferentPlatforms[2]['platform']], function (newConfigs) {
+                        assert.equal(newConfigs.length, 2);
+                        assert.equal(newConfigs[0]['platform_name'], reportsForDifferentPlatforms[2]['platform']);
+                        assert.equal(newConfigs[0]['metric_name'], 'FrameRate');
+                        assert.equal(newConfigs[0]['config_type'], 'baseline');
+                        assert.equal(newConfigs[1]['platform_name'], reportsForDifferentPlatforms[2]['platform']);
+                        assert.equal(newConfigs[1]['metric_name'], 'FrameRate');
+                        assert.equal(newConfigs[1]['config_type'], 'current');
+                        notifyDone();
+                    });
+                });
+            });
+        });
+    });
+
 });