Changeset 109057 in webkit


Ignore:
Timestamp:
Feb 27, 2012 6:14:37 PM (12 years ago)
Author:
rniwa@webkit.org
Message:

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):

Location:
trunk
Files:
1 added
1 deleted
8 edited

Legend:

Unmodified
Added
Removed
  • trunk/ChangeLog

    r109039 r109057  
     12012-02-27  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        Extract the logic to merge tests from MergeTestsHandler and add unit tests
     4        https://bugs.webkit.org/show_bug.cgi?id=79602
     5
     6        Reviewed by Hajime Morita.
     7
     8        Extracted Test.merge and TestResult.replace_to_change_test_name out of MergeTestsHandler,
     9        and moved MergeTestsHandler into admin_handlers.py where it belongs.
     10
     11        Added new backend "model-manipulator" to execute tasks to merge tests.
     12
     13        Also revive the inadvertently removed manual submission form on the admin page.
     14
     15        * Websites/webkit-perf.appspot.com/admin_handlers.py:
     16        (AdminDashboardHandler.get_tests):
     17        (MergeTestsHandler):
     18        (MergeTestsHandler.post):
     19        * Websites/webkit-perf.appspot.com/app.yaml:
     20        * Websites/webkit-perf.appspot.com/backends.yaml: Added.
     21        * Websites/webkit-perf.appspot.com/css/admin.css:
     22        * Websites/webkit-perf.appspot.com/js/admin.js:
     23        * Websites/webkit-perf.appspot.com/main.py:
     24        * Websites/webkit-perf.appspot.com/merge_tests_handler.py: Removed.
     25        * Websites/webkit-perf.appspot.com/models.py:
     26        (Test):
     27        (Test.merge):
     28        (TestResult.replace_to_change_test_name):
     29        * Websites/webkit-perf.appspot.com/models_unittest.py:
     30        (DataStoreTestsBase.assertOnlyInstance):
     31        (DataStoreTestsBase):
     32        (DataStoreTestsBase.assertOnlyInstances):
     33        (DataStoreTestsBase.assertEqualUnorderedModelList):
     34        (DataStoreTestsBase.assertEqualUnorderedList):
     35        (_create_build):
     36        (TestModelTests.test_merge):
     37        (TestResultTests):
     38        (TestResultTests.test_get_or_insert_value):
     39        (TestResultTests.test_get_or_insert_stat_value):
     40        (TestResultTests.test_replace_to_change_test_name):
     41        (TestResultTests.test_replace_to_change_test_name_with_stat_value):
     42        (TestResultTests.test_replace_to_change_test_name_overrides_conflicting_result):
     43
    1442012-02-27  ChangSeok Oh  <shivamidow@gmail.com>
    245
  • trunk/Websites/webkit-perf.appspot.com/admin_handlers.py

    r108917 r109057  
    3131import json
    3232
     33from google.appengine.api import taskqueue
    3334from google.appengine.api import users
    3435from google.appengine.ext.db import GqlQuery
    3536from google.appengine.ext.webapp import template
    3637
     38from controller import schedule_runs_update
     39from controller import schedule_dashboard_update
     40from controller import schedule_manifest_update
    3741from models import Branch
    3842from models import Builder
     
    8286        self.response.headers['Content-Type'] = 'application/json'
    8387        self.response.out.write(json.dumps([test.name for test in Test.all().fetch(limit=200)]))
     88
     89
     90class MergeTestsHandler(webapp2.RequestHandler):
     91    def post(self, task):
     92        self.response.headers['Content-Type'] = 'text/plain; charset=utf-8'
     93
     94        if task != 'run':
     95            try:
     96                payload = json.loads(self.request.body)
     97                merge = payload.get('merge', '')
     98                into = payload.get('into', '')
     99            except:
     100                self.response.out.write("Failed to parse the payload: %s" % self.request.body)
     101                return
     102
     103            if merge == into or not Test.get_by_key_name(merge) or not Test.get_by_key_name(into):
     104                self.response.out.write('Invalid test names')
     105                return
     106
     107            taskqueue.add(url='/admin/merge-tests/run', params={'merge': merge, 'into': into}, target='model-manipulator')
     108            self.response.out.write('OK')
     109            return
     110
     111        merge = Test.get_by_key_name(self.request.get('merge'))
     112        into = Test.get_by_key_name(self.request.get('into'))
     113
     114        branches_and_platforms_to_update = into.merge(merge)
     115        if branches_and_platforms_to_update == None:
     116            # FIXME: This message is invisible. Need to store this somewhere and let the admin page pull it.
     117            self.response.out.write('Cannot merge %s into %s. There are conflicting results.' % (merge.name, into.name))
     118            return
     119
     120        for branch_id, platform_id in branches_and_platforms_to_update:
     121            schedule_runs_update(into.id, branch_id, platform_id)
     122
     123        schedule_dashboard_update()
     124        schedule_manifest_update()
     125
     126        self.response.out.write('OK')
  • trunk/Websites/webkit-perf.appspot.com/app.yaml

    r108845 r109057  
    11application: webkit-perf
    2 version: 14
     2version: 15
    33runtime: python27
    44api_version: 1
     
    99  static_files: favicon.ico
    1010  upload: favicon\.ico
    11 
    12 - url: /admin/(.+\.html)
    13   static_files: static/\1
    14   upload: static
    15   secure: always
    16   login: admin
    1711
    1812- url: /
  • trunk/Websites/webkit-perf.appspot.com/css/admin.css

    r108921 r109057  
    55}
    66
     7#summary, #manual-submission {
     8    font-size: 14px;
     9    line-height: 36px;
     10}
     11
    712#summary {
    813    margin-top: 49px;
    9     font-size: 14px;
    10     line-height: 36px;
    1114    display: table;
    1215    border-collapse: collapse;
     
    1922    border-collapse: collapse;
    2023    min-height: 300px;
    21     max-width: 400px;
    2224}
    2325
     
    8385}
    8486
     87#manual-submission form {
     88    padding: 0;
     89}
     90#manual-submission textarea {
     91    font-family: monospace;
     92    font-size: 14px;
     93    border: none;
     94}
     95
    8596#footer {
    8697    margin: 10px;
  • trunk/Websites/webkit-perf.appspot.com/js/admin.js

    r108917 r109057  
    6060    event.preventDefault();
    6161
    62     var contents = {}
    63     for (var i = 0; i < this.elements.length; i++)
    64         contents[this.elements[i].name] = this.elements[i].value;
     62    var payload;
     63    if (this.payload)
     64        payload = this.payload.value;
     65    else {
     66        var contents = {};
     67        for (var i = 0; i < this.elements.length; i++)
     68            contents[this.elements[i].name] = this.elements[i].value;
     69        payload = JSON.stringify(contents);
     70    }
    6571
    6672    var xhr = new XMLHttpRequest;
     
    7278        else if (xhr.responseText != 'OK')
    7379            error(xhr.responseText);
    74        
    7580    }
    7681    xhr.open(this.method, this.action, true);
    77     xhr.send(JSON.stringify(contents));
     82    xhr.send(payload);
    7883
    7984    $(this).trigger('reload');
    8085});
     86
     87$('#manual-submission textarea').val(JSON.stringify({
     88    'branch': 'webkit-trunk',
     89    'platform': 'chromium-mac',
     90    'builder-name': 'Chromium Mac Release (Perf)',
     91    'build-number': '123',
     92    'timestamp': parseInt(Date.now() / 1000),
     93    'webkit-revision': 104856,
     94    'chromium-revision': 123059,
     95    'results':
     96        {
     97            'webkit_style_test': {'avg': 100, 'median': 102, 'stdev': 5, 'min': 90, 'max': 110},
     98            'some_test': 54,
     99        },
     100}, null, '  '));
  • trunk/Websites/webkit-perf.appspot.com/main.py

    r108917 r109057  
    2121import json
    2222
     23from admin_handlers import AdminDashboardHandler
    2324from admin_handlers import IsAdminHandler
    24 from admin_handlers import AdminDashboardHandler
     25from admin_handlers import MergeTestsHandler
    2526from controller import CachedDashboardHandler
    2627from controller import CachedManifestHandler
     
    3435from report_process_handler import ReportProcessHandler
    3536from report_logs_handler import ReportLogsHandler
    36 from merge_tests_handler import MergeTestsHandler
    3737
    3838routes = [
    3939    ('/admin/report/?', AdminReportHandler),
    40     ('/admin/merge-tests/?', MergeTestsHandler),
     40    (r'/admin/merge-tests(?:/(.*))?', MergeTestsHandler),
    4141    ('/admin/report-logs/?', ReportLogsHandler),
    4242    ('/admin/create/(.*)', CreateHandler),
  • trunk/Websites/webkit-perf.appspot.com/models.py

    r108399 r109057  
    143143    id = db.IntegerProperty(required=True)
    144144    name = db.StringProperty(required=True)
     145    # FIXME: Storing branches and platforms separately is flawed since a test maybe available on
     146    # one platform but only on some branch and vice versa.
    145147    branches = db.ListProperty(db.Key)
    146148    platforms = db.ListProperty(db.Key)
     
    171173        return create_in_transaction_with_numeric_id_holder(execute) or existing_test[0]
    172174
     175    def merge(self, other):
     176        assert self.key() != other.key()
     177
     178        merged_results = TestResult.all()
     179        merged_results.filter('name =', other.name)
     180
     181        # FIXME: We should be doing this check in a transaction but only ancestor queries are allowed
     182        for result in merged_results:
     183            if TestResult.get_by_key_name(TestResult.key_name(result.build, self.name)):
     184                return None
     185
     186        branches_and_platforms_to_update = set()
     187        for result in merged_results:
     188            branches_and_platforms_to_update.add((result.build.branch.id, result.build.platform.id))
     189            result.replace_to_change_test_name(self.name)
     190
     191        delete_model_with_numeric_id_holder(other)
     192
     193        return branches_and_platforms_to_update
     194
    173195
    174196class TestResult(db.Model):
     
    201223            valueMedian=_float_or_none(result, 'median'), valueStdev=_float_or_none(result, 'stdev'),
    202224            valueMin=_float_or_none(result, 'min'), valueMax=_float_or_none(result, 'max'))
     225
     226    def replace_to_change_test_name(self, new_name):
     227        clone = TestResult(key_name=TestResult.key_name(self.build, new_name), name=new_name, build=self.build,
     228            value=self.value, valueMedian=self.valueMedian, valueStdev=self.valueMin, valueMin=self.valueMin, valueMax=self.valueMax)
     229        clone.put()
     230        self.delete()
     231        return clone
    203232
    204233
  • trunk/Websites/webkit-perf.appspot.com/models_unittest.py

    r108399 r109057  
    5050
    5151    def assertOnlyInstance(self, only_instasnce):
    52         self.assertEqual(len(only_instasnce.__class__.all().fetch(5)), 1)
    53         self.assertTrue(only_instasnce.__class__.get(only_instasnce.key()))
     52        self.assertOnlyInstances([only_instasnce])
     53
     54    def assertOnlyInstances(self, expected_instances):
     55        actual_instances = expected_instances[0].__class__.all().fetch(len(expected_instances) + 1)
     56        self.assertEqualUnorderedModelList(actual_instances, expected_instances)
     57
     58    def assertEqualUnorderedModelList(self, list1, list2):
     59        self.assertEqualUnorderedList([item.key() for item in list1], [item.key() for item in list1])
    5460
    5561    def assertEqualUnorderedList(self, list1, list2):
    5662        self.assertEqual(set(list1), set(list2))
     63        self.assertEqual(len(list1), len(list2))
    5764
    5865
     
    199206    builder_key = models.Builder.create('some-builder', 'Some Builder')
    200207    return branch, platform, models.Builder.get(builder_key)
     208
     209
     210def _create_build(branch, platform, builder, key_name='some-build'):
     211    build_key = models.Build(key_name=key_name, branch=branch, platform=platform, builder=builder,
     212        buildNumber=1, revision=100, timestamp=datetime.now()).put()
     213    return models.Build.get(build_key)
    201214
    202215
     
    255268        self.assertEqualUnorderedList(test.platforms, [platform.key(), other_platform.key()])
    256269
     270    def test_merge(self):
     271        branch, platform, builder = _create_some_builder()
     272        some_build = _create_build(branch, platform, builder)
     273        some_result = models.TestResult.get_or_insert_from_parsed_json('some-test', some_build, 50)
     274        some_test = models.Test.update_or_insert('some-test', branch, platform)
     275
     276        other_build = _create_build(branch, platform, builder, 'other-build')
     277        other_result = models.TestResult.get_or_insert_from_parsed_json('other-test', other_build, 30)
     278        other_test = models.Test.update_or_insert('other-test', branch, platform)
     279
     280        self.assertOnlyInstances([some_result, other_result])
     281        self.assertNotEqual(some_result.key(), other_result.key())
     282        self.assertOnlyInstances([some_test, other_test])
     283
     284        self.assertRaises(AssertionError, some_test.merge, (some_test))
     285        self.assertOnlyInstances([some_test, other_test])
     286
     287        some_test.merge(other_test)
     288        results_for_some_test = models.TestResult.all()
     289        results_for_some_test.filter('name =', 'some-test')
     290        results_for_some_test = results_for_some_test.fetch(5)
     291        self.assertEqual(len(results_for_some_test), 2)
     292
     293        self.assertEqual(results_for_some_test[0].name, 'some-test')
     294        self.assertEqual(results_for_some_test[1].name, 'some-test')
     295
     296        if results_for_some_test[0].value == 50:
     297            self.assertEqual(results_for_some_test[1].value, 30)
     298        else:
     299            self.assertEqual(results_for_some_test[1].value, 50)
     300
    257301
    258302class TestResultTests(DataStoreTestsBase):
    259     def _create_build(self):
    260         branch, platform, builder = _create_some_builder()
    261         build_key = models.Build(key_name='some-build', branch=branch, platform=platform, builder=builder,
    262             buildNumber=1, revision=100, timestamp=datetime.now()).put()
    263         return models.Build.get(build_key)
    264 
    265303    def test_get_or_insert_value(self):
    266         build = self._create_build()
     304        branch, platform, builder = _create_some_builder()
     305        build = _create_build(branch, platform, builder)
    267306        self.assertThereIsNoInstanceOf(models.TestResult)
    268307        result = models.TestResult.get_or_insert_from_parsed_json('some-test', build, 50)
     
    277316
    278317    def test_get_or_insert_stat_value(self):
    279         build = self._create_build()
     318        branch, platform, builder = _create_some_builder()
     319        build = _create_build(branch, platform, builder)
    280320        self.assertThereIsNoInstanceOf(models.TestResult)
    281321        result = models.TestResult.get_or_insert_from_parsed_json('some-test', build,
     
    290330        self.assertEqual(result.valueMax, 45)
    291331
     332    def test_replace_to_change_test_name(self):
     333        branch, platform, builder = _create_some_builder()
     334        build = _create_build(branch, platform, builder)
     335        self.assertThereIsNoInstanceOf(models.TestResult)
     336        result = models.TestResult.get_or_insert_from_parsed_json('some-test', build, 50)
     337        self.assertOnlyInstance(result)
     338        self.assertEqual(result.name, 'some-test')
     339
     340        new_result = result.replace_to_change_test_name('other-test')
     341        self.assertNotEqual(result, new_result)
     342        self.assertOnlyInstance(new_result)
     343
     344        self.assertEqual(new_result.name, 'other-test')
     345        self.assertEqual(new_result.build.key(), result.build.key())
     346        self.assertEqual(new_result.value, result.value)
     347        self.assertEqual(new_result.valueMedian, None)
     348        self.assertEqual(new_result.valueStdev, None)
     349        self.assertEqual(new_result.valueMin, None)
     350        self.assertEqual(new_result.valueMax, None)
     351
     352    def test_replace_to_change_test_name_with_stat_value(self):
     353        branch, platform, builder = _create_some_builder()
     354        build = _create_build(branch, platform, builder)
     355        self.assertThereIsNoInstanceOf(models.TestResult)
     356        result = models.TestResult.get_or_insert_from_parsed_json('some-test', build,
     357            {"avg": 40, "median": "40.1", "stdev": 3.25, "min": 30.5, "max": 45})
     358        self.assertOnlyInstance(result)
     359        self.assertEqual(result.name, 'some-test')
     360
     361        new_result = result.replace_to_change_test_name('other-test')
     362        self.assertNotEqual(result, new_result)
     363        self.assertOnlyInstance(new_result)
     364
     365        self.assertEqual(new_result.name, 'other-test')
     366        self.assertEqual(new_result.build.key(), result.build.key())
     367        self.assertEqual(new_result.value, result.value)
     368        self.assertEqual(result.value, 40.0)
     369        self.assertEqual(result.valueMedian, 40.1)
     370        self.assertEqual(result.valueStdev, 3.25)
     371        self.assertEqual(result.valueMin, 30.5)
     372        self.assertEqual(result.valueMax, 45)
     373
     374    def test_replace_to_change_test_name_overrides_conflicting_result(self):
     375        branch, platform, builder = _create_some_builder()
     376        build = _create_build(branch, platform, builder)
     377        self.assertThereIsNoInstanceOf(models.TestResult)
     378        result = models.TestResult.get_or_insert_from_parsed_json('some-test', build, 20)
     379        self.assertOnlyInstance(result)
     380
     381        conflicting_result = models.TestResult.get_or_insert_from_parsed_json('other-test', build, 10)
     382
     383        new_result = result.replace_to_change_test_name('other-test')
     384        self.assertNotEqual(result, new_result)
     385        self.assertOnlyInstance(new_result)
     386
     387        self.assertEqual(new_result.name, 'other-test')
     388        self.assertEqual(models.TestResult.get(conflicting_result.key()).value, 20)
     389
    292390
    293391class ReportLogTests(DataStoreTestsBase):
Note: See TracChangeset for help on using the changeset viewer.