Move more logic from handler classes to model classes and add unit tests
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 21 Feb 2012 00:25:47 +0000 (00:25 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 21 Feb 2012 00:25:47 +0000 (00:25 +0000)
https://bugs.webkit.org/show_bug.cgi?id=78989

Reviewed by Hajime Morita.

Extracted various functions from CreateHandler, ReportHanlder, and RunsHanlder to model classes
in order to unit-test them, added DataStoreTestsBase to reduce the code duplication in tests,
and added a whole bunch of unit tests in models_unittest.py.

* Websites/webkit-perf.appspot.com/create_handler.py:
(CreateHandler._create_branch):
(CreateHandler._create_platform):
* Websites/webkit-perf.appspot.com/models.py:
(_create_if_possible):
(_create_if_possible.execute):
(Branch):
(Branch.create_if_possible):
(Platform):
(Platform.create_if_possible):
(Build):
(Build.get_or_insert_from_log):
(Test):
(Test.update_or_insert):
(Test.update_or_insert.execute):
(TestResult):
(TestResult.get_or_insert_from_parsed_json):
(TestResult.get_or_insert_from_parsed_json._float_or_none):
(TestResult.generate_runs):
* Websites/webkit-perf.appspot.com/models_unittest.py:
(DataStoreTestsBase):
(DataStoreTestsBase.assertThereIsNoInstanceOf):
(DataStoreTestsBase.assertOnlyInstance):
(DataStoreTestsBase.assertEqualUnorderedList):
(HelperTests):
(HelperTests.test_create_in_transaction_with_numeric_id_holder):
(HelperTests.test_failing_in_create_in_transaction_with_numeric_id_holder):
(HelperTests.test_raising_in_create_in_transaction_with_numeric_id_holder):
(HelperTests.test_delete_model_with_numeric_id_holder):
(BranchTests):
(BranchTests.test_create_if_possible):
(PlatformTests):
(PlatformTests.test_create_if_possible):
(BuilderTests):
(_create_some_builder):
(BuildTests):
(BuildTests.test_get_or_insert_from_log):
(TestModelTests):
(TestModelTests.test_update_or_insert):
(TestModelTests.test_update_or_insert_to_update):
(TestResultTests):
(TestResultTests._create_build):
(TestResultTests.test_get_or_insert_value):
(TestResultTests.test_get_or_insert_stat_value):
(TestResultTests._create_results):
(TestResultTests.test_generate_runs):
(ReportLogTests):
(ReportLogTests.test_branch):
(ReportLogTests.test_platform):
(PersistentCacheTests):
(PersistentCacheTests.setUp):
(PersistentCacheTests.test_set):
* Websites/webkit-perf.appspot.com/report_process_handler.py:
(ReportProcessHandler.post):
* Websites/webkit-perf.appspot.com/runs_handler.py:
(RunsHandler.get):
(RunsHandler.post):

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

ChangeLog
Websites/webkit-perf.appspot.com/create_handler.py
Websites/webkit-perf.appspot.com/models.py
Websites/webkit-perf.appspot.com/models_unittest.py
Websites/webkit-perf.appspot.com/report_process_handler.py
Websites/webkit-perf.appspot.com/runs_handler.py

index 1752163..2150509 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,72 @@
+2012-02-19  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Move more logic from handler classes to model classes and add unit tests
+        https://bugs.webkit.org/show_bug.cgi?id=78989
+
+        Reviewed by Hajime Morita.
+
+        Extracted various functions from CreateHandler, ReportHanlder, and RunsHanlder to model classes
+        in order to unit-test them, added DataStoreTestsBase to reduce the code duplication in tests,
+        and added a whole bunch of unit tests in models_unittest.py.
+
+        * Websites/webkit-perf.appspot.com/create_handler.py:
+        (CreateHandler._create_branch):
+        (CreateHandler._create_platform):
+        * Websites/webkit-perf.appspot.com/models.py:
+        (_create_if_possible):
+        (_create_if_possible.execute):
+        (Branch):
+        (Branch.create_if_possible):
+        (Platform):
+        (Platform.create_if_possible):
+        (Build):
+        (Build.get_or_insert_from_log):
+        (Test):
+        (Test.update_or_insert):
+        (Test.update_or_insert.execute):
+        (TestResult):
+        (TestResult.get_or_insert_from_parsed_json):
+        (TestResult.get_or_insert_from_parsed_json._float_or_none):
+        (TestResult.generate_runs):
+        * Websites/webkit-perf.appspot.com/models_unittest.py:
+        (DataStoreTestsBase):
+        (DataStoreTestsBase.assertThereIsNoInstanceOf):
+        (DataStoreTestsBase.assertOnlyInstance):
+        (DataStoreTestsBase.assertEqualUnorderedList):
+        (HelperTests):
+        (HelperTests.test_create_in_transaction_with_numeric_id_holder):
+        (HelperTests.test_failing_in_create_in_transaction_with_numeric_id_holder):
+        (HelperTests.test_raising_in_create_in_transaction_with_numeric_id_holder):
+        (HelperTests.test_delete_model_with_numeric_id_holder):
+        (BranchTests):
+        (BranchTests.test_create_if_possible):
+        (PlatformTests):
+        (PlatformTests.test_create_if_possible):
+        (BuilderTests):
+        (_create_some_builder):
+        (BuildTests):
+        (BuildTests.test_get_or_insert_from_log):
+        (TestModelTests):
+        (TestModelTests.test_update_or_insert):
+        (TestModelTests.test_update_or_insert_to_update):
+        (TestResultTests):
+        (TestResultTests._create_build):
+        (TestResultTests.test_get_or_insert_value):
+        (TestResultTests.test_get_or_insert_stat_value):
+        (TestResultTests._create_results):
+        (TestResultTests.test_generate_runs):
+        (ReportLogTests):
+        (ReportLogTests.test_branch):
+        (ReportLogTests.test_platform):
+        (PersistentCacheTests):
+        (PersistentCacheTests.setUp):
+        (PersistentCacheTests.test_set):
+        * Websites/webkit-perf.appspot.com/report_process_handler.py:
+        (ReportProcessHandler.post):
+        * Websites/webkit-perf.appspot.com/runs_handler.py:
+        (RunsHandler.get):
+        (RunsHandler.post):
+
 2012-02-20  Patrick Gansterer  <paroga@webkit.org>
 
         [CMake] Fix PLATFORM() define for Windows.
index 84b9ab2..1526ca6 100644 (file)
@@ -85,33 +85,9 @@ class CreateHandler(webapp2.RequestHandler):
     def _create_branch(self, key, name):
         if not key or not name:
             return 'Invalid key or name'
-
-        error = [None]
-
-        def execute(id):
-            if Branch.get_by_key_name(key):
-                error[0] = 'Branch "%s" already exists' % key
-                return
-            branch = Branch(id=id, name=name, key_name=key)
-            branch.put()
-            return branch
-
-        create_in_transaction_with_numeric_id_holder(execute)
-        return error[0]
+        return None if Branch.create_if_possible(key, name) else 'Branch "%s" already exists' % key
 
     def _create_platform(self, key, name):
         if not key or not name:
             return 'Invalid key name'
-
-        error = [None]
-
-        def execute(id):
-            if Platform.get_by_key_name(key):
-                error[0] = 'Platform "%s" already exists' % key
-                return
-            platform = Platform(id=id, name=name, key_name=key)
-            platform.put()
-            return platform
-
-        create_in_transaction_with_numeric_id_holder(execute)
-        return error[0]
+        return None if Platform.create_if_possible(key, name) else 'Platform "%s" already exists' % key
index d1932c4..8212a33 100644 (file)
@@ -34,6 +34,7 @@ import re
 from datetime import datetime
 from google.appengine.ext import db
 from google.appengine.api import memcache
+from time import mktime
 
 
 class NumericIdHolder(db.Model):
@@ -68,15 +69,35 @@ def model_from_numeric_id(id, expected_kind):
     return id_holder.owner if id_holder and id_holder.owner and isinstance(id_holder.owner, expected_kind) else None
 
 
+def _create_if_possible(model, key, name):
+
+    def execute(id):
+        if model.get_by_key_name(key):
+            return None
+        branch = model(id=id, name=name, key_name=key)
+        branch.put()
+        return branch
+
+    return create_in_transaction_with_numeric_id_holder(execute)
+
+
 class Branch(db.Model):
     id = db.IntegerProperty(required=True)
     name = db.StringProperty(required=True)
 
+    @staticmethod
+    def create_if_possible(key, name):
+        return _create_if_possible(Branch, key, name)
+
 
 class Platform(db.Model):
     id = db.IntegerProperty(required=True)
     name = db.StringProperty(required=True)
 
+    @staticmethod
+    def create_if_possible(key, name):
+        return _create_if_possible(Platform, key, name)
+
 
 class Builder(db.Model):
     name = db.StringProperty(required=True)
@@ -107,6 +128,15 @@ class Build(db.Model):
     chromiumRevision = db.IntegerProperty()
     timestamp = db.DateTimeProperty(required=True)
 
+    @staticmethod
+    def get_or_insert_from_log(log):
+        builder = log.builder()
+        key_name = builder.name + ':' + str(int(mktime(log.timestamp().timetuple())))
+
+        return Build.get_or_insert(key_name, branch=log.branch(), platform=log.platform(), builder=builder,
+            buildNumber=log.build_number(), timestamp=log.timestamp(),
+            revision=log.webkit_revision(), chromiumRevision=log.chromium_revision())
+
 
 # Used to generate TestMap in the manifest efficiently
 class Test(db.Model):
@@ -119,6 +149,26 @@ class Test(db.Model):
     def cache_key(test_id, branch_id, platform_id):
         return 'runs:%d,%d,%d' % (test_id, branch_id, platform_id)
 
+    @staticmethod
+    def update_or_insert(test_name, branch, platform):
+        existing_test = [None]
+
+        def execute(id):
+            test = Test.get_by_key_name(test_name)
+            if test:
+                if branch.key() not in test.branches:
+                    test.branches.append(branch.key())
+                if platform.key() not in test.platforms:
+                    test.platforms.append(platform.key())
+                existing_test[0] = test
+                return None
+
+            test = Test(id=id, name=test_name, key_name=test_name, branches=[branch.key()], platforms=[platform.key()])
+            test.put()
+            return test
+
+        return create_in_transaction_with_numeric_id_holder(execute) or existing_test[0]
+
 
 class TestResult(db.Model):
     name = db.StringProperty(required=True)
@@ -133,6 +183,37 @@ class TestResult(db.Model):
     def key_name(build, test_name):
         return build.key().name() + ':' + test_name
 
+    @classmethod
+    def get_or_insert_from_parsed_json(cls, test_name, build, result):
+        key_name = cls.key_name(build, test_name)
+
+        def _float_or_none(dictionary, key):
+            value = dictionary.get(key)
+            if value:
+                return float(value)
+            return None
+
+        if not isinstance(result, dict):
+            return cls.get_or_insert(key_name, name=test_name, build=build, value=float(result))
+
+        return cls.get_or_insert(key_name, name=test_name, build=build, value=float(result['avg']),
+            valueMedian=_float_or_none(result, 'median'), valueStdev=_float_or_none(result, 'stdev'),
+            valueMin=_float_or_none(result, 'min'), valueMax=_float_or_none(result, 'max'))
+
+    @staticmethod
+    def generate_runs(branch, platform, test_name):
+        builds = Build.all()
+        builds.filter('branch =', branch)
+        builds.filter('platform =', platform)
+
+        for build in builds:
+            results = TestResult.all()
+            results.filter('name =', test_name)
+            results.filter('build =', build)
+            for result in results:
+                yield build, result
+        raise StopIteration
+
 
 class ReportLog(db.Model):
     timestamp = db.DateTimeProperty(required=True)
index 1282219..e6c37b1 100644 (file)
@@ -33,9 +33,10 @@ import unittest
 from datetime import datetime
 from google.appengine.api import memcache
 from google.appengine.ext import testbed
+from time import mktime
 
 
-class HelperTests(unittest.TestCase):
+class DataStoreTestsBase(unittest.TestCase):
     def setUp(self):
         self.testbed = testbed.Testbed()
         self.testbed.activate()
@@ -44,6 +45,18 @@ class HelperTests(unittest.TestCase):
     def tearDown(self):
         self.testbed.deactivate()
 
+    def assertThereIsNoInstanceOf(self, model):
+        self.assertEqual(len(model.all().fetch(5)), 0)
+
+    def assertOnlyInstance(self, only_instasnce):
+        self.assertEqual(len(only_instasnce.__class__.all().fetch(5)), 1)
+        self.assertTrue(only_instasnce.__class__.get(only_instasnce.key()))
+
+    def assertEqualUnorderedList(self, list1, list2):
+        self.assertEqual(set(list1), set(list2))
+
+
+class HelperTests(DataStoreTestsBase):
     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)
