Skip to content

Commit 9bd0e65

Browse files
feat: implement fallback when tree_tests_rollup doesn't have data (#1853)
1 parent a3a2a71 commit 9bd0e65

3 files changed

Lines changed: 254 additions & 4 deletions

File tree

backend/kernelCI_app/helpers/treeDetails.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -478,12 +478,14 @@ def process_boots_summary(instance, row_data: dict[str, Any]) -> None:
478478
instance.boot_summary_typed.labs[test_lab].increment(test_status)
479479

480480

481-
def process_filters(instance, row_data: dict) -> None:
481+
def process_filters(instance, row_data: dict, skip_build_filters: bool = False) -> None:
482+
"""skip_build_filters is used on some calls to this function
483+
where we know that the build filters are already processed"""
482484
issue_id = row_data["issue_id"]
483485
issue_version = row_data["issue_version"]
484486
incident_test_id = row_data["incident_test_id"]
485487

486-
if row_data["build_id"] is not None:
488+
if not skip_build_filters and row_data["build_id"] is not None:
487489
build_status = row_data["build_status"]
488490
instance.global_configs.add(row_data["build_config_name"])
489491
instance.global_architectures.add(row_data["build_architecture"])
Lines changed: 166 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,166 @@
1+
from unittest.mock import patch
2+
from django.test import SimpleTestCase
3+
from rest_framework.test import APIRequestFactory
4+
5+
from kernelCI_app.views.treeDetailsSummaryView import TreeDetailsSummary
6+
from kernelCI_app.tests.unitTests.helpers.fixtures.tree_details_data import create_row
7+
8+
9+
COMMIT_HASH = "abc123def456rollupfallback"
10+
11+
BASE_BUILD_ROW: dict[str, object] = {
12+
"build_id": "build_fallback_001",
13+
"build_origin": "maestro",
14+
"build_comment": None,
15+
"build_start_time": None,
16+
"build_duration": None,
17+
"build_architecture": "x86_64",
18+
"build_command": None,
19+
"build_compiler": "gcc-12",
20+
"build_config_name": "defconfig",
21+
"build_config_url": None,
22+
"build_log_url": None,
23+
"build_status": "PASS",
24+
"build_misc": None,
25+
"checkout_id": "checkout_fallback_001",
26+
"checkout_git_repository_url": "https://git.kernel.org",
27+
"checkout_git_repository_branch": "for-kernelci",
28+
"checkout_git_commit_tags": [],
29+
"checkout_origin": "maestro",
30+
"incident_id": None,
31+
"incident_test_id": None,
32+
"incident_present": None,
33+
"issue_id": None,
34+
"issue_version": None,
35+
"issue_comment": None,
36+
"issue_report_url": None,
37+
}
38+
39+
# Legacy row representing a boot test (PASS, no issue)
40+
LEGACY_BOOT_ROW = create_row(
41+
test_id="legacy_boot_001",
42+
test_path="boot.test",
43+
test_status="PASS",
44+
incident_test_id="legacy_boot_001",
45+
issue_id=None,
46+
issue_version=None,
47+
)
48+
49+
# Legacy row representing a non-boot test (FAIL, no linked issue → unknown issue)
50+
LEGACY_TEST_ROW = create_row(
51+
test_id="legacy_test_001",
52+
test_path="test.something",
53+
test_status="FAIL",
54+
incident_test_id="legacy_test_001",
55+
issue_id=None,
56+
issue_version=None,
57+
)
58+
59+
60+
class TestTreeDetailsSummaryRollupFallback(SimpleTestCase):
61+
"""Tests for the rollup-empty fallback path in TreeDetailsSummary."""
62+
63+
def setUp(self):
64+
self.factory = APIRequestFactory()
65+
self.view = TreeDetailsSummary()
66+
self.url = f"/api/tree/{COMMIT_HASH}/summary"
67+
self.base_query = {
68+
"origin": "maestro",
69+
"git_url": "https://git.kernel.org",
70+
"git_branch": "for-kernelci",
71+
}
72+
73+
def _make_request(self):
74+
return self.factory.get(self.url, self.base_query)
75+
76+
@patch("kernelCI_app.views.treeDetailsSummaryView.out")
77+
@patch("kernelCI_app.views.treeDetailsSummaryView.get_tree_details_data")
78+
@patch("kernelCI_app.views.treeDetailsSummaryView.get_tree_details_builds")
79+
@patch("kernelCI_app.views.treeDetailsSummaryView.get_tree_details_rollup")
80+
def test_fallback_to_legacy_when_rollup_empty(
81+
self,
82+
mock_get_rollup,
83+
mock_get_builds,
84+
mock_get_legacy_data,
85+
mock_out,
86+
):
87+
"""When rollup is empty the endpoint falls back to legacy data and returns
88+
non-empty boot/test summaries."""
89+
mock_get_rollup.return_value = []
90+
mock_get_builds.return_value = [BASE_BUILD_ROW]
91+
mock_get_legacy_data.return_value = [LEGACY_BOOT_ROW, LEGACY_TEST_ROW]
92+
93+
response = self.view.get(self._make_request(), commit_hash=COMMIT_HASH)
94+
95+
self.assertEqual(response.status_code, 200)
96+
self.assertNotIn("error", response.data)
97+
98+
# Fallback was triggered and logged
99+
mock_out.assert_called_once()
100+
101+
# Boot summary is populated from legacy rows
102+
boot_status = response.data["summary"]["boots"]["status"]
103+
self.assertGreater(sum(boot_status.values()), 0)
104+
105+
# Test summary is populated from legacy rows
106+
test_status = response.data["summary"]["tests"]["status"]
107+
self.assertGreater(sum(test_status.values()), 0)
108+
109+
# Build summary is still correct (from _sanitize_builds_rows, not legacy)
110+
build_status = response.data["summary"]["builds"]["status"]
111+
self.assertGreater(sum(build_status.values()), 0)
112+
113+
@patch("kernelCI_app.views.treeDetailsSummaryView.out")
114+
@patch("kernelCI_app.views.treeDetailsSummaryView.get_tree_details_data")
115+
@patch("kernelCI_app.views.treeDetailsSummaryView.get_tree_details_builds")
116+
@patch("kernelCI_app.views.treeDetailsSummaryView.get_tree_details_rollup")
117+
def test_build_filter_flags_not_contaminated_by_legacy_test_rows(
118+
self,
119+
mock_get_rollup,
120+
mock_get_builds,
121+
mock_get_legacy_data,
122+
mock_out,
123+
):
124+
"""Build filter metadata must not be altered by test-incident rows processed
125+
in the legacy fallback path (fixes double-processing of build filter flags)."""
126+
# Build PASSES → has_unknown_issue for builds must stay False
127+
mock_get_rollup.return_value = []
128+
mock_get_builds.return_value = [BASE_BUILD_ROW]
129+
# A test row that has a FAIL status with incident_test_id set (test incident,
130+
# not a build incident). Without the fix this would incorrectly set the build
131+
# has_unknown_issue flag.
132+
mock_get_legacy_data.return_value = [LEGACY_TEST_ROW]
133+
134+
response = self.view.get(self._make_request(), commit_hash=COMMIT_HASH)
135+
136+
self.assertEqual(response.status_code, 200)
137+
self.assertNotIn("error", response.data)
138+
139+
# The build filter has_unknown_issue should be False: the build passed, and
140+
# the test-incident rows must not bleed into build filter metadata.
141+
builds_filter = response.data["filters"]["builds"]
142+
self.assertFalse(builds_filter["has_unknown_issue"])
143+
144+
@patch("kernelCI_app.views.treeDetailsSummaryView.out")
145+
@patch("kernelCI_app.views.treeDetailsSummaryView.get_tree_details_data")
146+
@patch("kernelCI_app.views.treeDetailsSummaryView.get_tree_details_builds")
147+
@patch("kernelCI_app.views.treeDetailsSummaryView.get_tree_details_rollup")
148+
def test_no_legacy_query_when_both_empty(
149+
self,
150+
mock_get_rollup,
151+
mock_get_builds,
152+
mock_get_legacy_data,
153+
mock_out,
154+
):
155+
"""When both builds and rollup are empty the guard clause returns a no-results
156+
error without attempting the legacy query."""
157+
mock_get_rollup.return_value = []
158+
mock_get_builds.return_value = []
159+
160+
response = self.view.get(self._make_request(), commit_hash=COMMIT_HASH)
161+
162+
self.assertEqual(response.status_code, 200)
163+
self.assertIn("error", response.data)
164+
165+
mock_get_legacy_data.assert_not_called()
166+
mock_out.assert_not_called()

