diff --git a/backend/test_observer/controllers/artefacts/models.py b/backend/test_observer/controllers/artefacts/models.py index 3ecc6c5b..d8fe7063 100644 --- a/backend/test_observer/controllers/artefacts/models.py +++ b/backend/test_observer/controllers/artefacts/models.py @@ -60,7 +60,7 @@ class TestExecutionDTO(BaseModel): c3_link: str | None environment: EnvironmentDTO status: TestExecutionStatus - test_results: list[TestResult] | None + test_results: list[TestResult] class ArtefactBuildDTO(BaseModel): diff --git a/backend/test_observer/external_apis/c3/c3.py b/backend/test_observer/external_apis/c3/c3.py index cdc43237..f7a509fc 100644 --- a/backend/test_observer/external_apis/c3/c3.py +++ b/backend/test_observer/external_apis/c3/c3.py @@ -5,7 +5,7 @@ import requests from requests import Request, Response -from .models import Report, SubmissionStatus, TestResult +from .models import Report, SubmissionStatus logger = logging.getLogger("test-observer-backend") @@ -21,7 +21,6 @@ def get_submissions_statuses( ) -> dict[int, SubmissionStatus]: str_ids = [str(status_id) for status_id in ids] - # Edge case: if there are 0 submissions, we don't need to make an API call if not str_ids: return {} @@ -43,7 +42,6 @@ def get_submissions_statuses( def get_reports(self, ids: Iterable[int]) -> dict[int, Report]: str_ids = [str(report_id) for report_id in ids] - # Edge case: if there are 0 reports, we don't need to make an API call if not str_ids: return {} @@ -57,25 +55,7 @@ def get_reports(self, ids: Iterable[int]) -> dict[int, Report]: if response.ok: reports = response.json()["results"] - return { - json["id"]: Report( - id=json["id"], - failed_test_count=json["failed_test_count"], - test_count=json["test_count"], - test_results=[ - TestResult( - id=test_result["test"]["id"], - name=test_result["test"]["name"], - status=test_result["status"], - type=test_result["test"]["type"], - io_log=test_result["io_log"], - comment=test_result["comment"], - ) - for test_result in json["testresult_set"] - ], - ) - for json in reports - } + return {json["id"]: Report(**json) for json in reports} else: logger.warning(response.text) return {} diff --git a/backend/test_observer/external_apis/c3/models.py b/backend/test_observer/external_apis/c3/models.py index 85bded75..6ec16050 100644 --- a/backend/test_observer/external_apis/c3/models.py +++ b/backend/test_observer/external_apis/c3/models.py @@ -1,6 +1,6 @@ from enum import Enum -from pydantic import BaseModel +from pydantic import AliasPath, BaseModel, Field class SubmissionProcessingStatus(str, Enum): @@ -14,11 +14,16 @@ class SubmissionStatus(BaseModel): report_id: int | None +class TestResultStatus(str, Enum): + PASS = "pass" + FAIL = "fail" + SKIP = "skip" + + class TestResult(BaseModel): - id: int - name: str - type: str - status: str + id: int = Field(validation_alias=AliasPath("test", "id")) + name: str = Field(validation_alias=AliasPath("test", "name")) + status: TestResultStatus comment: str io_log: str @@ -26,5 +31,4 @@ class TestResult(BaseModel): class Report(BaseModel): id: int failed_test_count: int - test_count: int - test_results: list[TestResult] + test_results: list[TestResult] = Field(validation_alias="testresult_set") diff --git a/backend/tests/controllers/artefacts/test_artefacts.py b/backend/tests/controllers/artefacts/test_artefacts.py index c7936068..bb0b3aec 100644 --- a/backend/tests/controllers/artefacts/test_artefacts.py +++ b/backend/tests/controllers/artefacts/test_artefacts.py @@ -180,7 +180,10 @@ def test_correct_test_execution_status( "get_submissions_statuses", lambda *_: {submission_status.id: submission_status}, ) - report = Report(id=237670, failed_test_count=0, test_count=0, test_results=[]) + + # The type checker cannot recognize that the testresult_set is an alias + # to the test_results field, we ignore the call-arg error because of this + report = Report(id=237670, failed_test_count=0, testresult_set=[]) # type: ignore monkeypatch.setattr(c3api_mock, "get_reports", lambda *_: {report.id: report}) app.dependency_overrides[C3Api] = c3api_mock diff --git a/backend/tests/external_apis/test_c3.py b/backend/tests/external_apis/test_c3.py index 92050176..fede515c 100644 --- a/backend/tests/external_apis/test_c3.py +++ b/backend/tests/external_apis/test_c3.py @@ -12,6 +12,7 @@ Report, SubmissionProcessingStatus, SubmissionStatus, + TestResultStatus, ) @@ -21,6 +22,25 @@ class StatusDict(TypedDict): status: SubmissionProcessingStatus +class C3TestCaseDict(TypedDict): + id: int + name: str + + +class C3TestResultDict(TypedDict): + id: int + comment: str + io_log: str + test: C3TestCaseDict + status: str + + +class C3ReportSummaryAPIResponse(TypedDict): + id: int + failed_test_count: int + testresult_set: list[C3TestResultDict] + + @pytest.fixture def prepare_c3api( requests_mock: Mocker, monkeypatch: pytest.MonkeyPatch @@ -111,10 +131,9 @@ def test_get_submissions_statuses( def test_get_reports(requests_mock: Mocker, prepare_c3api: tuple[C3Api, str]): c3, bearer_token = prepare_c3api - c3_api_response = { + c3_api_response: C3ReportSummaryAPIResponse = { "id": 237670, "failed_test_count": 0, - "test_count": 0, "testresult_set": [], } requests_mock.get( @@ -123,22 +142,68 @@ def test_get_reports(requests_mock: Mocker, prepare_c3api: tuple[C3Api, str]): json={"results": [c3_api_response]}, ) + reports = c3.get_reports([c3_api_response["id"]]) + + # The type checker cannot recognize that the testresult_set is an alias + # to the test_results field, we ignore the call-arg error because of this + expected_report = Report(**c3_api_response) # type: ignore + + assert reports == {expected_report.id: expected_report} + + +def test_get_reports_with_test_results( + requests_mock: Mocker, prepare_c3api: tuple[C3Api, str] +): + c3, bearer_token = prepare_c3api + c3_api_response: C3ReportSummaryAPIResponse = { + "id": 237670, + "failed_test_count": 0, + "testresult_set": [ + { + "id": 123123, + "comment": "Test comment", + "io_log": "IO log of the test run", + "status": "pass", + "test": { + "id": 12, + "name": "wireless", + }, + }, + { + "id": 123124, + "comment": "Test comment", + "io_log": "IO log of the test run", + "status": "fail", + "test": { + "id": 12, + "name": "camera", + }, + }, + ], + } + requests_mock.get( - f"https://certification.canonical.com/api/v2/reports/summary/{c3_api_response['id']}/", + f"https://certification.canonical.com/api/v2/reports/summary/?id__in={c3_api_response['id']}&limit=1", request_headers={"Authorization": f"Bearer {bearer_token}"}, json={"results": [c3_api_response]}, ) - # We ignore this typing error, it happens because of the - # "testresult_set" assigned to empty list in the c3_api_response - # dict, and the Python Typing system cannot be sure that the "id" - # field is int - reports = c3.get_reports([c3_api_response["id"]]) # type: ignore - - expected_report = Report( - id=237670, - failed_test_count=0, - test_count=0, - test_results=[], - ) - assert reports == {expected_report.id: expected_report} + reports = c3.get_reports([c3_api_response["id"]]) + + assert len(reports[237670].test_results) == 2 + + count_pass = 0 + count_fail = 0 + for test_result in reports[237670].test_results: + if test_result.status == TestResultStatus.PASS: + count_pass += 1 + elif test_result.status == TestResultStatus.FAIL: + count_fail += 1 + + assert count_pass == 1 and count_fail == 1 + + # The type checker cannot recognize that the testresult_set is an alias + # to the test_results field, we ignore the call-arg error because of this + expected_report = Report(**c3_api_response) # type: ignore + + assert reports == {expected_report.id: expected_report} \ No newline at end of file diff --git a/frontend/lib/models/test_execution.dart b/frontend/lib/models/test_execution.dart index 320e321d..0c9ea44e 100644 --- a/frontend/lib/models/test_execution.dart +++ b/frontend/lib/models/test_execution.dart @@ -17,7 +17,7 @@ class TestExecution with _$TestExecution { @JsonKey(name: 'c3_link') required String? c3Link, required TestExecutionStatus status, required Environment environment, - @JsonKey(name: 'test_results') required List? testResults, + @JsonKey(name: 'test_results') required List testResults, }) = _TestExecution; factory TestExecution.fromJson(Map json) => diff --git a/frontend/lib/models/test_results.dart b/frontend/lib/models/test_results.dart index 1bbae05f..a730b640 100644 --- a/frontend/lib/models/test_results.dart +++ b/frontend/lib/models/test_results.dart @@ -11,7 +11,6 @@ class TestResult with _$TestResult { const factory TestResult({ required int id, required String name, - required String type, required TestResultStatus status, @JsonKey(name: 'io_log') required String ioLog, required String comment, diff --git a/frontend/lib/ui/dashboard/artefact_dialog.dart b/frontend/lib/ui/dashboard/artefact_dialog.dart index f3e2c1e9..509fa247 100644 --- a/frontend/lib/ui/dashboard/artefact_dialog.dart +++ b/frontend/lib/ui/dashboard/artefact_dialog.dart @@ -277,21 +277,8 @@ class _TestExecutionView extends StatelessWidget { final c3Link = testExecution.c3Link; // Split the test result in separate categories based on Status (PASS/FAIL/SKIP) - final Map> testResultGroups = {}; - for (var value in TestResultStatus.values) { - final List currentTestCases = testExecution.testResults - ?.where( - (testResult) => testResult.status == value, - ) - .toList() ?? - []; - - if (currentTestCases.isEmpty) { - continue; - } - - testResultGroups[value] = currentTestCases; - } + final Map> testResultGroups = + testExecution.testResults.groupBy(((testResult) => testResult.status)); return YaruExpandable( header: Row( diff --git a/frontend/lib/ui/dashboard/testresults_components.dart b/frontend/lib/ui/dashboard/testresults_components.dart index 21a6f02a..f296e018 100644 --- a/frontend/lib/ui/dashboard/testresults_components.dart +++ b/frontend/lib/ui/dashboard/testresults_components.dart @@ -62,6 +62,7 @@ class _TestResultView extends StatelessWidget { Text( testResult.name, style: Theme.of(context).textTheme.titleLarge, + overflow: TextOverflow.ellipsis, ), const SizedBox(width: Spacing.level4), ], @@ -69,36 +70,39 @@ class _TestResultView extends StatelessWidget { expandButtonPosition: YaruExpandableButtonPosition.start, child: Padding( padding: const EdgeInsets.only(left: Spacing.level6), - child: Wrap( - children: [ - testResult.comment != '' - ? Wrap( + child: Align( + alignment: Alignment.topLeft, + child: Column( + crossAxisAlignment: CrossAxisAlignment.start, + children: [ + if (testResult.comment != '') + RichText( + textAlign: TextAlign.left, + text: TextSpan( children: [ - Text( - 'Test Comment:', + TextSpan( + text: 'Test Comment:', style: Theme.of(context).textTheme.titleMedium, ), - Text( - testResult.comment, - ), + TextSpan(text: testResult.comment), ], - ) - : const SizedBox.shrink(), - testResult.ioLog != '' - ? Wrap( - alignment: WrapAlignment.start, + ), + ), + if (testResult.ioLog != '') + RichText( + textAlign: TextAlign.left, + text: TextSpan( children: [ - Text( - 'Test IO Log:', + TextSpan( + text: 'Test IO Log:', style: Theme.of(context).textTheme.titleMedium, ), - Text( - testResult.ioLog, - ), + TextSpan(text: testResult.ioLog), ], - ) - : const SizedBox.shrink(), - ], + ), + ), + ], + ), ), ), );