@@ -55,8 +68,8 @@ class HelperTests(unittest.TestCase):
         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.assertThereIsNoInstanceOf(models.Branch)
+        self.assertThereIsNoInstanceOf(models.NumericIdHolder)
 
         self.assertTrue(models.create_in_transaction_with_numeric_id_holder(execute))
 
@@ -72,13 +85,13 @@ class HelperTests(unittest.TestCase):
         def execute(id):
             return None
 
-        self.assertEqual(len(models.Branch.all().fetch(5)), 0)
-        self.assertEqual(len(models.NumericIdHolder.all().fetch(5)), 0)
+        self.assertThereIsNoInstanceOf(models.Branch)
+        self.assertThereIsNoInstanceOf(models.NumericIdHolder)
 
         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)
+        self.assertThereIsNoInstanceOf(models.Branch)
+        self.assertThereIsNoInstanceOf(models.NumericIdHolder)
 
     def test_raising_in_create_in_transaction_with_numeric_id_holder(self):
 
@@ -86,13 +99,13 @@ class HelperTests(unittest.TestCase):
             raise TypeError
             return None
 
-        self.assertEqual(len(models.Branch.all().fetch(5)), 0)
-        self.assertEqual(len(models.NumericIdHolder.all().fetch(5)), 0)
+        self.assertThereIsNoInstanceOf(models.Branch)
+        self.assertThereIsNoInstanceOf(models.NumericIdHolder)
 
         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)
