Extract the logic to merge tests from MergeTestsHandler and add unit …

…tests

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

Reviewed by Hajime Morita.

Extracted Test.merge and TestResult.replace_to_change_test_name out of MergeTestsHandler,
and moved MergeTestsHandler into admin_handlers.py where it belongs.

Added new backend "model-manipulator" to execute tasks to merge tests.

Also revive the inadvertently removed manual submission form on the admin page.

* Websites/webkit-perf.appspot.com/admin_handlers.py:
(AdminDashboardHandler.get_tests):
(MergeTestsHandler):
(MergeTestsHandler.post):
* Websites/webkit-perf.appspot.com/app.yaml:
* Websites/webkit-perf.appspot.com/backends.yaml: Added.
* Websites/webkit-perf.appspot.com/css/admin.css:
* Websites/webkit-perf.appspot.com/js/admin.js:
* Websites/webkit-perf.appspot.com/main.py:
* Websites/webkit-perf.appspot.com/merge_tests_handler.py: Removed.
* Websites/webkit-perf.appspot.com/models.py:
(Test):
(Test.merge):
(TestResult.replace_to_change_test_name):
* Websites/webkit-perf.appspot.com/models_unittest.py:
(DataStoreTestsBase.assertOnlyInstance):
(DataStoreTestsBase):
(DataStoreTestsBase.assertOnlyInstances):
(DataStoreTestsBase.assertEqualUnorderedModelList):
(DataStoreTestsBase.assertEqualUnorderedList):
(_create_build):
(TestModelTests.test_merge):
(TestResultTests):
(TestResultTests.test_get_or_insert_value):
(TestResultTests.test_get_or_insert_stat_value):
(TestResultTests.test_replace_to_change_test_name):
(TestResultTests.test_replace_to_change_test_name_with_stat_value):
(TestResultTests.test_replace_to_change_test_name_overrides_conflicting_result):


Canonical link: https://commits.webkit.org/96819@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@109057 268f45cc-cd09-0410-ab3c-d52691b4dbfc

ChangeLog

@@ -1,3 +1,46 @@
+2012-02-27  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Extract the logic to merge tests from MergeTestsHandler and add unit tests
+        https://bugs.webkit.org/show_bug.cgi?id=79602
+
+        Reviewed by Hajime Morita.
+
+        Extracted Test.merge and TestResult.replace_to_change_test_name out of MergeTestsHandler,
+        and moved MergeTestsHandler into admin_handlers.py where it belongs.
+
+        Added new backend "model-manipulator" to execute tasks to merge tests.
+
+        Also revive the inadvertently removed manual submission form on the admin page.
+
+        * Websites/webkit-perf.appspot.com/admin_handlers.py:
+        (AdminDashboardHandler.get_tests):
+        (MergeTestsHandler):
+        (MergeTestsHandler.post):
+        * Websites/webkit-perf.appspot.com/app.yaml:
+        * Websites/webkit-perf.appspot.com/backends.yaml: Added.
+        * Websites/webkit-perf.appspot.com/css/admin.css:
+        * Websites/webkit-perf.appspot.com/js/admin.js:
+        * Websites/webkit-perf.appspot.com/main.py:
+        * Websites/webkit-perf.appspot.com/merge_tests_handler.py: Removed.
+        * Websites/webkit-perf.appspot.com/models.py:
+        (Test):
+        (Test.merge):
+        (TestResult.replace_to_change_test_name):
+        * Websites/webkit-perf.appspot.com/models_unittest.py:
+        (DataStoreTestsBase.assertOnlyInstance):
+        (DataStoreTestsBase):
+        (DataStoreTestsBase.assertOnlyInstances):
+        (DataStoreTestsBase.assertEqualUnorderedModelList):
+        (DataStoreTestsBase.assertEqualUnorderedList):
+        (_create_build):
+        (TestModelTests.test_merge):
+        (TestResultTests):
+        (TestResultTests.test_get_or_insert_value):
+        (TestResultTests.test_get_or_insert_stat_value):
+        (TestResultTests.test_replace_to_change_test_name):
+        (TestResultTests.test_replace_to_change_test_name_with_stat_value):
+        (TestResultTests.test_replace_to_change_test_name_overrides_conflicting_result):
+
 2012-02-27  ChangSeok Oh  <shivamidow@gmail.com>
 
         [EFL] Support mutation observers

