diff --git a/master/buildbot/data/test_results.py b/master/buildbot/data/test_results.py index 2a8ae851fb4..5bef4627db3 100644 --- a/master/buildbot/data/test_results.py +++ b/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): diff --git a/master/buildbot/db/test_results.py b/master/buildbot/db/test_results.py index 9b40bfd4e42..d187685966b 100644 --- a/master/buildbot/db/test_results.py +++ b/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, diff --git a/master/buildbot/test/fakedb/test_results.py b/master/buildbot/test/fakedb/test_results.py index a6a2d2550d4..8a8b5550ca4 100644 --- a/master/buildbot/test/fakedb/test_results.py +++ b/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) diff --git a/master/buildbot/test/unit/data/test_test_results.py b/master/buildbot/test/unit/data/test_test_results.py index 414ae8099af..ecfac7bf1c3 100644 --- a/master/buildbot/test/unit/data/test_test_results.py +++ b/master/buildbot/test/unit/data/test_test_results.py @@ -18,6 +18,7 @@ from twisted.trial import unittest from buildbot.data import test_results +from buildbot.db.test_results import TestResultModel from buildbot.test import fakedb from buildbot.test.fake import fakemaster from buildbot.test.reactor import TestReactorMixin @@ -125,69 +126,69 @@ def test_add_test_results(self): results = yield self.master.db.test_results.getTestResults( builderid=88, test_result_setid=13 ) - resultid = results[0]['id'] + resultid = results[0].id self.assertEqual( results, [ - { - 'id': resultid, - 'builderid': 88, - 'test_result_setid': 13, - 'test_name': 'name1', - 'test_code_path': None, - 'line': None, - 'duration_ns': None, - 'value': '1', - }, - { - 'id': resultid + 1, - 'builderid': 88, - 'test_result_setid': 13, - 'test_name': 'name2', - 'test_code_path': None, - 'line': None, - 'duration_ns': 1000, - 'value': '1', - }, - { - 'id': resultid + 2, - 'builderid': 88, - 'test_result_setid': 13, - 'test_name': 'name3', - 'test_code_path': 'path2', - 'line': None, - 'duration_ns': None, - 'value': '2', - }, - { - 'id': resultid + 3, - 'builderid': 88, - 'test_result_setid': 13, - 'test_name': 'name4', - 'test_code_path': 'path3', - 'line': None, - 'duration_ns': None, - 'value': '3', - }, - { - 'id': resultid + 4, - 'builderid': 88, - 'test_result_setid': 13, - 'test_name': 'name5', - 'test_code_path': 'path4', - 'line': 4, - 'duration_ns': None, - 'value': '4', - }, - { - 'id': resultid + 5, - 'builderid': 88, - 'test_result_setid': 13, - 'test_name': None, - 'test_code_path': 'path5', - 'line': 5, - 'duration_ns': None, - 'value': '5', - }, + TestResultModel( + id=resultid, + builderid=88, + test_result_setid=13, + test_name='name1', + test_code_path=None, + line=None, + duration_ns=None, + value='1', + ), + TestResultModel( + id=resultid + 1, + builderid=88, + test_result_setid=13, + test_name='name2', + test_code_path=None, + line=None, + duration_ns=1000, + value='1', + ), + TestResultModel( + id=resultid + 2, + builderid=88, + test_result_setid=13, + test_name='name3', + test_code_path='path2', + line=None, + duration_ns=None, + value='2', + ), + TestResultModel( + id=resultid + 3, + builderid=88, + test_result_setid=13, + test_name='name4', + test_code_path='path3', + line=None, + duration_ns=None, + value='3', + ), + TestResultModel( + id=resultid + 4, + builderid=88, + test_result_setid=13, + test_name='name5', + test_code_path='path4', + line=4, + duration_ns=None, + value='4', + ), + TestResultModel( + id=resultid + 5, + builderid=88, + test_result_setid=13, + test_name=None, + test_code_path='path5', + line=5, + duration_ns=None, + value='5', + ), ], ) diff --git a/master/buildbot/test/unit/db/test_test_results.py b/master/buildbot/test/unit/db/test_test_results.py index 30c1b2340f8..c467698e9e5 100644 --- a/master/buildbot/test/unit/db/test_test_results.py +++ b/master/buildbot/test/unit/db/test_test_results.py @@ -20,7 +20,6 @@ from buildbot.test import fakedb from buildbot.test.util import connector_component from buildbot.test.util import interfaces -from buildbot.test.util import validation class Tests(interfaces.InterfaceTests): @@ -88,89 +87,89 @@ def test_add_set_results(self): result_dicts = yield self.db.test_results.getTestResults(builderid=88, test_result_setid=13) for d in result_dicts: - validation.verifyDbDict(self, 'test_resultdict', d) + self.assertIsInstance(d, test_results.TestResultModel) - result_dicts = sorted(result_dicts, key=lambda x: x['id']) - resultid = result_dicts[0]['id'] + result_dicts = sorted(result_dicts, key=lambda x: x.id) + resultid = result_dicts[0].id self.assertEqual( result_dicts, [ - { - 'id': resultid, - 'builderid': 88, - 'test_result_setid': 13, - 'test_name': 'name1', - 'test_code_path': None, - 'line': None, - 'duration_ns': None, - 'value': '1', - }, - { - 'id': resultid + 1, - 'builderid': 88, - 'test_result_setid': 13, - 'test_name': 'name1', - 'test_code_path': None, - 'line': None, - 'duration_ns': 1000, - 'value': '2', - }, - { - 'id': resultid + 2, - 'builderid': 88, - 'test_result_setid': 13, - 'test_name': 'name2', - 'test_code_path': 'path2', - 'line': None, - 'duration_ns': None, - 'value': '3', - }, - { - 'id': resultid + 3, - 'builderid': 88, - 'test_result_setid': 13, - 'test_name': 'name3', - 'test_code_path': 'path3', - 'line': None, - 'duration_ns': None, - 'value': '4', - }, - { - 'id': resultid + 4, - 'builderid': 88, - 'test_result_setid': 13, - 'test_name': 'name4', - 'test_code_path': 'path4', - 'line': 4, - 'duration_ns': None, - 'value': '5', - }, - { - 'id': resultid + 5, - 'builderid': 88, - 'test_result_setid': 13, - 'test_name': None, - 'test_code_path': 'path5', - 'line': 5, - 'duration_ns': None, - 'value': '6', - }, + test_results.TestResultModel( + id=resultid, + builderid=88, + test_result_setid=13, + test_name='name1', + test_code_path=None, + line=None, + duration_ns=None, + value='1', + ), + test_results.TestResultModel( + id=resultid + 1, + builderid=88, + test_result_setid=13, + test_name='name1', + test_code_path=None, + line=None, + duration_ns=1000, + value='2', + ), + test_results.TestResultModel( + id=resultid + 2, + builderid=88, + test_result_setid=13, + test_name='name2', + test_code_path='path2', + line=None, + duration_ns=None, + value='3', + ), + test_results.TestResultModel( + id=resultid + 3, + builderid=88, + test_result_setid=13, + test_name='name3', + test_code_path='path3', + line=None, + duration_ns=None, + value='4', + ), + test_results.TestResultModel( + id=resultid + 4, + builderid=88, + test_result_setid=13, + test_name='name4', + test_code_path='path4', + line=4, + duration_ns=None, + value='5', + ), + test_results.TestResultModel( + id=resultid + 5, + builderid=88, + test_result_setid=13, + test_name=None, + test_code_path='path5', + line=5, + duration_ns=None, + value='6', + ), ], ) result_dict = yield self.db.test_results.getTestResult(test_resultid=resultid) self.assertEqual( result_dict, - { - 'id': resultid, - 'builderid': 88, - 'test_result_setid': 13, - 'test_name': 'name1', - 'test_code_path': None, - 'line': None, - 'duration_ns': None, - 'value': '1', - }, + test_results.TestResultModel( + id=resultid, + builderid=88, + test_result_setid=13, + test_name='name1', + test_code_path=None, + line=None, + duration_ns=None, + value='1', + ), ) @defer.inlineCallbacks diff --git a/master/buildbot/test/util/validation.py b/master/buildbot/test/util/validation.py index 42d1e7f0eed..6138ce2ea83 100644 --- a/master/buildbot/test/util/validation.py +++ b/master/buildbot/test/util/validation.py @@ -645,17 +645,6 @@ def validate(self, name, arg_object): None, MessageValidator(events=[b'new'], messageValidator=_test_results_msgdict) ) -dbdict['test_resultdict'] = DictValidator( - id=IntValidator(), - builderid=IntValidator(), - test_result_setid=IntValidator(), - test_name=NoneOk(StringValidator()), - test_code_path=NoneOk(StringValidator()), - line=NoneOk(IntValidator()), - duration_ns=NoneOk(IntValidator()), - value=StringValidator(), -) - # external functions diff --git a/master/docs/spelling_wordlist.txt b/master/docs/spelling_wordlist.txt index 4f492d914d7..66016b7e7b7 100644 --- a/master/docs/spelling_wordlist.txt +++ b/master/docs/spelling_wordlist.txt @@ -869,6 +869,7 @@ tcp templateCache templating testability +TestResultModel testsuite textarea tgrid diff --git a/newsfragments/db-TestResultDict.removal b/newsfragments/db-TestResultDict.removal new file mode 100644 index 00000000000..6127e975130 --- /dev/null +++ b/newsfragments/db-TestResultDict.removal @@ -0,0 +1 @@ +:class:`buildbot.db.test_results.TestResultDict` is deprecated in favor of :class:`buildbot.db.test_results.TestResultModel` diff --git a/newsfragments/db-introduce-TestResultModel.change b/newsfragments/db-introduce-TestResultModel.change new file mode 100644 index 00000000000..c76e5482fbf --- /dev/null +++ b/newsfragments/db-introduce-TestResultModel.change @@ -0,0 +1 @@ +:class:`TestResultsConnectorComponent` `getTestResult`, and `getTestResults` now return a :class`TestResultModel` instead of a dictionary of TestResult data \ No newline at end of file