+        self.assertThereIsNoInstanceOf(models.Branch)
+        self.assertThereIsNoInstanceOf(models.NumericIdHolder)
 
     def test_delete_model_with_numeric_id_holder(self):
 
@@ -100,12 +113,12 @@ class HelperTests(unittest.TestCase):
             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)
+        self.assertOnlyInstance(branch)
 
         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)
+        self.assertThereIsNoInstanceOf(models.Branch)
+        self.assertThereIsNoInstanceOf(models.NumericIdHolder)
 
     def test_model_from_numeric_id(self):
 
@@ -120,15 +133,37 @@ class HelperTests(unittest.TestCase):
         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()
+class BranchTests(DataStoreTestsBase):
+    def test_create_if_possible(self):
+        self.assertThereIsNoInstanceOf(models.Branch)
 
-    def tearDown(self):
-        self.testbed.deactivate()
+        branch = models.Branch.create_if_possible('some-branch', 'some branch')
+        self.assertTrue(branch)
+        self.assertTrue(branch.key().name(), 'some-branch')
+        self.assertTrue(branch.name, 'some branch')
+        self.assertOnlyInstance(branch)
+
+        self.assertFalse(models.Branch.create_if_possible('some-branch', 'some other branch'))
+        self.assertTrue(branch.name, 'some branch')
+        self.assertOnlyInstance(branch)
+
+
+class PlatformTests(DataStoreTestsBase):
+    def test_create_if_possible(self):
+        self.assertThereIsNoInstanceOf(models.Platform)
 