Websites/webkit-perf.appspot.com/admin_handlers.py

@@ -30,10 +30,14 @@
 import webapp2
 import json
 
+from google.appengine.api import taskqueue
 from google.appengine.api import users
 from google.appengine.ext.db import GqlQuery
 from google.appengine.ext.webapp import template
 
+from controller import schedule_runs_update
+from controller import schedule_dashboard_update
+from controller import schedule_manifest_update
 from models import Branch
 from models import Builder
 from models import Platform
@@ -81,3 +85,42 @@ def get_builders(self):
     def get_tests(self):
         self.response.headers['Content-Type'] = 'application/json'
         self.response.out.write(json.dumps([test.name for test in Test.all().fetch(limit=200)]))
+
+
+class MergeTestsHandler(webapp2.RequestHandler):
+    def post(self, task):
+        self.response.headers['Content-Type'] = 'text/plain; charset=utf-8'
+
+        if task != 'run':
+            try:
+                payload = json.loads(self.request.body)
+                merge = payload.get('merge', '')
+                into = payload.get('into', '')
+            except:
+                self.response.out.write("Failed to parse the payload: %s" % self.request.body)
+                return
+
+            if merge == into or not Test.get_by_key_name(merge) or not Test.get_by_key_name(into):
+                self.response.out.write('Invalid test names')
+                return
+
+            taskqueue.add(url='/admin/merge-tests/run', params={'merge': merge, 'into': into}, target='model-manipulator')
+            self.response.out.write('OK')
+            return
+
+        merge = Test.get_by_key_name(self.request.get('merge'))
+        into = Test.get_by_key_name(self.request.get('into'))
+
+        branches_and_platforms_to_update = into.merge(merge)
+        if branches_and_platforms_to_update == None:
+            # FIXME: This message is invisible. Need to store this somewhere and let the admin page pull it.
+            self.response.out.write('Cannot merge %s into %s. There are conflicting results.' % (merge.name, into.name))
+            return
+
+        for branch_id, platform_id in branches_and_platforms_to_update:
+            schedule_runs_update(into.id, branch_id, platform_id)
+
+        schedule_dashboard_update()
+        schedule_manifest_update()
+
+        self.response.out.write('OK')

Websites/webkit-perf.appspot.com/app.yaml

@@ -1,5 +1,5 @@
 application: webkit-perf
-version: 14
+version: 15
 runtime: python27
 api_version: 1
 threadsafe: false
@@ -9,12 +9,6 @@ handlers:
   static_files: favicon.ico
   upload: favicon\.ico
 
-- url: /admin/(.+\.html)
-  static_files: static/\1
-  upload: static
-  secure: always
-  login: admin
-
 - url: /
   static_files: index.html
   upload: index.html

Websites/webkit-perf.appspot.com/backends.yaml

@@ -0,0 +1,3 @@
+backends:
+- model-manipulator
+  options: dynamic

Websites/webkit-perf.appspot.com/css/admin.css

@@ -4,10 +4,13 @@ html {
     overflow: auto;
 }
 
-#summary {
-    margin-top: 49px;
+#summary, #manual-submission {
     font-size: 14px;
     line-height: 36px;
+}
+
+#summary {
+    margin-top: 49px;
     display: table;
     border-collapse: collapse;
 }
@@ -18,7 +21,6 @@ section {
     border: 1px solid lightgrey;
     border-collapse: collapse;
     min-height: 300px;
-    max-width: 400px;
 }
 
 h2 {
@@ -82,6 +84,15 @@ form dd {
     width: 150px;
 }
 
+#manual-submission form {
+    padding: 0;
+}
+#manual-submission textarea {
+    font-family: monospace;
+    font-size: 14px;
+    border: none;
+}
+
 #footer {
     margin: 10px;
 }

Websites/webkit-perf.appspot.com/js/admin.js