backend/kernelCI_app/views/treeDetailsSummaryView.py

Lines changed: 84 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,20 @@
88
from kernelCI_app.helpers.discordWebhook import send_discord_notification
99
from kernelCI_app.helpers.filters import FilterParams
1010
from kernelCI_app.helpers.logger import create_endpoint_notification
11+
from kernelCI_app.helpers.logger import out
1112
from kernelCI_app.helpers.treeDetails import (
13+
call_based_on_compatible_and_misc_platform,
14+
decide_if_is_boot_filtered_out,
1215
decide_if_is_build_filtered_out,
16+
decide_if_is_full_row_filtered_out,
17+
decide_if_is_test_filtered_out,
1318
get_build,
19+
get_current_row_data,
20+
process_boots_summary,
1421
process_builds_issue,
22+
process_filters,
23+
process_test_summary,
24+
process_tests_issue,
1525
process_tree_url,
1626
)
1727
from kernelCI_app.helpers.treeDetailsRollup import (
@@ -22,7 +32,12 @@
2232
process_rollup_summary,
2333
rollup_test_or_boot_filtered_out,
2434
)
25-
from kernelCI_app.queries.tree import get_tree_details_builds, get_tree_details_rollup
35+
from kernelCI_app.queries.tree import (
36+
get_tree_details_builds,
37+
get_tree_details_data,
38+
get_tree_details_rollup,
39+
)
40+
from kernelCI_app.utils import is_boot
2641
from kernelCI_app.typeModels.commonDetails import (
2742
BaseBuildSummary,
2843
BuildSummary,
@@ -202,6 +217,56 @@ def _sanitize_rollup_rows(self, rollup_rows: list[dict]) -> None:
202217
issues_dict=self.test_issues_dict
203218
)
204219