+        platform = models.Platform.create_if_possible('some-platform', 'some platform')
+        self.assertTrue(platform)
+        self.assertTrue(platform.key().name(), 'some-platform')
+        self.assertTrue(platform.name, 'some platform')
+        self.assertOnlyInstance(platform)
+
+        self.assertFalse(models.Platform.create_if_possible('some-platform', 'some other platform'))
+        self.assertTrue(platform.name, 'some platform')
+        self.assertOnlyInstance(platform)
+
+
+class BuilderTests(DataStoreTestsBase):
     def test_create(self):
         builder_key = models.Builder.create('some builder', 'some password')
         self.assertTrue(builder_key)
@@ -158,15 +193,123 @@ class BuilderTests(unittest.TestCase):
         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_some_builder():
+    branch = models.Branch.create_if_possible('some-branch', 'Some Branch')
+    platform = models.Platform.create_if_possible('some-platform', 'Some Platform')
+    builder_key = models.Builder.create('some-builder', 'Some Builder')
+    return branch, platform, models.Builder.get(builder_key)
+
+
+class BuildTests(DataStoreTestsBase):
+    def test_get_or_insert_from_log(self):
+        branch, platform, builder = _create_some_builder()
+
+        timestamp = datetime.now().replace(microsecond=0)
+        log = models.ReportLog(timestamp=timestamp, headers='some headers',
+            payload='{"branch": "some-branch", "platform": "some-platform", "builder-name": "some-builder",' +
+                '"build-number": 123, "webkit-revision": 456, "timestamp": %d}' % int(mktime(timestamp.timetuple())))
+
+        self.assertThereIsNoInstanceOf(models.Build)
+
+        build = models.Build.get_or_insert_from_log(log)
+        self.assertTrue(build)
+        self.assertEqual(build.branch.key(), branch.key())
+        self.assertEqual(build.platform.key(), platform.key())
+        self.assertEqual(build.builder.key(), builder.key())
+        self.assertEqual(build.buildNumber, 123)
+        self.assertEqual(build.revision, 456)
+        self.assertEqual(build.chromiumRevision, None)
+        self.assertEqual(build.timestamp, timestamp)
+
+        self.assertOnlyInstance(build)
+
+
+class TestModelTests(DataStoreTestsBase):
+    def test_update_or_insert(self):
+        branch = models.Branch.create_if_possible('some-branch', 'Some Branch')
+        platform = models.Platform.create_if_possible('some-platform', 'Some Platform')
+
+        self.assertThereIsNoInstanceOf(models.Test)
+
+        test = models.Test.update_or_insert('some-test', branch, platform)
+        self.assertTrue(test)
+        self.assertEqual(test.branches, [branch.key()])
+        self.assertEqual(test.platforms, [platform.key()])
+        self.assertOnlyInstance(test)
+
+    def test_update_or_insert_to_update(self):
+        branch = models.Branch.create_if_possible('some-branch', 'Some Branch')
+        platform = models.Platform.create_if_possible('some-platform', 'Some Platform')
+        test = models.Test.update_or_insert('some-test', branch, platform)
+        self.assertOnlyInstance(test)
+
+        other_branch = models.Branch.create_if_possible('other-branch', 'Other Branch')
+        other_platform = models.Platform.create_if_possible('other-platform', 'Other Platform')
+        test = models.Test.update_or_insert('some-test', other_branch, other_platform)
+        self.assertOnlyInstance(test)
+        self.assertEqualUnorderedList(test.branches, [branch.key(), other_branch.key()])
+        self.assertEqualUnorderedList(test.platforms, [platform.key(), other_platform.key()])
+
+
+class TestResultTests(DataStoreTestsBase):
+    def _create_build(self):
+        branch, platform, builder = _create_some_builder()
+        build_key = models.Build(key_name='some-build', branch=branch, platform=platform, builder=builder,
+            buildNumber=1, revision=100, timestamp=datetime.now()).put()
+        return models.Build.get(build_key)
+
+    def test_get_or_insert_value(self):
+        build = self._create_build()
+        self.assertThereIsNoInstanceOf(models.TestResult)
+        result = models.TestResult.get_or_insert_from_parsed_json('some-test', build, 50)
+        self.assertOnlyInstance(result)
+        self.assertEqual(result.name, 'some-test')
+        self.assertEqual(result.build.key(), build.key())
+        self.assertEqual(result.value, 50.0)
+        self.assertEqual(result.valueMedian, None)
+        self.assertEqual(result.valueStdev, None)
+        self.assertEqual(result.valueMin, None)
+        self.assertEqual(result.valueMax, None)
+
+    def test_get_or_insert_stat_value(self):
+        build = self._create_build()
+        self.assertThereIsNoInstanceOf(models.TestResult)
+        result = models.TestResult.get_or_insert_from_parsed_json('some-test', build,
+            {"avg": 40, "median": "40.1", "stdev": 3.25, "min": 30.5, "max": 45})
+        self.assertOnlyInstance(result)
+        self.assertEqual(result.name, 'some-test')
+        self.assertEqual(result.build.key(), build.key())
+        self.assertEqual(result.value, 40.0)
+        self.assertEqual(result.valueMedian, 40.1)
+        self.assertEqual(result.valueStdev, 3.25)
+        self.assertEqual(result.valueMin, 30.5)
+        self.assertEqual(result.valueMax, 45)
+
+    def _create_results(self, test_name, values):
+        branch, platform, builder = _create_some_builder()
+        results = []
+        for i, value in enumerate(values):
+            build = models.Build(branch=branch, platform=platform, builder=builder,
+                buildNumber=i, revision=100 + i, timestamp=datetime.now())
+            build.put()
+            result = models.TestResult(name=test_name, build=build, value=value)
+            result.put()
+            results.append(result)
+        return branch, platform, results
+
+    def test_generate_runs(self):
+        branch, platform, results = self._create_results('some-test', [50.0, 51.0, 52.0, 49.0, 48.0])
+        last_i = 0
+        for i, (build, result) in enumerate(models.TestResult.generate_runs(branch, platform, "some-test")):
+            self.assertEqual(build.buildNumber, i)
+            self.assertEqual(build.revision, 100 + i)
+            self.assertEqual(result.name, 'some-test')
+            self.assertEqual(result.value, results[i].value)
+            last_i = i
+        self.assertTrue(last_i + 1, len(results))
+
+
+class ReportLogTests(DataStoreTestsBase):
     def _create_log_with_payload(self, payload):
         return models.ReportLog(timestamp=datetime.now(), headers='some headers', payload=payload)
 
