Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions backend/kernelCI_app/helpers/treeDetails.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: add a comment for what the skip_build_filters is for

"""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"])
Expand Down
Original file line number Diff line number Diff line change
@@ -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()
86 changes: 84 additions & 2 deletions backend/kernelCI_app/views/treeDetailsSummaryView.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand All @@ -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,
Expand Down Expand Up @@ -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)

Comment thread
gustavobtflores marked this conversation as resolved.
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,
Expand Down Expand Up @@ -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)
Comment thread
gustavobtflores marked this conversation as resolved.

try:
valid_response = SummaryResponse(
Expand Down
Loading