Merge pull request #7650 from tdesveaux/typing/db/test_results

 typing: Add TestResultModel dataclass

master/buildbot/data/test_results.py

@@ -14,25 +14,31 @@
 # Copyright Buildbot Team Members
 
 
+from __future__ import annotations
+
+from typing import TYPE_CHECKING
+
 from twisted.internet import defer
 
 from buildbot.data import base
 from buildbot.data import types
 
+if TYPE_CHECKING:
+    from buildbot.db.test_results import TestResultModel
+
 
 class Db2DataMixin:
-    def db2data(self, dbdict):
-        data = {
-            'test_resultid': dbdict['id'],
-            'builderid': dbdict['builderid'],
-            'test_result_setid': dbdict['test_result_setid'],
-            'test_name': dbdict['test_name'],
-            'test_code_path': dbdict['test_code_path'],
-            'line': dbdict['line'],
-            'duration_ns': dbdict['duration_ns'],
-            'value': dbdict['value'],
+    def db2data(self, model: TestResultModel):
+        return {
+            'test_resultid': model.id,
+            'builderid': model.builderid,
+            'test_result_setid': model.test_result_setid,
+            'test_name': model.test_name,
+            'test_code_path': model.test_code_path,
+            'line': model.line,
+            'duration_ns': model.duration_ns,
+            'value': model.value,
         }
-        return defer.succeed(data)
 
 
 class TestResultsEndpoint(Db2DataMixin, base.Endpoint):
@@ -54,10 +60,7 @@ def get(self, resultSpec, kwargs):
             set_dbdict['builderid'], kwargs['test_result_setid'], result_spec=resultSpec
         )
 
-        results = []
-        for dbdict in result_dbdicts:
-            results.append((yield self.db2data(dbdict)))
-        return results
+        return [self.db2data(result) for result in result_dbdicts]
 
 
 class TestResult(base.ResourceType):

master/buildbot/db/test_results.py

@@ -13,24 +13,60 @@
 #
 # Copyright Buildbot Team Members
 
+from __future__ import annotations
+
+from dataclasses import dataclass
+
 import sqlalchemy as sa
 from twisted.internet import defer
+from twisted.python import deprecate
+from twisted.python import versions
 
 from buildbot.db import base
+from buildbot.warnings import warn_deprecated
+
+
+@dataclass
+class TestResultModel:
+    id: int
+    builderid: int
+    test_result_setid: int
+    test_name: str | None
+    test_code_path: str | None
+    line: int | None
+    duration_ns: int | None
+    value: str | None
+
+    # For backward compatibility
+    def __getitem__(self, key: str):
+        warn_deprecated(
+            '4.1.0',
+            (
+                'TestResultsConnectorComponent '
+                'getTestResult, and getTestResults '
+                'no longer return TestResult as dictionnaries. '
+                'Usage of [] accessor is deprecated: please access the member directly'
+            ),
+        )
+
+        if hasattr(self, key):
+            return getattr(self, key)
+
+        raise KeyError(key)
 
 
-class TestResultDict(dict):
+@deprecate.deprecated(versions.Version("buildbot", 4, 1, 0), TestResultModel)
+class TestResultDict(TestResultModel):
     pass
 
 
 class TestResultsConnectorComponent(base.DBConnectorComponent):
-    @defer.inlineCallbacks
-    def _add_code_paths(self, builderid, paths):
+    def _add_code_paths(self, builderid: int, paths: set[str]) -> defer.Deferred[dict[str, int]]:
         # returns a dictionary of path to id in the test_code_paths table.
         # For paths that already exist, the id of the row in the test_code_paths is retrieved.
         assert isinstance(paths, set)
 
-        def thd(conn):
+        def thd(conn) -> dict[str, int]:
             paths_to_ids = {}
             paths_table = self.db.model.test_code_paths
 
@@ -77,12 +113,12 @@ def thd(conn):
 
             return paths_to_ids
 
-        paths_to_id = yield self.db.pool.do(thd)
-        return paths_to_id
+        return self.db.pool.do(thd)
 
-    @defer.inlineCallbacks
-    def getTestCodePaths(self, builderid, path_prefix=None, result_spec=None):
-        def thd(conn):
+    def getTestCodePaths(
+        self, builderid, path_prefix: str | None = None, result_spec=None
+    ) -> defer.Deferred[list[str]]:
+        def thd(conn) -> list[str]:
             paths_table = self.db.model.test_code_paths
             q = paths_table.select()
             if path_prefix is not None:
@@ -92,16 +128,14 @@ def thd(conn):
             res = conn.execute(q)
             return [row['path'] for row in res.fetchall()]
 
-        res = yield self.db.pool.do(thd)
-        return res
+        return self.db.pool.do(thd)
 
-    @defer.inlineCallbacks
-    def _add_names(self, builderid, names):
+    def _add_names(self, builderid: int, names: set[str]) -> defer.Deferred[dict[str, int]]:
         # returns a dictionary of name to id in the test_names table.
         # For names that already exist, the id of the row in the test_names is retrieved.
         assert isinstance(names, set)
 
-        def thd(conn):
+        def thd(conn) -> dict[str, int]:
             names_to_ids = {}
             names_table = self.db.model.test_names
 