@@ -206,7 +349,27 @@ class ReportLog(unittest.TestCase):
         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_branch(self):
+        log = self._create_log_with_payload('{"key": "value"}')
+        self.assertEqual(log.branch(), None)
+
+        log = self._create_log_with_payload('{"branch": "some-branch"}')
+        self.assertEqual(log.branch(), None)
+
+        branch = models.Branch.create_if_possible("some-branch", "Some Branch")
+        log = self._create_log_with_payload('{"branch": "some-branch"}')
+        self.assertEqual(log.branch().key(), branch.key())
+
+    def test_platform(self):
+        log = self._create_log_with_payload('{"key": "value"}')
+        self.assertEqual(log.platform(), None)
+
+        log = self._create_log_with_payload('{"platform": "some-platform"}')
+        self.assertEqual(log.platform(), None)
+
+        platform = models.Platform.create_if_possible("some-platform", "Some Platform")
+        log = self._create_log_with_payload('{"platform": "some-platform"}')
+        self.assertEqual(log.platform().key(), platform.key())
 
     def test_build_number(self):
         log = self._create_log_with_payload('{"build-number": 123}')
@@ -230,22 +393,19 @@ class ReportLog(unittest.TestCase):
         self.assertEqual(log.webkit_revision(), None)
 
 