220+
def _sanitize_test_rows_legacy(self, rows: list[tuple]) -> None:
221+
"""Process legacy test/boot rows when rollup data is not available.
222+
223+
This method processes only test/boot rows from the legacy get_tree_details_data()
224+
query. Builds are already handled by _sanitize_builds_rows().
225+
226+
Modeled after BaseTreeDetails._sanitize_rows() but without:
227+
- _process_builds() calls (builds already handled)
228+
- testHistory/bootHistory tracking (summary endpoint doesn't return individual test items)
229+
- git_commit_tags assignment (already set by _sanitize_builds_rows)
230+
- process_tree_url call (already set by _sanitize_builds_rows)
231+
"""
232+
processed_tests: set[str] = set() # local dedup set
233+
234+
for row in rows:
235+
row_data = get_current_row_data(row)
236+
237+
call_based_on_compatible_and_misc_platform(row_data, self.hardwareUsed.add)
238+
process_filters(self, row_data, skip_build_filters=True)
239+
240+
if decide_if_is_full_row_filtered_out(self, row_data):
241+
continue
242+
243+
if row_data["test_id"] is None:
244+
continue
245+
246+
test_id = row_data["test_id"]
247+
248+
if is_boot(row_data["test_path"]):
249+
if decide_if_is_boot_filtered_out(self, row_data):
250+
continue
251+
process_tests_issue(instance=self, row_data=row_data, is_boot=True)
252+
if test_id not in processed_tests:
253+
processed_tests.add(test_id)
254+
process_boots_summary(self, row_data)
255+
else:
256+
if decide_if_is_test_filtered_out(self, row_data):
257+
continue
258+
process_tests_issue(instance=self, row_data=row_data)
259+
if test_id not in processed_tests:
260+
processed_tests.add(test_id)
261+
process_test_summary(self, row_data)
262+
263+
self.bootIssues = convert_issues_dict_to_list_typed(
264+
issues_dict=self.boot_issues_dict
265+
)
266+
self.testIssues = convert_issues_dict_to_list_typed(
267+
issues_dict=self.test_issues_dict
268+
)
269+
205270
def get(
206271
self,
207272
request: HttpRequest,
@@ -244,7 +309,24 @@ def get(
244309
self.filters = FilterParams(request)
245310

246311
self._sanitize_builds_rows(builds_rows)
247-
self._sanitize_rollup_rows(rollup_rows or [])
312+
313+
if rollup_rows:
314+
self._sanitize_rollup_rows(rollup_rows)
315+
else:
316+
out(
317+
f"Rollup data empty for commit {commit_hash} "
318+
f"(origin={origin_param}, tree={tree_name}, git_url={git_url_param}), "
319+
f"falling back to legacy query"
320+
)
321+
legacy_rows = get_tree_details_data(
322+
origin_param=origin_param,
323+
git_url_param=git_url_param,
324+
tree_name=tree_name,
325+
git_branch_param=git_branch_param,
326+
commit_hash=commit_hash,
327+
)
328+
if legacy_rows:
329+
self._sanitize_test_rows_legacy(legacy_rows)
248330

249331
try:
250332
valid_response = SummaryResponse(

0 commit comments

Comments
 (0)