diff --git a/backend/kernelCI_app/helpers/treeDetails.py b/backend/kernelCI_app/helpers/treeDetails.py index 9448e2858..9946f2db4 100644 --- a/backend/kernelCI_app/helpers/treeDetails.py +++ b/backend/kernelCI_app/helpers/treeDetails.py @@ -478,12 +478,14 @@ def process_boots_summary(instance, row_data: dict[str, Any]) -> None: instance.boot_summary_typed.labs[test_lab].increment(test_status) -def process_filters(instance, row_data: dict) -> None: +def process_filters(instance, row_data: dict, skip_build_filters: bool = False) -> None: + """skip_build_filters is used on some calls to this function + where we know that the build filters are already processed""" issue_id = row_data["issue_id"] issue_version = row_data["issue_version"] incident_test_id = row_data["incident_test_id"] - if row_data["build_id"] is not None: + if not skip_build_filters and row_data["build_id"] is not None: build_status = row_data["build_status"] instance.global_configs.add(row_data["build_config_name"]) instance.global_architectures.add(row_data["build_architecture"]) diff --git a/backend/kernelCI_app/tests/unitTests/views/treeDetailsSummaryView_test.py b/backend/kernelCI_app/tests/unitTests/views/treeDetailsSummaryView_test.py new file mode 100644 index 000000000..055a087fc --- /dev/null +++ b/backend/kernelCI_app/tests/unitTests/views/treeDetailsSummaryView_test.py @@ -0,0 +1,166 @@ +from unittest.mock import patch +from django.test import SimpleTestCase +from rest_framework.test import APIRequestFactory + +from kernelCI_app.views.treeDetailsSummaryView import TreeDetailsSummary +from kernelCI_app.tests.unitTests.helpers.fixtures.tree_details_data import create_row + + +COMMIT_HASH = "abc123def456rollupfallback" + +BASE_BUILD_ROW: dict[str, object] = { + "build_id": "build_fallback_001", + "build_origin": "maestro", + "build_comment": None, + "build_start_time": None, + "build_duration": None, + "build_architecture": "x86_64", + "build_command": None, + "build_compiler": "gcc-12", + "build_config_name": "defconfig", + "build_config_url": None, + "build_log_url": None, + "build_status": "PASS", + "build_misc": None, + "checkout_id": "checkout_fallback_001", + "checkout_git_repository_url": "https://git.kernel.org", + "checkout_git_repository_branch": "for-kernelci", + "checkout_git_commit_tags": [], + "checkout_origin": "maestro", + "incident_id": None, + "incident_test_id": None, + "incident_present": None, + "issue_id": None, + "issue_version": None, + "issue_comment": None, + "issue_report_url": None, +} + +# Legacy row representing a boot test (PASS, no issue) +LEGACY_BOOT_ROW = create_row( + test_id="legacy_boot_001", + test_path="boot.test", + test_status="PASS", + incident_test_id="legacy_boot_001", + issue_id=None, + issue_version=None, +) + +# Legacy row representing a non-boot test (FAIL, no linked issue → unknown issue) +LEGACY_TEST_ROW = create_row( + test_id="legacy_test_001", + test_path="test.something", + test_status="FAIL", + incident_test_id="legacy_test_001", + issue_id=None, + issue_version=None, +) + + +class TestTreeDetailsSummaryRollupFallback(SimpleTestCase): + """Tests for the rollup-empty fallback path in TreeDetailsSummary.""" + + def setUp(self): + self.factory = APIRequestFactory() + self.view = TreeDetailsSummary() + self.url = f"/api/tree/{COMMIT_HASH}/summary" + self.base_query = { + "origin": "maestro", + "git_url": "https://git.kernel.org", + "git_branch": "for-kernelci", + } + + def _make_request(self): + return self.factory.get(self.url, self.base_query) + + @patch("kernelCI_app.views.treeDetailsSummaryView.out") + @patch("kernelCI_app.views.treeDetailsSummaryView.get_tree_details_data") + @patch("kernelCI_app.views.treeDetailsSummaryView.get_tree_details_builds") + @patch("kernelCI_app.views.treeDetailsSummaryView.get_tree_details_rollup") + def test_fallback_to_legacy_when_rollup_empty( + self, + mock_get_rollup, + mock_get_builds, + mock_get_legacy_data, + mock_out, + ): + """When rollup is empty the endpoint falls back to legacy data and returns + non-empty boot/test summaries.""" + mock_get_rollup.return_value = [] + mock_get_builds.return_value = [BASE_BUILD_ROW] + mock_get_legacy_data.return_value = [LEGACY_BOOT_ROW, LEGACY_TEST_ROW] + + response = self.view.get(self._make_request(), commit_hash=COMMIT_HASH) + + self.assertEqual(response.status_code, 200) + self.assertNotIn("error", response.data) + + # Fallback was triggered and logged + mock_out.assert_called_once() + + # Boot summary is populated from legacy rows + boot_status = response.data["summary"]["boots"]["status"] + self.assertGreater(sum(boot_status.values()), 0) + + # Test summary is populated from legacy rows + test_status = response.data["summary"]["tests"]["status"] + self.assertGreater(sum(test_status.values()), 0) + + # Build summary is still correct (from _sanitize_builds_rows, not legacy) + build_status = response.data["summary"]["builds"]["status"] + self.assertGreater(sum(build_status.values()), 0) + + @patch("kernelCI_app.views.treeDetailsSummaryView.out") + @patch("kernelCI_app.views.treeDetailsSummaryView.get_tree_details_data") + @patch("kernelCI_app.views.treeDetailsSummaryView.get_tree_details_builds") + @patch("kernelCI_app.views.treeDetailsSummaryView.get_tree_details_rollup") + def test_build_filter_flags_not_contaminated_by_legacy_test_rows( + self, + mock_get_rollup, + mock_get_builds, + mock_get_legacy_data, + mock_out, + ): + """Build filter metadata must not be altered by test-incident rows processed + in the legacy fallback path (fixes double-processing of build filter flags).""" + # Build PASSES → has_unknown_issue for builds must stay False + mock_get_rollup.return_value = [] + mock_get_builds.return_value = [BASE_BUILD_ROW] + # A test row that has a FAIL status with incident_test_id set (test incident, + # not a build incident). Without the fix this would incorrectly set the build + # has_unknown_issue flag. + mock_get_legacy_data.return_value = [LEGACY_TEST_ROW] + + response = self.view.get(self._make_request(), commit_hash=COMMIT_HASH) + + self.assertEqual(response.status_code, 200) + self.assertNotIn("error", response.data) + + # The build filter has_unknown_issue should be False: the build passed, and + # the test-incident rows must not bleed into build filter metadata. + builds_filter = response.data["filters"]["builds"] + self.assertFalse(builds_filter["has_unknown_issue"]) + + @patch("kernelCI_app.views.treeDetailsSummaryView.out") + @patch("kernelCI_app.views.treeDetailsSummaryView.get_tree_details_data") + @patch("kernelCI_app.views.treeDetailsSummaryView.get_tree_details_builds") + @patch("kernelCI_app.views.treeDetailsSummaryView.get_tree_details_rollup") + def test_no_legacy_query_when_both_empty( + self, + mock_get_rollup, + mock_get_builds, + mock_get_legacy_data, + mock_out, + ): + """When both builds and rollup are empty the guard clause returns a no-results + error without attempting the legacy query.""" + mock_get_rollup.return_value = [] + mock_get_builds.return_value = [] + + response = self.view.get(self._make_request(), commit_hash=COMMIT_HASH) + + self.assertEqual(response.status_code, 200) + self.assertIn("error", response.data) + + mock_get_legacy_data.assert_not_called() + mock_out.assert_not_called() diff --git a/backend/kernelCI_app/views/treeDetailsSummaryView.py b/backend/kernelCI_app/views/treeDetailsSummaryView.py index 53268134f..e2b15ed22 100644 --- a/backend/kernelCI_app/views/treeDetailsSummaryView.py +++ b/backend/kernelCI_app/views/treeDetailsSummaryView.py @@ -8,10 +8,20 @@ from kernelCI_app.helpers.discordWebhook import send_discord_notification from kernelCI_app.helpers.filters import FilterParams from kernelCI_app.helpers.logger import create_endpoint_notification +from kernelCI_app.helpers.logger import out from kernelCI_app.helpers.treeDetails import ( + call_based_on_compatible_and_misc_platform, + decide_if_is_boot_filtered_out, decide_if_is_build_filtered_out, + decide_if_is_full_row_filtered_out, + decide_if_is_test_filtered_out, get_build, + get_current_row_data, + process_boots_summary, process_builds_issue, + process_filters, + process_test_summary, + process_tests_issue, process_tree_url, ) from kernelCI_app.helpers.treeDetailsRollup import ( @@ -22,7 +32,12 @@ process_rollup_summary, rollup_test_or_boot_filtered_out, ) -from kernelCI_app.queries.tree import get_tree_details_builds, get_tree_details_rollup +from kernelCI_app.queries.tree import ( + get_tree_details_builds, + get_tree_details_data, + get_tree_details_rollup, +) +from kernelCI_app.utils import is_boot from kernelCI_app.typeModels.commonDetails import ( BaseBuildSummary, BuildSummary, @@ -202,6 +217,56 @@ def _sanitize_rollup_rows(self, rollup_rows: list[dict]) -> None: issues_dict=self.test_issues_dict ) + def _sanitize_test_rows_legacy(self, rows: list[tuple]) -> None: + """Process legacy test/boot rows when rollup data is not available. + + This method processes only test/boot rows from the legacy get_tree_details_data() + query. Builds are already handled by _sanitize_builds_rows(). + + Modeled after BaseTreeDetails._sanitize_rows() but without: + - _process_builds() calls (builds already handled) + - testHistory/bootHistory tracking (summary endpoint doesn't return individual test items) + - git_commit_tags assignment (already set by _sanitize_builds_rows) + - process_tree_url call (already set by _sanitize_builds_rows) + """ + processed_tests: set[str] = set() # local dedup set + + for row in rows: + row_data = get_current_row_data(row) + + call_based_on_compatible_and_misc_platform(row_data, self.hardwareUsed.add) + process_filters(self, row_data, skip_build_filters=True) + + if decide_if_is_full_row_filtered_out(self, row_data): + continue + + if row_data["test_id"] is None: + continue + + test_id = row_data["test_id"] + + if is_boot(row_data["test_path"]): + if decide_if_is_boot_filtered_out(self, row_data): + continue + process_tests_issue(instance=self, row_data=row_data, is_boot=True) + if test_id not in processed_tests: + processed_tests.add(test_id) + process_boots_summary(self, row_data) + else: + if decide_if_is_test_filtered_out(self, row_data): + continue + process_tests_issue(instance=self, row_data=row_data) + if test_id not in processed_tests: + processed_tests.add(test_id) + process_test_summary(self, row_data) + + self.bootIssues = convert_issues_dict_to_list_typed( + issues_dict=self.boot_issues_dict + ) + self.testIssues = convert_issues_dict_to_list_typed( + issues_dict=self.test_issues_dict + ) + def get( self, request: HttpRequest, @@ -244,7 +309,24 @@ def get( self.filters = FilterParams(request) self._sanitize_builds_rows(builds_rows) - self._sanitize_rollup_rows(rollup_rows or []) + + if rollup_rows: + self._sanitize_rollup_rows(rollup_rows) + else: + out( + f"Rollup data empty for commit {commit_hash} " + f"(origin={origin_param}, tree={tree_name}, git_url={git_url_param}), " + f"falling back to legacy query" + ) + legacy_rows = get_tree_details_data( + origin_param=origin_param, + git_url_param=git_url_param, + tree_name=tree_name, + git_branch_param=git_branch_param, + commit_hash=commit_hash, + ) + if legacy_rows: + self._sanitize_test_rows_legacy(legacy_rows) try: valid_response = SummaryResponse(