-class PersistentCacheTests(unittest.TestCase):
+class PersistentCacheTests(DataStoreTestsBase):
     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)
+        self.assertThereIsNoInstanceOf(models.PersistentCache)
 
         models.PersistentCache.set_cache('some-cache', 'some data')
         self._assert_persistent_cache('some-cache', 'some data')
index 4e6e2e8..ecf6e38 100644 (file)
@@ -39,7 +39,6 @@ from models import Build
 from models import ReportLog
 from models import Test
 from models import TestResult
-from models import create_in_transaction_with_numeric_id_holder
 
 
 class ReportProcessHandler(webapp2.RequestHandler):
@@ -55,11 +54,11 @@ class ReportProcessHandler(webapp2.RequestHandler):
 
         branch = log.branch()
         platform = log.platform()
-        build = self._create_build_if_possible(log, branch, platform)
+        build = Build.get_or_insert_from_log(log)
 
         for test_name, result in log.results().iteritems():
-            test = self._add_test_if_needed(test_name, branch, platform)
-            self._add_test_result_if_needed(test_name, build, result)
+            test = Test.update_or_insert(test_name, branch, platform)
+            TestResult.get_or_insert_from_parsed_json(test_name, build, result)
             schedule_runs_update(test.id, branch.id, platform.id)
 
         log = ReportLog.get(log.key())
@@ -70,42 +69,3 @@ class ReportProcessHandler(webapp2.RequestHandler):
         schedule_manifest_update()
 
         self.response.out.write('OK')