@@ -147,12 +181,12 @@ def thd(conn):
 
             return names_to_ids
 
-        names_to_id = yield self.db.pool.do(thd)
-        return names_to_id
+        return self.db.pool.do(thd)
 
-    @defer.inlineCallbacks
-    def getTestNames(self, builderid, name_prefix=None, result_spec=None):
-        def thd(conn):
+    def getTestNames(
+        self, builderid, name_prefix=None, result_spec=None
+    ) -> defer.Deferred[list[str]]:
+        def thd(conn) -> list[str]:
             names_table = self.db.model.test_names
             q = names_table.select().where(names_table.c.builderid == builderid)
             if name_prefix is not None:
@@ -162,8 +196,7 @@ def thd(conn):
             res = conn.execute(q)
             return [row['name'] for row in res.fetchall()]
 
-        res = yield self.db.pool.do(thd)
-        return res
+        return self.db.pool.do(thd)
 
     @defer.inlineCallbacks
     def addTestResults(self, builderid, test_result_setid, result_values):
@@ -224,9 +257,8 @@ def thd(conn):
 
         yield self.db.pool.do(thd)
 
-    @defer.inlineCallbacks
-    def getTestResult(self, test_resultid):
-        def thd(conn):
+    def getTestResult(self, test_resultid: int) -> defer.Deferred[TestResultModel | None]:
+        def thd(conn) -> TestResultModel | None:
             results_table = self.db.model.test_results
             code_paths_table = self.db.model.test_code_paths
             names_table = self.db.model.test_names
@@ -240,14 +272,14 @@ def thd(conn):
             row = res.fetchone()
             if not row:
                 return None
-            return self._thd_row2dict(conn, row)
+            return self._mode_from_row(row)
 
-        res = yield self.db.pool.do(thd)
-        return res
+        return self.db.pool.do(thd)
 
-    @defer.inlineCallbacks
-    def getTestResults(self, builderid, test_result_setid, result_spec=None):
-        def thd(conn):
+    def getTestResults(
+        self, builderid: int, test_result_setid: int, result_spec=None
+    ) -> defer.Deferred[list[TestResultModel]]:
+        def thd(conn) -> list[TestResultModel]:
             results_table = self.db.model.test_results
             code_paths_table = self.db.model.test_code_paths
             names_table = self.db.model.test_names
@@ -274,15 +306,14 @@ def thd(conn):
             )
 
             if result_spec is not None:
-                return result_spec.thd_execute(conn, q, lambda x: self._thd_row2dict(conn, x))
+                return result_spec.thd_execute(conn, q, self._mode_from_row)
             res = conn.execute(q)
-            return [self._thd_row2dict(conn, row) for row in res.fetchall()]
+            return [self._mode_from_row(row) for row in res.fetchall()]
 
-        res = yield self.db.pool.do(thd)
-        return res
+        return self.db.pool.do(thd)
 
-    def _thd_row2dict(self, conn, row):
-        return TestResultDict(
+    def _mode_from_row(self, row):
+        return TestResultModel(
             id=row.id,
             builderid=row.builderid,
             test_result_setid=row.test_result_setid,

master/buildbot/test/fakedb/test_results.py

@@ -13,8 +13,11 @@
 #
 # Copyright Buildbot Team Members
 
+from __future__ import annotations
+
 from twisted.internet import defer
 
+from buildbot.db.test_results import TestResultModel
 from buildbot.test.fakedb.base import FakeDBComponent
 from buildbot.test.fakedb.row import Row
 
@@ -211,7 +214,7 @@ def getTestCodePaths(self, builderid, path_prefix=None, result_spec=None):
 
         return defer.succeed(ret)
 
-    def _fill_extra_data(self, id, row):
+    def _model_from_id(self, id: int, row: dict) -> TestResultModel:
         row = row.copy()
         row['id'] = id
 
@@ -227,13 +230,22 @@ def _fill_extra_data(self, id, row):
             row['test_code_path'] = None
         del row['test_code_pathid']
 
-        return row
+        return TestResultModel(
+            id=row['id'],
+            builderid=row['builderid'],
+            test_result_setid=row['test_result_setid'],
+            line=row['line'],
+            duration_ns=row['duration_ns'],
+            value=row['value'],
+            test_name=row['test_name'],
+            test_code_path=row['test_code_path'],
+        )
 
     # returns a Deferred
     def getTestResult(self, test_resultid):
         if test_resultid not in self.results:
             return defer.succeed(None)
-        return defer.succeed(self._fill_extra_data(test_resultid, self.results[test_resultid]))
+        return defer.succeed(self._model_from_id(test_resultid, self.results[test_resultid]))
 
     # returns a Deferred
     def getTestResults(self, builderid, test_result_setid, result_spec=None):
@@ -243,7 +255,7 @@ def getTestResults(self, builderid, test_result_setid, result_spec=None):
                 continue
             if row['test_result_setid'] != test_result_setid:
                 continue
-            ret.append(self._fill_extra_data(id, row))
+            ret.append(self._model_from_id(id, row))
 
         if result_spec is not None:
             ret = self.applyResultSpec(ret, result_spec)