Diff
Modified: trunk/ChangeLog (108093 => 108094)
--- trunk/ChangeLog 2012-02-17 18:27:32 UTC (rev 108093)
+++ trunk/ChangeLog 2012-02-17 18:47:05 UTC (rev 108094)
@@ -1,3 +1,83 @@
+2012-02-17 Ryosuke Niwa <[email protected]>
+
+ perf-o-matic needs model unit tests
+ https://bugs.webkit.org/show_bug.cgi?id=78885
+
+ Reviewed by Adam Barth.
+
+ Added unit tests for models.py.
+
+ Also renamed modelFromNumericId to model_from_numeric_id and moved set_persitent_cache and get_persistent_cache
+ from controller to PersistentCache.set_cache and PersistentCahce.set_cache respectively.
+
+ * Websites/webkit-perf.appspot.com/controller.py:
+ (cache_manifest):
+ (CachedManifestHandler.get):
+ (cache_dashboard):
+ (CachedDashboardHandler.get):
+ (cache_runs):
+ (CachedRunsHandler.get):
+ * Websites/webkit-perf.appspot.com/create_handler.py:
+ (CreateHandler._create_builder):
+ (CreateHandler._create_builder.execute):
+ * Websites/webkit-perf.appspot.com/models.py:
+ (create_in_transaction_with_numeric_id_holder):
+ (model_from_numeric_id):
+ (Builder):
+ (Builder.create):
+ (Builder.update_password):
+ (Builder._hashed_password):
+ (TestResult.key_name):
+ (ReportLog.get_value):
+ (ReportLog._integer_in_payload):
+ (ReportLog):
+ (ReportLog.timestamp):
+ (PersistentCache):
+ (PersistentCache.set_cache):
+ (PersistentCache.set_cache.execute):
+ (PersistentCache.get_cache):
+ * Websites/webkit-perf.appspot.com/models_unittest.py: Added.
+ (HelperTests):
+ (HelperTests.setUp):
+ (HelperTests.tearDown):
+ (HelperTests._assert_there_is_exactly_one_id_holder_and_matches):
+ (HelperTests.test_create_in_transaction_with_numeric_id_holder):
+ (HelperTests.test_create_in_transaction_with_numeric_id_holder.execute):
+ (HelperTests.test_failing_in_create_in_transaction_with_numeric_id_holder):
+ (HelperTests.test_failing_in_create_in_transaction_with_numeric_id_holder.execute):
+ (HelperTests.test_raising_in_create_in_transaction_with_numeric_id_holder):
+ (HelperTests.test_raising_in_create_in_transaction_with_numeric_id_holder.execute):
+ (HelperTests.test_delete_model_with_numeric_id_holder):
+ (HelperTests.test_delete_model_with_numeric_id_holder.execute):
+ (HelperTests.test_model_from_numeric_id):
+ (HelperTests.test_model_from_numeric_id.execute):
+ (BuilderTests):
+ (BuilderTests.setUp):
+ (BuilderTests.tearDown):
+ (BuilderTests.test_create):
+ (BuilderTests.test_update_password):
+ (BuilderTests.test_hashed_password):
+ (BuilderTests.test_authenticate):
+ (ReportLog):
+ (ReportLog.setUp):
+ (ReportLog.tearDown):
+ (ReportLog._create_log_with_payload):
+ (ReportLog.test_parsed_payload):
+ (ReportLog.test_get_value):
+ (ReportLog.test_results):
+ (ReportLog.test_builder):
+ (ReportLog.test_build_number):
+ (ReportLog.test_webkit_revision):
+ (ReportLog.chromium_revision):
+ (PersistentCacheTests):
+ (PersistentCacheTests.setUp):
+ (PersistentCacheTests.tearDown):
+ (PersistentCacheTests._assert_persistent_cache):
+ (PersistentCacheTests.test_set):
+ (PersistentCacheTests.test_get):
+ * Websites/webkit-perf.appspot.com/runs_handler.py:
+ (RunsHandler.post):
+
2012-02-17 Carlos Garcia Campos <[email protected]>
Unreviewed. Fix GTK+ build after r108003.
Modified: trunk/Websites/webkit-perf.appspot.com/controller.py (108093 => 108094)
--- trunk/Websites/webkit-perf.appspot.com/controller.py 2012-02-17 18:27:32 UTC (rev 108093)
+++ trunk/Websites/webkit-perf.appspot.com/controller.py 2012-02-17 18:47:05 UTC (rev 108094)
@@ -28,7 +28,6 @@
# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
import webapp2
-from google.appengine.api import memcache
from google.appengine.api import taskqueue
from google.appengine.ext import db
@@ -36,31 +35,8 @@
from models import PersistentCache
-def set_persistent_cache(name, value):
- memcache.set(name, value)
-
- def execute():
- cache = PersistentCache.get_by_key_name(name)
- if cache:
- cache.value = value
- cache.put()
- else:
- PersistentCache(key_name=name, value=value).put()
-
- db.run_in_transaction(execute)
-
-
-def get_persistent_cache(name):
- value = memcache.get(name)
- if value:
- return value
- cache = PersistentCache.get_by_key_name(name)
- memcache.set(name, cache.value)
- return cache.value
-
-
def cache_manifest(cache):
- set_persistent_cache('manifest', cache)
+ PersistentCache.set_cache('manifest', cache)
def schedule_manifest_update():
@@ -70,7 +46,7 @@
class CachedManifestHandler(webapp2.RequestHandler):
def get(self):
self.response.headers['Content-Type'] = 'application/json'
- manifest = get_persistent_cache('manifest')
+ manifest = PersistentCache.get_cache('manifest')
if manifest:
self.response.out.write(manifest)
else:
@@ -78,7 +54,7 @@
def cache_dashboard(cache):
- set_persistent_cache('dashboard', cache)
+ PersistentCache.set_cache('dashboard', cache)
def schedule_dashboard_update():
@@ -88,7 +64,7 @@
class CachedDashboardHandler(webapp2.RequestHandler):
def get(self):
self.response.headers['Content-Type'] = 'application/json'
- dashboard = get_persistent_cache('dashboard')
+ dashboard = PersistentCache.get_cache('dashboard')
if dashboard:
self.response.out.write(dashboard)
else:
@@ -96,7 +72,7 @@
def cache_runs(test_id, branch_id, platform_id, cache):
- set_persistent_cache(Test.cache_key(test_id, branch_id, platform_id), cache)
+ PersistentCache.set_cache(Test.cache_key(test_id, branch_id, platform_id), cache)
def schedule_runs_update(test_id, branch_id, platform_id):
@@ -117,7 +93,7 @@
branch_id = 0
platform_id = 0
- runs = get_persistent_cache(Test.cache_key(test_id, branch_id, platform_id))
+ runs = PersistentCache.get_cache(Test.cache_key(test_id, branch_id, platform_id))
if runs:
self.response.out.write(runs)
else:
Modified: trunk/Websites/webkit-perf.appspot.com/create_handler.py (108093 => 108094)
--- trunk/Websites/webkit-perf.appspot.com/create_handler.py 2012-02-17 18:27:32 UTC (rev 108093)
+++ trunk/Websites/webkit-perf.appspot.com/create_handler.py 2012-02-17 18:47:05 UTC (rev 108094)
@@ -70,17 +70,14 @@
if not name or not password:
return 'Invalid name or password'
- password = Builder.hashed_password(password)
-
def execute():
message = None
bot = Builder.get_by_key_name(name)
if bot:
message = 'Updating the password since bot "%s" already exists' % name
- bot.password = password
+ bot.update_password(password)
else:
- bot = Builder(name=name, password=password, key_name=name)
- bot.put()
+ Builder.create(name, password)
return message
return db.run_in_transaction(execute)
Modified: trunk/Websites/webkit-perf.appspot.com/models.py (108093 => 108094)
--- trunk/Websites/webkit-perf.appspot.com/models.py 2012-02-17 18:27:32 UTC (rev 108093)
+++ trunk/Websites/webkit-perf.appspot.com/models.py 2012-02-17 18:47:05 UTC (rev 108094)
@@ -33,6 +33,7 @@
from datetime import datetime
from google.appengine.ext import db
+from google.appengine.api import memcache
class NumericIdHolder(db.Model):
@@ -44,12 +45,15 @@
id_holder = NumericIdHolder()
id_holder.put()
id_holder = NumericIdHolder.get(id_holder.key())
- owner = db.run_in_transaction(callback, id_holder.key().id())
- if owner:
- id_holder.owner = owner
- id_holder.put()
- else:
- id_holder.delete()
+ owner = None
+ try:
+ owner = db.run_in_transaction(callback, id_holder.key().id())
+ if owner:
+ id_holder.owner = owner
+ id_holder.put()
+ finally:
+ if not owner:
+ id_holder.delete()
return owner
@@ -59,7 +63,7 @@
id_holder.delete()
-def modelFromNumericId(id, expected_kind):
+def model_from_numeric_id(id, expected_kind):
id_holder = NumericIdHolder.get_by_id(id)
return id_holder.owner if id_holder and id_holder.owner and isinstance(id_holder.owner, expected_kind) else None
@@ -78,11 +82,19 @@
name = db.StringProperty(required=True)
password = db.StringProperty(required=True)
+ @staticmethod
+ def create(name, raw_password):
+ return Builder(name=name, password=Builder._hashed_password(raw_password), key_name=name).put()
+
+ def update_password(self, raw_password):
+ self.password = Builder._hashed_password(raw_password)
+ self.put()
+
def authenticate(self, raw_password):
return self.password == hashlib.sha256(raw_password).hexdigest()
@staticmethod
- def hashed_password(raw_password):
+ def _hashed_password(raw_password):
return hashlib.sha256(raw_password).hexdigest()
@@ -122,7 +134,6 @@
return build.key().name() + ':' + test_name
-# Temporarily store log reports sent by bots
class ReportLog(db.Model):
timestamp = db.DateTimeProperty(required=True)
headers = db.TextProperty()
@@ -140,7 +151,7 @@
def get_value(self, keyName):
if not self._parsed_payload():
return None
- return self._parsed.get(keyName, '')
+ return self._parsed.get(keyName)
def results(self):
return self.get_value('results')
@@ -172,9 +183,12 @@
def _integer_in_payload(self, keyName):
try:
return int(self.get_value(keyName))
+ except TypeError:
+ return None
except ValueError:
return None
+ # FIXME: We also have timestamp as a member variable.
def timestamp(self):
try:
return datetime.fromtimestamp(self._integer_in_payload('timestamp'))
@@ -184,6 +198,30 @@
return None
-# Used when memcache entry is evicted
class PersistentCache(db.Model):
value = db.TextProperty(required=True)
+
+ @staticmethod
+ def set_cache(name, value):
+ memcache.set(name, value)
+
+ def execute():
+ cache = PersistentCache.get_by_key_name(name)
+ if cache:
+ cache.value = value
+ cache.put()
+ else:
+ PersistentCache(key_name=name, value=value).put()
+
+ db.run_in_transaction(execute)
+
+ @staticmethod
+ def get_cache(name):
+ value = memcache.get(name)
+ if value:
+ return value
+ cache = PersistentCache.get_by_key_name(name)
+ if not cache:
+ return None
+ memcache.set(name, cache.value)
+ return cache.value
Added: trunk/Websites/webkit-perf.appspot.com/models_unittest.py (0 => 108094)
--- trunk/Websites/webkit-perf.appspot.com/models_unittest.py (rev 0)
+++ trunk/Websites/webkit-perf.appspot.com/models_unittest.py 2012-02-17 18:47:05 UTC (rev 108094)
@@ -0,0 +1,272 @@
+#!/usr/bin/env python
+# Copyright (C) 2012 Google Inc. All rights reserved.
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions are
+# met:
+#
+# * Redistributions of source code must retain the above copyright
+# notice, this list of conditions and the following disclaimer.
+# * Redistributions in binary form must reproduce the above
+# copyright notice, this list of conditions and the following disclaimer
+# in the documentation and/or other materials provided with the
+# distribution.
+# * Neither the name of Google Inc. nor the names of its
+# contributors may be used to endorse or promote products derived from
+# this software without specific prior written permission.
+#
+# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+import models
+import unittest
+
+from datetime import datetime
+from google.appengine.api import memcache
+from google.appengine.ext import testbed
+
+
+class HelperTests(unittest.TestCase):
+ def setUp(self):
+ self.testbed = testbed.Testbed()
+ self.testbed.activate()
+ self.testbed.init_datastore_v3_stub()
+
+ def tearDown(self):
+ self.testbed.deactivate()
+
+ def _assert_there_is_exactly_one_id_holder_and_matches(self, id):
+ id_holders = models.NumericIdHolder.all().fetch(5)
+ self.assertEqual(len(id_holders), 1)
+ self.assertTrue(id_holders[0])
+ self.assertEqual(id_holders[0].key().id(), id)
+
+ def test_create_in_transaction_with_numeric_id_holder(self):
+
+ def execute(id):
+ return models.Branch(id=id, name='some branch', key_name='some-branch').put()
+
+ self.assertEqual(len(models.Branch.all().fetch(5)), 0)
+ self.assertEqual(len(models.NumericIdHolder.all().fetch(5)), 0)
+
+ self.assertTrue(models.create_in_transaction_with_numeric_id_holder(execute))
+
+ branches = models.Branch.all().fetch(5)
+ self.assertEqual(len(branches), 1)
+ self.assertEqual(branches[0].name, 'some branch')
+ self.assertEqual(branches[0].key().name(), 'some-branch')
+
+ self._assert_there_is_exactly_one_id_holder_and_matches(branches[0].id)
+
+ def test_failing_in_create_in_transaction_with_numeric_id_holder(self):
+
+ def execute(id):
+ return None
+
+ self.assertEqual(len(models.Branch.all().fetch(5)), 0)
+ self.assertEqual(len(models.NumericIdHolder.all().fetch(5)), 0)
+
+ self.assertFalse(models.create_in_transaction_with_numeric_id_holder(execute))
+
+ self.assertEqual(len(models.Branch.all().fetch(5)), 0)
+ self.assertEqual(len(models.NumericIdHolder.all().fetch(5)), 0)
+
+ def test_raising_in_create_in_transaction_with_numeric_id_holder(self):
+
+ def execute(id):
+ raise TypeError
+ return None
+
+ self.assertEqual(len(models.Branch.all().fetch(5)), 0)
+ self.assertEqual(len(models.NumericIdHolder.all().fetch(5)), 0)
+
+ self.assertRaises(TypeError, models.create_in_transaction_with_numeric_id_holder, (execute))
+
+ self.assertEqual(len(models.Branch.all().fetch(5)), 0)
+ self.assertEqual(len(models.NumericIdHolder.all().fetch(5)), 0)
+
+ def test_delete_model_with_numeric_id_holder(self):
+
+ def execute(id):
+ return models.Branch(id=id, name='some branch', key_name='some-branch').put()
+
+ branch = models.Branch.get(models.create_in_transaction_with_numeric_id_holder(execute))
+ self.assertEqual(len(models.NumericIdHolder.all().fetch(5)), 1)
+
+ models.delete_model_with_numeric_id_holder(branch)
+
+ self.assertEqual(len(models.Branch.all().fetch(5)), 0)
+ self.assertEqual(len(models.NumericIdHolder.all().fetch(5)), 0)
+
+ def test_model_from_numeric_id(self):
+
+ def execute(id):
+ return models.Branch(id=id, name='some branch', key_name='some-branch').put()
+
+ branch = models.Branch.get(models.create_in_transaction_with_numeric_id_holder(execute))
+
+ self.assertEqual(models.model_from_numeric_id(branch.id, models.Branch).key(), branch.key())
+ self.assertEqual(models.model_from_numeric_id(branch.id + 1, models.Branch), None)
+ models.delete_model_with_numeric_id_holder(branch)
+ self.assertEqual(models.model_from_numeric_id(branch.id, models.Branch), None)
+
+
+class BuilderTests(unittest.TestCase):
+ def setUp(self):
+ self.testbed = testbed.Testbed()
+ self.testbed.activate()
+ self.testbed.init_datastore_v3_stub()
+
+ def tearDown(self):
+ self.testbed.deactivate()
+
+ def test_create(self):
+ builder_key = models.Builder.create('some builder', 'some password')
+ self.assertTrue(builder_key)
+ builder = models.Builder.get(builder_key)
+ self.assertEqual(builder.key().name(), 'some builder')
+ self.assertEqual(builder.name, 'some builder')
+ self.assertEqual(builder.password, models.Builder._hashed_password('some password'))
+
+ def test_update_password(self):
+ builder = models.Builder.get(models.Builder.create('some builder', 'some password'))
+ self.assertEqual(builder.password, models.Builder._hashed_password('some password'))
+ builder.update_password('other password')
+ self.assertEqual(builder.password, models.Builder._hashed_password('other password'))
+
+ # Make sure it's saved
+ builder = models.Builder.get(builder.key())
+ self.assertEqual(builder.password, models.Builder._hashed_password('other password'))
+
+ def test_hashed_password(self):
+ self.assertNotEqual(models.Builder._hashed_password('some password'), 'some password')
+ self.assertFalse('some password' in models.Builder._hashed_password('some password'))
+ self.assertEqual(len(models.Builder._hashed_password('some password')), 64)
+
+ def test_authenticate(self):
+ builder = models.Builder.get(models.Builder.create('some builder', 'some password'))
+ self.assertTrue(builder.authenticate('some password'))
+ self.assertFalse(builder.authenticate('bad password'))
+
+
+class ReportLog(unittest.TestCase):
+ def setUp(self):
+ self.testbed = testbed.Testbed()
+ self.testbed.activate()
+ self.testbed.init_datastore_v3_stub()
+
+ def tearDown(self):
+ self.testbed.deactivate()
+
+ def _create_log_with_payload(self, payload):
+ return models.ReportLog(timestamp=datetime.now(), headers='some headers', payload=payload)
+
+ def test_parsed_payload(self):
+ log = self._create_log_with_payload('')
+ self.assertFalse('_parsed' in log.__dict__)
+ self.assertEqual(log._parsed_payload(), False)
+ self.assertEqual(log._parsed, False)
+
+ log = self._create_log_with_payload('{"key": "value", "another key": 1}')
+ self.assertEqual(log._parsed_payload(), {"key": "value", "another key": 1})
+ self.assertEqual(log._parsed, {"key": "value", "another key": 1})
+
+ def test_get_value(self):
+ log = self._create_log_with_payload('{"string": "value", "integer": 1, "float": 1.1}')
+ self.assertEqual(log.get_value('string'), 'value')
+ self.assertEqual(log.get_value('integer'), 1)
+ self.assertEqual(log.get_value('float'), 1.1)
+ self.assertEqual(log.get_value('bad'), None)
+
+ def test_results(self):
+ log = self._create_log_with_payload('{"results": 123}')
+ self.assertEqual(log.results(), 123)
+
+ log = self._create_log_with_payload('{"key": "value"}')
+ self.assertEqual(log.results(), None)
+
+ def test_builder(self):
+ log = self._create_log_with_payload('{"key": "value"}')
+ self.assertEqual(log.builder(), None)
+
+ builder_name = "Chromium Mac Release (Perf)"
+ log = self._create_log_with_payload('{"builder-name": "%s"}' % builder_name)
+ self.assertEqual(log.builder(), None)
+
+ builder_key = models.Builder.create(builder_name, 'some password')
+ log = self._create_log_with_payload('{"builder-name": "%s"}' % builder_name)
+ self.assertEqual(log.builder().key(), builder_key)
+
+ # FIXME test_branch and test_platform
+
+ def test_build_number(self):
+ log = self._create_log_with_payload('{"build-number": 123}')
+ self.assertEqual(log.build_number(), 123)
+
+ log = self._create_log_with_payload('{"key": "value"}')
+ self.assertEqual(log.build_number(), None)
+
+ def test_webkit_revision(self):
+ log = self._create_log_with_payload('{"key": "value"}')
+ self.assertEqual(log.webkit_revision(), None)
+
+ log = self._create_log_with_payload('{"webkit-revision": 123}')
+ self.assertEqual(log.webkit_revision(), 123)
+
+ def chromium_revision(self):
+ log = self._create_log_with_payload('{"chromium-revision": 123}')
+ self.assertEqual(log.webkit_revision(), 123)
+
+ log = self._create_log_with_payload('{"key": "value"}')
+ self.assertEqual(log.webkit_revision(), None)
+
+
+class PersistentCacheTests(unittest.TestCase):
+ def setUp(self):
+ self.testbed = testbed.Testbed()
+ self.testbed.activate()
+ self.testbed.init_datastore_v3_stub()
+ self.testbed.init_memcache_stub()
+
+ def tearDown(self):
+ self.testbed.deactivate()
+
+ def _assert_persistent_cache(self, name, value):
+ self.assertEqual(models.PersistentCache.get_by_key_name(name).value, value)
+ self.assertEqual(memcache.get(name), value)
+
+ def test_set(self):
+ self.assertEqual(len(models.PersistentCache.all().fetch(5)), 0)
+
+ models.PersistentCache.set_cache('some-cache', 'some data')
+ self._assert_persistent_cache('some-cache', 'some data')
+
+ models.PersistentCache.set_cache('some-cache', 'some other data')
+
+ self._assert_persistent_cache('some-cache', 'some other data')
+
+ def test_get(self):
+ self.assertEqual(memcache.get('some-cache'), None)
+ self.assertEqual(models.PersistentCache.get_cache('some-cache'), None)
+
+ models.PersistentCache.set_cache('some-cache', 'some data')
+
+ self.assertEqual(memcache.get('some-cache'), 'some data')
+ self.assertEqual(models.PersistentCache.get_cache('some-cache'), 'some data')
+
+ memcache.delete('some-cache')
+ self.assertEqual(memcache.get('some-cache'), None)
+ self.assertEqual(models.PersistentCache.get_cache('some-cache'), 'some data')
+
+
+if __name__ == '__main__':
+ unittest.main()
Modified: trunk/Websites/webkit-perf.appspot.com/runs_handler.py (108093 => 108094)
--- trunk/Websites/webkit-perf.appspot.com/runs_handler.py 2012-02-17 18:27:32 UTC (rev 108093)
+++ trunk/Websites/webkit-perf.appspot.com/runs_handler.py 2012-02-17 18:47:05 UTC (rev 108094)
@@ -41,7 +41,7 @@
from models import Platform
from models import Test
from models import TestResult
-from models import modelFromNumericId
+from models import model_from_numeric_id
class RunsHandler(webapp2.RequestHandler):
@@ -62,10 +62,10 @@
# days = self.request.get('days', 365)
builds = Build.all()
- builds.filter('branch =', modelFromNumericId(branch_id, Branch))
- builds.filter('platform =', modelFromNumericId(platform_id, Platform))
+ builds.filter('branch =', model_from_numeric_id(branch_id, Branch))
+ builds.filter('platform =', model_from_numeric_id(platform_id, Platform))
- test = modelFromNumericId(test_id, Test)
+ test = model_from_numeric_id(test_id, Test)
test_name = test.name if test else None
test_runs = []
averages = {}