@@ -59,9 +59,15 @@ $('form').trigger('reload');
 $('form').bind('submit', function (event) {
     event.preventDefault();
 
-    var contents = {}
-    for (var i = 0; i < this.elements.length; i++)
-        contents[this.elements[i].name] = this.elements[i].value;
+    var payload;
+    if (this.payload)
+        payload = this.payload.value;
+    else {
+        var contents = {};
+        for (var i = 0; i < this.elements.length; i++)
+            contents[this.elements[i].name] = this.elements[i].value;
+        payload = JSON.stringify(contents);
+    }
 
     var xhr = new XMLHttpRequest;
     xhr.onreadystatechange = function () {
@@ -71,10 +77,24 @@ $('form').bind('submit', function (event) {
             error('HTTP status: ' + xhr.status);
         else if (xhr.responseText != 'OK')
             error(xhr.responseText);
-
     }
     xhr.open(this.method, this.action, true);
-    xhr.send(JSON.stringify(contents));
+    xhr.send(payload);
 
     $(this).trigger('reload');
 });
+
+$('#manual-submission textarea').val(JSON.stringify({
+    'branch': 'webkit-trunk',
+    'platform': 'chromium-mac',
+    'builder-name': 'Chromium Mac Release (Perf)',
+    'build-number': '123',
+    'timestamp': parseInt(Date.now() / 1000),
+    'webkit-revision': 104856,
+    'chromium-revision': 123059,
+    'results':
+        {
+            'webkit_style_test': {'avg': 100, 'median': 102, 'stdev': 5, 'min': 90, 'max': 110},
+            'some_test': 54,
+        },
+}, null, '  '));

Websites/webkit-perf.appspot.com/main.py

@@ -20,8 +20,9 @@
 
 import json
 
-from admin_handlers import IsAdminHandler
 from admin_handlers import AdminDashboardHandler
+from admin_handlers import IsAdminHandler
+from admin_handlers import MergeTestsHandler
 from controller import CachedDashboardHandler
 from controller import CachedManifestHandler
 from controller import CachedRunsHandler
@@ -33,11 +34,10 @@
 from report_handler import AdminReportHandler
 from report_process_handler import ReportProcessHandler
 from report_logs_handler import ReportLogsHandler
-from merge_tests_handler import MergeTestsHandler
 
 routes = [
     ('/admin/report/?', AdminReportHandler),
-    ('/admin/merge-tests/?', MergeTestsHandler),
+    (r'/admin/merge-tests(?:/(.*))?', MergeTestsHandler),
     ('/admin/report-logs/?', ReportLogsHandler),
     ('/admin/create/(.*)', CreateHandler),
     (r'/admin/([A-Za-z\-]*)', AdminDashboardHandler),

Websites/webkit-perf.appspot.com/models.py

@@ -142,6 +142,8 @@ def get_or_insert_from_log(log):
 class Test(db.Model):
     id = db.IntegerProperty(required=True)
     name = db.StringProperty(required=True)
+    # FIXME: Storing branches and platforms separately is flawed since a test maybe available on
+    # one platform but only on some branch and vice versa.
     branches = db.ListProperty(db.Key)
     platforms = db.ListProperty(db.Key)
 
@@ -170,6 +172,26 @@ def execute(id):
 
         return create_in_transaction_with_numeric_id_holder(execute) or existing_test[0]
 
+    def merge(self, other):
+        assert self.key() != other.key()
+
+        merged_results = TestResult.all()
+        merged_results.filter('name =', other.name)
+
+        # FIXME: We should be doing this check in a transaction but only ancestor queries are allowed
+        for result in merged_results:
+            if TestResult.get_by_key_name(TestResult.key_name(result.build, self.name)):
+                return None
+
+        branches_and_platforms_to_update = set()
+        for result in merged_results:
+            branches_and_platforms_to_update.add((result.build.branch.id, result.build.platform.id))
+            result.replace_to_change_test_name(self.name)
+
+        delete_model_with_numeric_id_holder(other)
+
+        return branches_and_platforms_to_update
+
 
 class TestResult(db.Model):
     name = db.StringProperty(required=True)
@@ -201,6 +223,13 @@ def _float_or_none(dictionary, key):
             valueMedian=_float_or_none(result, 'median'), valueStdev=_float_or_none(result, 'stdev'),
             valueMin=_float_or_none(result, 'min'), valueMax=_float_or_none(result, 'max'))
 
+    def replace_to_change_test_name(self, new_name):
+        clone = TestResult(key_name=TestResult.key_name(self.build, new_name), name=new_name, build=self.build,
+            value=self.value, valueMedian=self.valueMedian, valueStdev=self.valueMin, valueMin=self.valueMin, valueMax=self.valueMax)
+        clone.put()
+        self.delete()
+        return clone
+
 
 class ReportLog(db.Model):
     timestamp = db.DateTimeProperty(required=True)