-
-    def _create_build_if_possible(self, log, branch, platform):
-        builder = log.builder()
-        key_name = builder.name + ':' + str(int(time.mktime(log.timestamp().timetuple())))
-
-        return Build.get_or_insert(key_name, branch=branch, platform=platform, builder=builder, buildNumber=log.build_number(),
-            timestamp=log.timestamp(), revision=log.webkit_revision(), chromiumRevision=log.chromium_revision())
-
-    def _add_test_if_needed(self, test_name, branch, platform):
-
-        def execute(id):
-            test = Test.get_by_key_name(test_name)
-            returnValue = None
-            if not test:
-                test = Test(id=id, name=test_name, key_name=test_name)
-                returnValue = test
-            if branch.key() not in test.branches:
-                test.branches.append(branch.key())
-            if platform.key() not in test.platforms:
-                test.platforms.append(platform.key())
-            test.put()
-            return returnValue
-        return create_in_transaction_with_numeric_id_holder(execute) or Test.get_by_key_name(test_name)
-
-    def _add_test_result_if_needed(self, test_name, build, result):
-        key_name = TestResult.key_name(build, test_name)
-
-        def _float_or_none(dictionary, key):
-            value = dictionary.get(key)
-            if value:
-                return float(value)
-            return None
-
-        if not isinstance(result, dict):
-            return TestResult.get_or_insert(key_name, name=test_name, build=build, value=float(result))
-
-        return TestResult.get_or_insert(key_name, name=test_name, build=build, value=float(result['avg']),
-            valueMedian=_float_or_none(result, 'median'), valueStdev=_float_or_none(result, 'stdev'),
-            valueMin=_float_or_none(result, 'min'), valueMax=_float_or_none(result, 'max'))
index 280ae2f..5715193 100644 (file)
@@ -61,41 +61,38 @@ class RunsHandler(webapp2.RequestHandler):
         # FIXME: Just fetch builds specified by "days"
         # days = self.request.get('days', 365)
 
-        builds = Build.all()
-        builds.filter('branch =', model_from_numeric_id(branch_id, Branch))
-        builds.filter('platform =', model_from_numeric_id(platform_id, Platform))
-
+        branch = model_from_numeric_id(branch_id, Branch)
+        platform = model_from_numeric_id(platform_id, Platform)
         test = model_from_numeric_id(test_id, Test)
-        test_name = test.name if test else None
+        assert branch
+        assert platform
+        assert test
+
         test_runs = []
         averages = {}
         values = []
         timestamps = []
 
-        for build in builds:
-            results = TestResult.all()
-            results.filter('name =', test_name)
-            results.filter('build =', build)
-            for result in results:
-                builderId = build.builder.key().id()
-                posixTimestamp = mktime(build.timestamp.timetuple())
-                statistics = None
-                supplementary_revisions = None
-                if result.valueStdev != None and result.valueMin != None and result.valueMax != None:
-                    statistics = {'stdev': result.valueStdev, 'min': result.valueMin, 'max': result.valueMax}
-                if build.chromiumRevision != None:
-                    supplementary_revisions = {'Chromium': build.chromiumRevision}
+        for build, result in TestResult.generate_runs(branch, platform, test.name):
+            builderId = build.builder.key().id()
+            posixTimestamp = mktime(build.timestamp.timetuple())
+            statistics = None
+            supplementary_revisions = None
+            if result.valueStdev != None and result.valueMin != None and result.valueMax != None:
+                statistics = {'stdev': result.valueStdev, 'min': result.valueMin, 'max': result.valueMax}
+            if build.chromiumRevision != None:
+                supplementary_revisions = {'Chromium': build.chromiumRevision}
 
-                test_runs.append([result.key().id(),
-                    [build.key().id(), build.buildNumber, build.revision, supplementary_revisions],
-                    posixTimestamp, result.value, 0,  # runNumber
-                    [],  # annotations
-                    builderId, statistics])
+            test_runs.append([result.key().id(),
+                [build.key().id(), build.buildNumber, build.revision, supplementary_revisions],
+                posixTimestamp, result.value, 0,  # runNumber
+                [],  # annotations
+                builderId, statistics])
 
-                # FIXME: Calculate the average; in practice, we wouldn't have more than one value for a given revision
-                averages[build.revision] = result.value
-                values.append(result.value)
-                timestamps.append(posixTimestamp)
+            # FIXME: Calculate the average; in practice, we wouldn't have more than one value for a given revision
+            averages[build.revision] = result.value
+            values.append(result.value)
+            timestamps.append(posixTimestamp)
 
         result = json.dumps({
             'test_runs': test_runs,