From c8fc570e28b603c04ad7916627d49da787475533 Mon Sep 17 00:00:00 2001 From: Felipe Bergamin Date: Thu, 16 Jul 2026 10:35:27 -0300 Subject: [PATCH 1/8] chore: add dev-down to Makefile Signed-off-by: Felipe Bergamin --- Makefile | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Makefile b/Makefile index d5c4b5e36..59d2a4d82 100644 --- a/Makefile +++ b/Makefile @@ -50,6 +50,9 @@ ci: ## Reproduce full CI pipeline (lint + build + test + integration) dev: ## Start development environment with docker-compose.dev.yml docker compose -f docker-compose.dev.yml up -d +dev-down: ## Stop development environment with docker-compose.dev.yml + docker compose -f docker-compose.dev.yml down + dev-build: ## Start development environment and rebuild images docker compose -f docker-compose.dev.yml up --build -d From 8dcfe9dc4059a6dc43d3138f20f15a5d9057216d Mon Sep 17 00:00:00 2001 From: Felipe Bergamin Date: Thu, 16 Jul 2026 16:27:45 -0300 Subject: [PATCH 2/8] feat(tree-compare): add backend endpoint to compare tree commits Part of #1951 Signed-off-by: Felipe Bergamin --- backend/kernelCI_app/helpers/treeCompare.py | 220 ++++++++++++++++++ backend/kernelCI_app/queries/tree.py | 172 ++++++++++++++ .../queries/treeCompareQueries_test.py | 67 ++++++ .../unitTests/views/treeCompareView_test.py | 196 ++++++++++++++++ .../kernelCI_app/typeModels/treeCompare.py | 68 ++++++ backend/kernelCI_app/urls.py | 5 + .../views/hardwareDetailsSummaryView.py | 2 +- .../kernelCI_app/views/treeCommitsHistory.py | 2 +- backend/kernelCI_app/views/treeCompareView.py | 123 ++++++++++ backend/requests/tree-compare-get.sh | 21 ++ 10 files changed, 874 insertions(+), 2 deletions(-) create mode 100644 backend/kernelCI_app/helpers/treeCompare.py create mode 100644 backend/kernelCI_app/tests/unitTests/queries/treeCompareQueries_test.py create mode 100644 backend/kernelCI_app/tests/unitTests/views/treeCompareView_test.py create mode 100644 backend/kernelCI_app/typeModels/treeCompare.py create mode 100644 backend/kernelCI_app/views/treeCompareView.py create mode 100644 backend/requests/tree-compare-get.sh diff --git a/backend/kernelCI_app/helpers/treeCompare.py b/backend/kernelCI_app/helpers/treeCompare.py new file mode 100644 index 000000000..be208ebd7 --- /dev/null +++ b/backend/kernelCI_app/helpers/treeCompare.py @@ -0,0 +1,220 @@ +from collections import defaultdict +from dataclasses import dataclass, field +from typing import Callable, Literal, Optional + +from kernelCI_app.constants.general import UNKNOWN_STRING +from kernelCI_app.constants.process_pending import ROLLUP_STATUS_FIELDS +from kernelCI_app.typeModels.treeCompare import ( + CompareDelta, + CompareEntitySummary, + CompareGroupRow, + CompareGroups, + CompareStatusCounts, + CompareSummary, + TreeCompareResponse, +) + +BucketKey = Literal["pass", "fail", "inconclusive"] + + +@dataclass +class _HashAccumulator: + builds: CompareStatusCounts = field(default_factory=CompareStatusCounts) + boots: CompareStatusCounts = field(default_factory=CompareStatusCounts) + tests: CompareStatusCounts = field(default_factory=CompareStatusCounts) + build_groups: dict[str, CompareStatusCounts] = field( + default_factory=lambda: defaultdict(CompareStatusCounts) + ) + boot_groups: dict[str, CompareStatusCounts] = field( + default_factory=lambda: defaultdict(CompareStatusCounts) + ) + test_groups: dict[str, CompareStatusCounts] = field( + default_factory=lambda: defaultdict(CompareStatusCounts) + ) + + +def _empty_counts() -> CompareStatusCounts: + return CompareStatusCounts() + + +def _status_to_bucket(status: Optional[str]) -> BucketKey: + if status is None: + return "inconclusive" + normalized = status.upper() + if normalized == "PASS": + return "pass" + if normalized == "FAIL": + return "fail" + return "inconclusive" + + +def _increment_bucket( + counts: CompareStatusCounts, bucket: BucketKey, amount: int +) -> None: + if amount <= 0: + return + if bucket == "pass": + counts.pass_count += amount + elif bucket == "fail": + counts.fail_count += amount + else: + counts.inconclusive += amount + + +def _rollup_status_to_bucket(status_name: str) -> BucketKey: + if status_name == "PASS": + return "pass" + if status_name == "FAIL": + return "fail" + return "inconclusive" + + +def _boot_group_key(row_dict: dict) -> str: + return ( + row_dict.get("test_platform") + or row_dict.get("hardware_key") + or UNKNOWN_STRING + ) + + +def _test_group_key(row_dict: dict) -> str: + path_group = row_dict.get("path_group") or UNKNOWN_STRING + arch = row_dict.get("build_architecture") or UNKNOWN_STRING + if arch == UNKNOWN_STRING: + return path_group + return f"{path_group}/{arch}" + + +def process_rollup_rows( + *, + rows: list[dict], + commit_hashes: list[str], +) -> dict[str, _HashAccumulator]: + accumulators = {commit_hash: _HashAccumulator() for commit_hash in commit_hashes} + + for row_dict in rows: + commit_hash = row_dict["git_commit_hash"] + is_boot_row = row_dict["is_boot"] + acc = accumulators.setdefault(commit_hash, _HashAccumulator()) + group_id = _boot_group_key(row_dict) if is_boot_row else _test_group_key(row_dict) + target = acc.boots if is_boot_row else acc.tests + group_map = acc.boot_groups if is_boot_row else acc.test_groups + + for status_name, field_name in ROLLUP_STATUS_FIELDS.items(): + count = row_dict.get(field_name, 0) or 0 + if count <= 0: + continue + bucket = _rollup_status_to_bucket(status_name) + _increment_bucket(target, bucket, count) + _increment_bucket(group_map[group_id], bucket, count) + + return accumulators + + +def process_build_rows( + *, + rows: list[dict], + commit_hashes: list[str], +) -> dict[str, _HashAccumulator]: + accumulators = {commit_hash: _HashAccumulator() for commit_hash in commit_hashes} + + for row in rows: + commit_hash = row["git_commit_hash"] + config = row.get("config_name") or UNKNOWN_STRING + count = row.get("count") or 0 + acc = accumulators.setdefault(commit_hash, _HashAccumulator()) + bucket = _status_to_bucket(row.get("status")) + _increment_bucket(acc.builds, bucket, count) + _increment_bucket(acc.build_groups[config], bucket, count) + + return accumulators + + +def _make_delta(side_a: CompareStatusCounts, side_b: CompareStatusCounts) -> CompareDelta: + return CompareDelta( + **{ + "pass": side_b.pass_count - side_a.pass_count, + "fail": side_b.fail_count - side_a.fail_count, + } + ) + + +def _make_entity_summary( + *, + side_a: CompareStatusCounts, + side_b: CompareStatusCounts, +) -> CompareEntitySummary: + return CompareEntitySummary( + sideA=side_a, + sideB=side_b, + delta=_make_delta(side_a, side_b), + ) + + +def _make_group_rows( + *, + group_maps: tuple[dict[str, CompareStatusCounts], dict[str, CompareStatusCounts]], + label_fn: Optional[Callable[[str], str]] = None, +) -> list[CompareGroupRow]: + side_a_groups, side_b_groups = group_maps + all_ids = set(side_a_groups) | set(side_b_groups) + rows: list[CompareGroupRow] = [] + + for group_id in all_ids: + side_a = side_a_groups.get(group_id, _empty_counts()) + side_b = side_b_groups.get(group_id, _empty_counts()) + label = label_fn(group_id) if label_fn else group_id + rows.append( + CompareGroupRow( + id=group_id, + label=label, + sideA=side_a, + sideB=side_b, + delta=_make_delta(side_a, side_b), + ) + ) + + return rows + + +def _test_group_label(group_id: str) -> str: + if "/" in group_id: + path_group, arch = group_id.split("/", 1) + return f"{path_group} · {arch}" + return group_id + + +def build_compare_response( + *, + hash_a: str, + hash_b: str, + tree_name: str, + branch: str, + git_url: str, + accumulators: dict[str, _HashAccumulator], +) -> TreeCompareResponse: + acc_a = accumulators.get(hash_a, _HashAccumulator()) + acc_b = accumulators.get(hash_b, _HashAccumulator()) + + return TreeCompareResponse( + treeName=tree_name, + branch=branch, + gitUrl=git_url, + summary=CompareSummary( + builds=_make_entity_summary(side_a=acc_a.builds, side_b=acc_b.builds), + boots=_make_entity_summary(side_a=acc_a.boots, side_b=acc_b.boots), + tests=_make_entity_summary(side_a=acc_a.tests, side_b=acc_b.tests), + ), + groups=CompareGroups( + builds=_make_group_rows( + group_maps=(acc_a.build_groups, acc_b.build_groups), + ), + boots=_make_group_rows( + group_maps=(acc_a.boot_groups, acc_b.boot_groups), + ), + tests=_make_group_rows( + group_maps=(acc_a.test_groups, acc_b.test_groups), + label_fn=_test_group_label, + ), + ), + ) diff --git a/backend/kernelCI_app/queries/tree.py b/backend/kernelCI_app/queries/tree.py index 52f755a1f..0e1d64b49 100644 --- a/backend/kernelCI_app/queries/tree.py +++ b/backend/kernelCI_app/queries/tree.py @@ -1204,3 +1204,175 @@ def get_latest_tree( query = query.order_by("-start_time").first() return query + + +def _get_compare_checkout_clauses( + *, + git_branch_param: Optional[str], + tree_name: Optional[str], +) -> tuple[str, str]: + checkout_clauses = create_checkouts_where_clauses( + git_url=None, + git_branch=git_branch_param, + tree_name=tree_name, + ) + + git_branch_clause = checkout_clauses.get("git_branch_clause") + tree_name_clause = checkout_clauses.get("tree_name_clause") + tree_name_full_clause = "AND " + tree_name_clause if tree_name_clause else "" + + return git_branch_clause, tree_name_full_clause + + +def get_tree_compare_rollup( + *, + commit_hashes: list[str], + origin_param: str, + git_branch_param: Optional[str], + tree_name: Optional[str] = None, +) -> list[dict]: + if not commit_hashes: + return [] + + cache_key = "treeCompareRollup" + params = { + "commit_hashes": commit_hashes, + "tree_name": tree_name, + "origin_param": origin_param, + "git_branch_param": git_branch_param, + } + + rows = get_query_cache(cache_key, params) + if rows is not None: + return rows + + git_branch_clause, tree_name_full_clause = _get_compare_checkout_clauses( + git_branch_param=git_branch_param, + tree_name=tree_name, + ) + + query = f""" + WITH RELEVANT_CHECKOUTS AS ( + SELECT DISTINCT ON (c.git_commit_hash) + c.git_commit_hash, + c.tree_name, + c.git_repository_branch, + c.git_repository_url, + c.origin + FROM + checkouts c + WHERE + c.git_commit_hash = ANY(%(commit_hashes)s) + {tree_name_full_clause} + AND {git_branch_clause} + AND c.origin = %(origin_param)s + ORDER BY + c.git_commit_hash, + c._timestamp DESC + ) + SELECT + tr.git_commit_hash, + tr.path_group, + tr.build_architecture, + tr.hardware_key, + tr.test_platform, + tr.is_boot, + tr.pass_tests, + tr.fail_tests, + tr.skip_tests, + tr.error_tests, + tr.miss_tests, + tr.done_tests, + tr.null_tests, + tr.total_tests + FROM + tree_tests_rollup tr + INNER JOIN RELEVANT_CHECKOUTS rc ON ( + tr.git_commit_hash = rc.git_commit_hash + AND tr.origin = rc.origin + AND tr.tree_name IS NOT DISTINCT FROM rc.tree_name + AND tr.git_repository_branch IS NOT DISTINCT FROM rc.git_repository_branch + AND tr.git_repository_url IS NOT DISTINCT FROM rc.git_repository_url + ) + ORDER BY + tr.total_tests DESC + """ + + with connection.cursor() as cursor: + cursor.execute(query, params) + rows = dict_fetchall(cursor=cursor) + set_query_cache(key=cache_key, params=params, rows=rows) + + return rows + + +def get_tree_compare_builds( + *, + commit_hashes: list[str], + origin_param: str, + git_branch_param: Optional[str], + tree_name: Optional[str] = None, +) -> list[dict]: + if not commit_hashes: + return [] + + cache_key = "treeCompareBuilds" + params = { + "commit_hashes": commit_hashes, + "tree_name": tree_name, + "origin_param": origin_param, + "git_branch_param": git_branch_param, + } + + rows = get_query_cache(cache_key, params) + if rows is not None: + return rows + + git_branch_clause, tree_name_full_clause = _get_compare_checkout_clauses( + git_branch_param=git_branch_param, + tree_name=tree_name, + ) + + query = f""" + WITH RELEVANT_CHECKOUTS AS ( + SELECT DISTINCT ON (c.git_commit_hash) + c.id AS checkout_id, + c.git_commit_hash, + c.git_repository_url + FROM + checkouts c + WHERE + c.git_commit_hash = ANY(%(commit_hashes)s) + {tree_name_full_clause} + AND {git_branch_clause} + AND c.origin = %(origin_param)s + ORDER BY + c.git_commit_hash, + c._timestamp DESC + ) + SELECT + rc.git_commit_hash, + rc.git_repository_url, + b.config_name, + b.status, + COUNT(DISTINCT b.id) AS count + FROM + RELEVANT_CHECKOUTS rc + INNER JOIN builds b ON b.checkout_id = rc.checkout_id + WHERE + b.config_name IS NOT NULL + AND b.id NOT LIKE 'maestro:dummy_%%' + GROUP BY + rc.git_commit_hash, + rc.git_repository_url, + b.config_name, + b.status + """ + + with connection.cursor() as cursor: + cursor.execute(query, params) + rows = dict_fetchall(cursor=cursor) + set_query_cache(key=cache_key, params=params, rows=rows) + + return rows + diff --git a/backend/kernelCI_app/tests/unitTests/queries/treeCompareQueries_test.py b/backend/kernelCI_app/tests/unitTests/queries/treeCompareQueries_test.py new file mode 100644 index 000000000..0d213e398 --- /dev/null +++ b/backend/kernelCI_app/tests/unitTests/queries/treeCompareQueries_test.py @@ -0,0 +1,67 @@ +from unittest.mock import MagicMock, patch + +from django.test import SimpleTestCase + +from kernelCI_app.queries.tree import ( + get_tree_compare_builds, + get_tree_compare_rollup, +) + + +class TestTreeCompareQueries(SimpleTestCase): + """Query-shape checks for the compare endpoint performance path.""" + + @patch("kernelCI_app.queries.tree.connection") + @patch("kernelCI_app.queries.tree.get_query_cache", return_value=None) + @patch("kernelCI_app.queries.tree.set_query_cache") + @patch("kernelCI_app.queries.tree.dict_fetchall", return_value=[]) + def test_compare_rollup_uses_single_any_query( + self, + mock_fetchall, + mock_set_cache, + mock_get_cache, + mock_connection, + ): + mock_cursor = MagicMock() + mock_connection.cursor.return_value.__enter__.return_value = mock_cursor + + get_tree_compare_rollup( + commit_hashes=["hash_a", "hash_b"], + origin_param="maestro", + git_branch_param="master", + tree_name="linux", + ) + + executed_query = mock_cursor.execute.call_args[0][0] + params = mock_cursor.execute.call_args[0][1] + self.assertIn("git_commit_hash = ANY(%(commit_hashes)s)", executed_query) + self.assertIn("DISTINCT ON (c.git_commit_hash)", executed_query) + self.assertIn("tree_tests_rollup", executed_query) + self.assertNotIn("git_url_param", params) + + @patch("kernelCI_app.queries.tree.connection") + @patch("kernelCI_app.queries.tree.get_query_cache", return_value=None) + @patch("kernelCI_app.queries.tree.set_query_cache") + @patch("kernelCI_app.queries.tree.dict_fetchall", return_value=[]) + def test_compare_builds_aggregates_in_sql( + self, + mock_fetchall, + mock_set_cache, + mock_get_cache, + mock_connection, + ): + mock_cursor = MagicMock() + mock_connection.cursor.return_value.__enter__.return_value = mock_cursor + + get_tree_compare_builds( + commit_hashes=["hash_a", "hash_b"], + origin_param="maestro", + git_branch_param="master", + tree_name="linux", + ) + + executed_query = mock_cursor.execute.call_args[0][0] + self.assertIn("COUNT(DISTINCT b.id)", executed_query) + self.assertIn("GROUP BY", executed_query) + self.assertNotIn("known_issues", executed_query) + self.assertNotIn("SELECT\n b.id AS build_id", executed_query) diff --git a/backend/kernelCI_app/tests/unitTests/views/treeCompareView_test.py b/backend/kernelCI_app/tests/unitTests/views/treeCompareView_test.py new file mode 100644 index 000000000..b188375c2 --- /dev/null +++ b/backend/kernelCI_app/tests/unitTests/views/treeCompareView_test.py @@ -0,0 +1,196 @@ +from unittest.mock import patch + +from django.test import SimpleTestCase +from rest_framework.test import APIRequestFactory + +from kernelCI_app.helpers.treeCompare import ( + build_compare_response, + process_build_rows, + process_rollup_rows, +) +from kernelCI_app.views.treeCompareView import TreeCompareView + +HASH_A = "a" * 40 +HASH_B = "b" * 40 + +ROLLUP_BOOT_ROW = { + "git_commit_hash": HASH_A, + "path_group": "boot", + "build_architecture": "arm64", + "hardware_key": "qemu-arm64", + "test_platform": "qemu-arm64", + "is_boot": True, + "pass_tests": 10, + "fail_tests": 2, + "skip_tests": 0, + "error_tests": 1, + "miss_tests": 0, + "done_tests": 0, + "null_tests": 0, + "total_tests": 13, +} + +ROLLUP_TEST_ROW_A = { + **ROLLUP_BOOT_ROW, + "git_commit_hash": HASH_A, + "path_group": "kselftest", + "build_architecture": "x86_64", + "is_boot": False, + "pass_tests": 20, + "fail_tests": 5, + "error_tests": 0, + "total_tests": 25, +} + +ROLLUP_TEST_ROW_B = { + **ROLLUP_TEST_ROW_A, + "git_commit_hash": HASH_B, + "pass_tests": 18, + "fail_tests": 7, + "total_tests": 25, +} + +BUILD_ROW_A = { + "git_commit_hash": HASH_A, + "git_repository_url": "https://git.kernel.org/linux.git", + "config_name": "arm64-defconfig", + "status": "PASS", + "count": 3, +} + +BUILD_ROW_B = { + **BUILD_ROW_A, + "git_commit_hash": HASH_B, + "status": "FAIL", + "count": 2, +} + + +class TestTreeCompareHelper(SimpleTestCase): + def test_process_rollup_rows_aggregates_summary_and_groups(self): + accumulators = process_rollup_rows( + rows=[ROLLUP_BOOT_ROW, ROLLUP_TEST_ROW_A], + commit_hashes=[HASH_A], + ) + + acc = accumulators[HASH_A] + self.assertEqual(acc.boots.pass_count, 10) + self.assertEqual(acc.boots.fail_count, 2) + self.assertEqual(acc.boots.inconclusive, 1) + self.assertEqual(acc.tests.pass_count, 20) + self.assertEqual(acc.tests.fail_count, 5) + self.assertIn("qemu-arm64", acc.boot_groups) + self.assertIn("kselftest/x86_64", acc.test_groups) + + def test_process_build_rows_groups_by_config(self): + accumulators = process_build_rows( + rows=[BUILD_ROW_A, BUILD_ROW_B], + commit_hashes=[HASH_A, HASH_B], + ) + + self.assertEqual(accumulators[HASH_A].builds.pass_count, 3) + self.assertEqual(accumulators[HASH_B].builds.fail_count, 2) + self.assertEqual( + accumulators[HASH_A].build_groups["arm64-defconfig"].pass_count, 3 + ) + + def test_build_compare_response_computes_deltas(self): + rollup_data = process_rollup_rows( + rows=[ROLLUP_TEST_ROW_A, ROLLUP_TEST_ROW_B], + commit_hashes=[HASH_A, HASH_B], + ) + build_data = process_build_rows( + rows=[BUILD_ROW_A, BUILD_ROW_B], + commit_hashes=[HASH_A, HASH_B], + ) + + accumulators = { + HASH_A: rollup_data[HASH_A], + HASH_B: rollup_data[HASH_B], + } + accumulators[HASH_A].builds = build_data[HASH_A].builds + accumulators[HASH_A].build_groups = build_data[HASH_A].build_groups + accumulators[HASH_B].builds = build_data[HASH_B].builds + accumulators[HASH_B].build_groups = build_data[HASH_B].build_groups + + response = build_compare_response( + hash_a=HASH_A, + hash_b=HASH_B, + tree_name="linux", + branch="master", + git_url="https://git.kernel.org/linux.git", + accumulators=accumulators, + ) + + payload = response.model_dump(by_alias=True) + self.assertEqual(payload["summary"]["tests"]["delta"]["pass"], -2) + self.assertEqual(payload["summary"]["tests"]["delta"]["fail"], 2) + self.assertEqual(payload["summary"]["builds"]["delta"]["pass"], -3) + self.assertEqual(payload["summary"]["builds"]["delta"]["fail"], 2) + + +class TestTreeCompareView(SimpleTestCase): + def setUp(self): + self.factory = APIRequestFactory() + self.view = TreeCompareView.as_view() + self.url = "/api/tree/linux/master/compare" + + @patch("kernelCI_app.views.treeCompareView.get_tree_compare_rollup") + @patch("kernelCI_app.views.treeCompareView.get_tree_compare_builds") + def test_get_returns_compare_payload(self, mock_builds, mock_rollup): + mock_builds.return_value = [BUILD_ROW_A, BUILD_ROW_B] + mock_rollup.return_value = [ + ROLLUP_BOOT_ROW, + ROLLUP_TEST_ROW_A, + ROLLUP_TEST_ROW_B, + ] + + request = self.factory.get( + self.url, + { + "hash_a": HASH_A, + "hash_b": HASH_B, + "origin": "maestro", + }, + ) + response = self.view(request, tree_name="linux", git_branch="master") + + self.assertEqual(response.status_code, 200) + self.assertEqual(response.data["treeName"], "linux") + self.assertEqual(response.data["branch"], "master") + self.assertIn("summary", response.data) + self.assertIn("groups", response.data) + mock_builds.assert_called_once() + mock_rollup.assert_called_once() + self.assertNotIn("git_url_param", mock_builds.call_args.kwargs) + + def test_missing_hashes_returns_400(self): + request = self.factory.get( + self.url, + { + "hash_a": HASH_A, + "origin": "maestro", + }, + ) + response = self.view(request, tree_name="linux", git_branch="master") + self.assertEqual(response.status_code, 400) + + @patch("kernelCI_app.views.treeCompareView.get_tree_compare_rollup") + @patch("kernelCI_app.views.treeCompareView.get_tree_compare_builds") + def test_missing_checkout_returns_zero_counts(self, mock_builds, mock_rollup): + mock_builds.return_value = [] + mock_rollup.return_value = [] + + request = self.factory.get( + self.url, + { + "hash_a": HASH_A, + "hash_b": HASH_B, + "origin": "maestro", + }, + ) + response = self.view(request, tree_name="linux", git_branch="master") + + self.assertEqual(response.status_code, 200) + self.assertEqual(response.data["summary"]["builds"]["sideA"]["pass"], 0) + self.assertEqual(response.data["summary"]["tests"]["sideB"]["fail"], 0) diff --git a/backend/kernelCI_app/typeModels/treeCompare.py b/backend/kernelCI_app/typeModels/treeCompare.py new file mode 100644 index 000000000..bd303e09d --- /dev/null +++ b/backend/kernelCI_app/typeModels/treeCompare.py @@ -0,0 +1,68 @@ +from pydantic import BaseModel, ConfigDict, Field + +from kernelCI_app.constants.general import DEFAULT_ORIGIN +from kernelCI_app.constants.localization import DocStrings + + +class CompareStatusCounts(BaseModel): + model_config = ConfigDict(populate_by_name=True) + + pass_count: int = Field(alias="pass", default=0) + fail_count: int = Field(alias="fail", default=0) + inconclusive: int = 0 + + +class CompareDelta(BaseModel): + model_config = ConfigDict(populate_by_name=True) + + pass_count: int = Field(alias="pass", default=0) + fail_count: int = Field(alias="fail", default=0) + + +class CompareEntitySummary(BaseModel): + model_config = ConfigDict(populate_by_name=True) + + side_a: CompareStatusCounts = Field(alias="sideA") + side_b: CompareStatusCounts = Field(alias="sideB") + delta: CompareDelta + + +class CompareGroupRow(BaseModel): + model_config = ConfigDict(populate_by_name=True) + + id: str + label: str + side_a: CompareStatusCounts = Field(alias="sideA") + side_b: CompareStatusCounts = Field(alias="sideB") + delta: CompareDelta + + +class CompareSummary(BaseModel): + builds: CompareEntitySummary + boots: CompareEntitySummary + tests: CompareEntitySummary + + +class CompareGroups(BaseModel): + builds: list[CompareGroupRow] + boots: list[CompareGroupRow] + tests: list[CompareGroupRow] + + +class TreeCompareResponse(BaseModel): + model_config = ConfigDict(populate_by_name=True) + + tree_name: str = Field(alias="treeName") + branch: str + git_url: str = Field(alias="gitUrl", default="") + summary: CompareSummary + groups: CompareGroups + + +class TreeCompareQueryParameters(BaseModel): + hash_a: str = Field(description="Commit hash for side A") + hash_b: str = Field(description="Commit hash for side B") + origin: str = Field( + default=DEFAULT_ORIGIN, + description=DocStrings.TREE_QUERY_ORIGIN_DESCRIPTION, + ) diff --git a/backend/kernelCI_app/urls.py b/backend/kernelCI_app/urls.py index a89890862..57f5a37fa 100644 --- a/backend/kernelCI_app/urls.py +++ b/backend/kernelCI_app/urls.py @@ -92,6 +92,11 @@ def view_cache(view, timeout: int = settings.CACHE_TIMEOUT): views.TreeDetailsTestsDirect.as_view(), name="treeDetailsTestsDirectView", ), + path( + "tree///compare", + view_cache(views.TreeCompareView), + name="treeCompare", + ), path( "tree//", view_cache(views.TreeLatest), diff --git a/backend/kernelCI_app/views/hardwareDetailsSummaryView.py b/backend/kernelCI_app/views/hardwareDetailsSummaryView.py index 499236e62..48d869c35 100644 --- a/backend/kernelCI_app/views/hardwareDetailsSummaryView.py +++ b/backend/kernelCI_app/views/hardwareDetailsSummaryView.py @@ -128,7 +128,7 @@ def filter_instance( return True filtered_issues = filters.filterIssues.get(filter_type, set()) - if filtered_issues and not known_issues.issubset(filtered_issues): + if filtered_issues and known_issues.isdisjoint(filtered_issues): return True return False diff --git a/backend/kernelCI_app/views/treeCommitsHistory.py b/backend/kernelCI_app/views/treeCommitsHistory.py index 0d1860580..f658194cc 100644 --- a/backend/kernelCI_app/views/treeCommitsHistory.py +++ b/backend/kernelCI_app/views/treeCommitsHistory.py @@ -592,7 +592,7 @@ def filter_instance( if is_filtered_out(architecture, filters.filterArchitecture): return True filtered_issues = filters.filterIssues.get(filter_type, set()) - if filtered_issues and not known_issues.issubset(filtered_issues): + if filtered_issues and known_issues.isdisjoint(filtered_issues): return True return False diff --git a/backend/kernelCI_app/views/treeCompareView.py b/backend/kernelCI_app/views/treeCompareView.py new file mode 100644 index 000000000..533695029 --- /dev/null +++ b/backend/kernelCI_app/views/treeCompareView.py @@ -0,0 +1,123 @@ +from http import HTTPStatus + +from django.http import HttpRequest +from drf_spectacular.utils import extend_schema +from pydantic import ValidationError +from rest_framework.response import Response +from rest_framework.views import APIView + +from kernelCI_app.constants.general import DEFAULT_ORIGIN +from kernelCI_app.helpers.errorHandling import create_api_error_response +from kernelCI_app.helpers.treeCompare import ( + _HashAccumulator, + build_compare_response, + process_build_rows, + process_rollup_rows, +) +from kernelCI_app.queries.tree import ( + get_tree_compare_builds, + get_tree_compare_rollup, +) +from kernelCI_app.typeModels.commonOpenApiParameters import ( + GIT_BRANCH_PATH_PARAM, + TREE_NAME_PATH_PARAM, +) +from kernelCI_app.typeModels.treeCompare import ( + TreeCompareQueryParameters, + TreeCompareResponse, +) + + +class TreeCompareView(APIView): + def _merge_accumulators( + self, + *, + target: dict[str, _HashAccumulator], + builds_data: dict[str, _HashAccumulator], + boots_tests_data: dict[str, _HashAccumulator], + ) -> None: + for commit_hash, build_acc in builds_data.items(): + target_acc = target.setdefault(commit_hash, _HashAccumulator()) + target_acc.builds = build_acc.builds + target_acc.build_groups = build_acc.build_groups + + for commit_hash, entity_acc in boots_tests_data.items(): + target_acc = target.setdefault(commit_hash, _HashAccumulator()) + target_acc.boots = entity_acc.boots + target_acc.tests = entity_acc.tests + target_acc.boot_groups = entity_acc.boot_groups + target_acc.test_groups = entity_acc.test_groups + + @extend_schema( + parameters=[ + TREE_NAME_PATH_PARAM, + GIT_BRANCH_PATH_PARAM, + TreeCompareQueryParameters, + ], + responses=TreeCompareResponse, + ) + def get( + self, + request: HttpRequest, + tree_name: str, + git_branch: str, + ) -> Response: + try: + query_params = TreeCompareQueryParameters( + hash_a=request.GET.get("hash_a", ""), + hash_b=request.GET.get("hash_b", ""), + origin=request.GET.get("origin", DEFAULT_ORIGIN), + ) + except ValidationError as error: + return Response(data=error.json(), status=HTTPStatus.BAD_REQUEST) + + if not query_params.hash_a or not query_params.hash_b: + return create_api_error_response( + status_code=HTTPStatus.BAD_REQUEST, + error_message="hash_a and hash_b are required", + ) + + commit_hashes = [query_params.hash_a, query_params.hash_b] + query_kwargs = { + "commit_hashes": commit_hashes, + "origin_param": query_params.origin, + "git_branch_param": git_branch, + "tree_name": tree_name, + } + + build_rows = get_tree_compare_builds(**query_kwargs) + builds_data = process_build_rows( + rows=build_rows, + commit_hashes=commit_hashes, + ) + boots_tests_data = process_rollup_rows( + rows=get_tree_compare_rollup(**query_kwargs), + commit_hashes=commit_hashes, + ) + + accumulators = { + commit_hash: _HashAccumulator() for commit_hash in commit_hashes + } + self._merge_accumulators( + target=accumulators, + builds_data=builds_data, + boots_tests_data=boots_tests_data, + ) + + git_url = "" + if build_rows: + git_url = build_rows[0].get("git_repository_url") or "" + + response = build_compare_response( + hash_a=query_params.hash_a, + hash_b=query_params.hash_b, + tree_name=tree_name, + branch=git_branch, + git_url=git_url, + accumulators=accumulators, + ) + + return Response( + data=response.model_dump(by_alias=True), + status=HTTPStatus.OK, + ) diff --git a/backend/requests/tree-compare-get.sh b/backend/requests/tree-compare-get.sh new file mode 100644 index 000000000..3457b2117 --- /dev/null +++ b/backend/requests/tree-compare-get.sh @@ -0,0 +1,21 @@ +http 'http://localhost:8000/api/tree/linux/master/compare' \ + origin==maestro \ + hash_a==abc1234567890abcdef1234567890abcdef12 \ + hash_b==def5678901234abcdef5678901234abcdef56 + +# HTTP/1.1 200 OK +# { +# "treeName": "linux", +# "branch": "master", +# "gitUrl": "https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git", +# "summary": { +# "builds": { "sideA": { "pass": 0, "fail": 0, "inconclusive": 0 }, ... }, +# "boots": { ... }, +# "tests": { ... } +# }, +# "groups": { +# "builds": [], +# "boots": [], +# "tests": [] +# } +# } From 2855a05bd4ebd27b824a2f89c5937b3dd9f83484 Mon Sep 17 00:00:00 2001 From: Felipe Bergamin Date: Thu, 16 Jul 2026 16:29:23 -0300 Subject: [PATCH 3/8] feat(tree-compare): implement frontend for tree comparison Signed-off-by: Felipe Bergamin --- dashboard/e2e/tree-compare.spec.ts | 63 ++++ dashboard/src/api/treeCompare.ts | 46 +++ dashboard/src/locales/messages/index.ts | 22 ++ .../src/pages/TreeCompare/TreeCompareLink.tsx | 35 +++ .../src/pages/TreeCompare/TreeComparePage.tsx | 280 ++++++++++++++++++ .../components/CompareDeltaTable.tsx | 96 ++++++ .../components/CompareStatusDisplay.tsx | 88 ++++++ .../TreeCompare/components/CompareSummary.tsx | 95 ++++++ .../components/RevisionSelector.tsx | 161 ++++++++++ .../src/pages/TreeDetails/TreeDetails.tsx | 14 +- dashboard/src/routeTree.gen.ts | 56 ++++ .../tree/$treeName/$branch/compare/index.tsx | 9 + .../tree/$treeName/$branch/compare/route.tsx | 13 + dashboard/src/types/tree/TreeCompare.ts | 76 +++++ dashboard/src/types/tree/TreeDetails.tsx | 4 +- 15 files changed, 1056 insertions(+), 2 deletions(-) create mode 100644 dashboard/e2e/tree-compare.spec.ts create mode 100644 dashboard/src/api/treeCompare.ts create mode 100644 dashboard/src/pages/TreeCompare/TreeCompareLink.tsx create mode 100644 dashboard/src/pages/TreeCompare/TreeComparePage.tsx create mode 100644 dashboard/src/pages/TreeCompare/components/CompareDeltaTable.tsx create mode 100644 dashboard/src/pages/TreeCompare/components/CompareStatusDisplay.tsx create mode 100644 dashboard/src/pages/TreeCompare/components/CompareSummary.tsx create mode 100644 dashboard/src/pages/TreeCompare/components/RevisionSelector.tsx create mode 100644 dashboard/src/routes/_main/tree/$treeName/$branch/compare/index.tsx create mode 100644 dashboard/src/routes/_main/tree/$treeName/$branch/compare/route.tsx create mode 100644 dashboard/src/types/tree/TreeCompare.ts diff --git a/dashboard/e2e/tree-compare.spec.ts b/dashboard/e2e/tree-compare.spec.ts new file mode 100644 index 000000000..6f8e804f4 --- /dev/null +++ b/dashboard/e2e/tree-compare.spec.ts @@ -0,0 +1,63 @@ +import { expect, test } from '@playwright/test'; + +const FULL_HASH_LENGTH = 40; +const HASH_A = 'a'.repeat(FULL_HASH_LENGTH); +const HASH_B = 'b'.repeat(FULL_HASH_LENGTH); + +test('loads revisions and comparison data from the API', async ({ page }) => { + await page.route('**/api/tree/linux/master/commits?**', route => + route.fulfill({ + json: [ + { + git_commit_hash: HASH_A, + last_checkout: '2026-07-14T10:00:00Z', + }, + { + git_commit_hash: HASH_B, + last_checkout: '2026-07-13T10:00:00Z', + }, + ], + }), + ); + + await page.route('**/api/tree/linux/master/compare?**', route => + route.fulfill({ + json: { + treeName: 'linux', + branch: 'master', + gitUrl: 'https://git.kernel.org/linux.git', + summary: { + builds: { + sideA: { pass: 42, fail: 1, inconclusive: 0 }, + sideB: { pass: 40, fail: 3, inconclusive: 0 }, + delta: { pass: -2, fail: 2 }, + }, + boots: { + sideA: { pass: 20, fail: 0, inconclusive: 1 }, + sideB: { pass: 18, fail: 2, inconclusive: 1 }, + delta: { pass: -2, fail: 2 }, + }, + tests: { + sideA: { pass: 100, fail: 5, inconclusive: 2 }, + sideB: { pass: 95, fail: 10, inconclusive: 2 }, + delta: { pass: -5, fail: 5 }, + }, + }, + groups: { + builds: [], + boots: [], + tests: [], + }, + }, + }), + ); + + await page.goto( + `/tree/linux/master/compare?hashA=${HASH_A}&hashB=${HASH_B}&origin=maestro`, + ); + + await expect(page.getByText('Tree summary')).toBeVisible(); + await expect(page.getByText('42', { exact: true }).first()).toBeVisible(); + await expect(page.getByText('100', { exact: true }).first()).toBeVisible(); + await expect(page.getByText(/data is mocked/i)).toHaveCount(0); +}); diff --git a/dashboard/src/api/treeCompare.ts b/dashboard/src/api/treeCompare.ts new file mode 100644 index 000000000..ddc8a591c --- /dev/null +++ b/dashboard/src/api/treeCompare.ts @@ -0,0 +1,46 @@ +import type { UseQueryResult } from '@tanstack/react-query'; +import { useQuery } from '@tanstack/react-query'; + +import type { TreeCompareData } from '@/types/tree/TreeCompare'; + +import { RequestData } from './commonRequest'; + +const fetchTreeCompare = async ({ + treeName, + branch, + hashA, + hashB, + origin, +}: { + treeName: string; + branch: string; + hashA: string; + hashB: string; + origin: string; +}): Promise => + RequestData.get(`/api/tree/${treeName}/${branch}/compare`, { + params: { + hash_a: hashA, + hash_b: hashB, + origin, + }, + }); + +export const useTreeCompare = ({ + treeName, + branch, + hashA, + hashB, + origin, +}: { + treeName: string; + branch: string; + hashA: string; + hashB: string; + origin: string; +}): UseQueryResult => + useQuery({ + queryKey: ['treeCompare', treeName, branch, hashA, hashB, origin], + queryFn: () => fetchTreeCompare({ treeName, branch, hashA, hashB, origin }), + enabled: !!hashA && !!hashB, + }); diff --git a/dashboard/src/locales/messages/index.ts b/dashboard/src/locales/messages/index.ts index e1e111107..6ec841520 100644 --- a/dashboard/src/locales/messages/index.ts +++ b/dashboard/src/locales/messages/index.ts @@ -373,10 +373,32 @@ export const messages = { 'title.hardwareDetails': 'Hardware: {hardwareName}', 'title.issueDetails': 'Issue: {issueName}', 'title.testDetails': 'Test: {testName}', + 'title.treeCompare': 'Compare: {treeName}', 'title.treeDetails': 'Tree: {treeName}', 'tree.details': 'Trees Details', 'tree.path': 'Trees', 'tree.searchPlaceholder': 'Search by tree, branch or tag with a regex', + 'treeCompare.backToDetails': 'Back to tree details', + 'treeCompare.breadcrumb': 'Compare', + 'treeCompare.breakdownTitle': 'Grouped breakdown', + 'treeCompare.changed': 'Changed', + 'treeCompare.delta': 'Delta', + 'treeCompare.deltaFail': 'Δ fail', + 'treeCompare.deltaPass': 'Δ pass', + 'treeCompare.description': + 'Compare pass/fail counts between two revisions on the same tree and branch.', + 'treeCompare.group.boots': 'Platform', + 'treeCompare.group.builds': 'Config', + 'treeCompare.group.tests': 'Path', + 'treeCompare.openCompare': 'Compare revisions', + 'treeCompare.selectRevision': 'Select a revision', + 'treeCompare.sideA': 'Side A', + 'treeCompare.sideB': 'Side B', + 'treeCompare.suggestion.branchHead': 'Branch head', + 'treeCompare.suggestion.previous': 'Previous commit', + 'treeCompare.suggestion.swap': 'Swap sides', + 'treeCompare.suggestions': 'Suggestions', + 'treeCompare.summaryTitle': 'Tree summary', 'treeDetails.bootsHistory': 'Boots History', 'treeDetails.branch': 'Branch', 'treeDetails.buildsHistory': 'Builds History', diff --git a/dashboard/src/pages/TreeCompare/TreeCompareLink.tsx b/dashboard/src/pages/TreeCompare/TreeCompareLink.tsx new file mode 100644 index 000000000..48806f30b --- /dev/null +++ b/dashboard/src/pages/TreeCompare/TreeCompareLink.tsx @@ -0,0 +1,35 @@ +import { Link } from '@tanstack/react-router'; +import type { JSX } from 'react'; + +import { GitCompareArrows } from 'lucide-react'; +import { FormattedMessage } from 'react-intl'; + +import { Button } from '@/components/ui/button'; + +interface TreeCompareLinkProps { + treeName: string; + branch: string; + hash: string; + origin: string; +} + +export function TreeCompareLink({ + treeName, + branch, + hash, + origin, +}: TreeCompareLinkProps): JSX.Element { + return ( + + ); +} diff --git a/dashboard/src/pages/TreeCompare/TreeComparePage.tsx b/dashboard/src/pages/TreeCompare/TreeComparePage.tsx new file mode 100644 index 000000000..5728a0184 --- /dev/null +++ b/dashboard/src/pages/TreeCompare/TreeComparePage.tsx @@ -0,0 +1,280 @@ +import { useCallback, useEffect, useMemo, type JSX } from 'react'; + +import { + Link, + useNavigate, + useParams, + useSearch, +} from '@tanstack/react-router'; +import { FormattedMessage, useIntl } from 'react-intl'; + +import { + Breadcrumb, + BreadcrumbItem, + BreadcrumbLink, + BreadcrumbList, + BreadcrumbPage, + BreadcrumbSeparator, +} from '@/components/Breadcrumb/Breadcrumb'; + +import PageWithTitle from '@/components/PageWithTitle'; +import QuerySwitcher from '@/components/QuerySwitcher/QuerySwitcher'; +import Tabs from '@/components/Tabs/Tabs'; +import type { ITabItem } from '@/components/Tabs/Tabs'; + +import { useCommits } from '@/api/commitHistory'; +import { useTreeCompare } from '@/api/treeCompare'; + +import { + compareNavigateFrom, + compareRouteName, +} from '@/types/tree/TreeCompare'; +import type { PossibleTabs } from '@/types/tree/TreeDetails'; + +import { CompareDeltaTable } from './components/CompareDeltaTable'; +import { CompareSummary } from './components/CompareSummary'; +import { RevisionSelectorBar } from './components/RevisionSelector'; + +const SHORT_HASH_LENGTH = 7; + +const TreeComparePage = (): JSX.Element => { + const { formatMessage } = useIntl(); + const { treeName, branch } = useParams({ from: compareRouteName }); + const { hashA, hashB, origin, currentPageTab } = useSearch({ + from: compareRouteName, + }); + const navigate = useNavigate({ from: compareNavigateFrom }); + + const commitsQuery = useCommits({ + origin, + gitUrl: '', + gitBranch: branch, + treeName, + }); + + const revisions = useMemo( + () => + (commitsQuery.data ?? []).map(commit => ({ + hash: commit.git_commit_hash, + shortHash: commit.git_commit_hash.slice(0, SHORT_HASH_LENGTH), + commitName: commit.git_commit_name ?? '', + date: commit.last_checkout ?? commit.earliest_checkout ?? '', + })), + [commitsQuery.data], + ); + + const resolvedHashA = hashA || revisions[0]?.hash || ''; + const resolvedHashB = + hashB || + revisions.find(revision => revision.hash !== resolvedHashA)?.hash || + ''; + + const compareQuery = useTreeCompare({ + treeName, + branch, + hashA: resolvedHashA, + hashB: resolvedHashB, + origin, + }); + + const updateSearch = useCallback( + (updates: { + hashA?: string; + hashB?: string; + currentPageTab?: PossibleTabs; + }) => { + navigate({ + search: previous => ({ + ...previous, + hashA: updates.hashA ?? previous.hashA, + hashB: updates.hashB ?? previous.hashB, + currentPageTab: + updates.currentPageTab ?? + previous.currentPageTab ?? + 'global.builds', + }), + params: { treeName, branch }, + }); + }, + [navigate, treeName, branch], + ); + + useEffect(() => { + if ((!hashA || !hashB) && resolvedHashA && resolvedHashB) { + updateSearch({ hashA: resolvedHashA, hashB: resolvedHashB }); + } + }, [hashA, hashB, resolvedHashA, resolvedHashB, updateSearch]); + + const handleSuggestion = useCallback( + (suggestion: 'previous' | 'branchHead' | 'swap') => { + if (suggestion === 'swap') { + updateSearch({ hashA: resolvedHashB, hashB: resolvedHashA }); + return; + } + + if (suggestion === 'previous') { + const currentIndex = revisions.findIndex(r => r.hash === resolvedHashB); + const previousIndex = Math.min( + revisions.length - 1, + Math.max(currentIndex, 0) + 1, + ); + updateSearch({ + hashA: resolvedHashB, + hashB: revisions[previousIndex]?.hash ?? resolvedHashA, + }); + return; + } + + updateSearch({ + hashA: resolvedHashB, + hashB: revisions[0]?.hash ?? resolvedHashA, + }); + }, + [revisions, resolvedHashA, resolvedHashB, updateSearch], + ); + + const tabs: ITabItem[] = useMemo( + () => [ + { + name: 'global.builds', + content: ( + + ), + }, + { + name: 'global.boots', + content: ( + + ), + }, + { + name: 'global.tests', + content: ( + + ), + }, + ], + [compareQuery.data?.groups], + ); + + const pageTitle = formatMessage( + { id: 'title.treeCompare' }, + { treeName: `${treeName}/${branch}` }, + ); + + return ( + +
+ + + + s}> + + + + + + s} + > + + + + + + + + + + + + +
+

{pageTitle}

+

+ +

+
+ +
+
+
+ {treeName} + · + {branch} +
+ s} + > + + +
+ + updateSearch({ hashA: value })} + onHashBChange={value => updateSearch({ hashB: value })} + onSuggestion={handleSuggestion} + /> + +
+ + + {compareQuery.data && ( + <> + + +
+

+ +

+ + updateSearch({ currentPageTab: value as PossibleTabs }) + } + /> +
+ + )} + +
+ + ); +}; + +export default TreeComparePage; diff --git a/dashboard/src/pages/TreeCompare/components/CompareDeltaTable.tsx b/dashboard/src/pages/TreeCompare/components/CompareDeltaTable.tsx new file mode 100644 index 000000000..b499699df --- /dev/null +++ b/dashboard/src/pages/TreeCompare/components/CompareDeltaTable.tsx @@ -0,0 +1,96 @@ +import type { JSX } from 'react'; + +import { FormattedMessage } from 'react-intl'; + +import { + Table, + TableBody, + TableCell, + TableHead, + TableHeader, + TableRow, +} from '@/components/ui/table'; + +import type { CompareGroupRow } from '@/types/tree/TreeCompare'; + +import { cn } from '@/lib/utils'; + +import { DeltaPair, StatusCountsDisplay } from './CompareStatusDisplay'; + +interface CompareDeltaTableProps { + rows: CompareGroupRow[]; + groupColumnLabelId: 'treeCompare.group.builds' | 'treeCompare.group.boots' | 'treeCompare.group.tests'; +} + +export function CompareDeltaTable({ + rows, + groupColumnLabelId, +}: CompareDeltaTableProps): JSX.Element { + const sortedRows = [...rows].sort((a, b) => { + const aImpact = Math.abs(a.delta.pass) + Math.abs(a.delta.fail); + const bImpact = Math.abs(b.delta.pass) + Math.abs(b.delta.fail); + if (aImpact !== bImpact) { + return bImpact - aImpact; + } + return a.label.localeCompare(b.label); + }); + + return ( +
+ + + + + + + + + + + + + + + + + + + + {sortedRows.map(row => { + const hasChange = row.delta.pass !== 0 || row.delta.fail !== 0; + + return ( + + + {row.label} + + +
+ +
+ + → + +
+ +
+ + +
+ +
+ + + ); + })} + +
+
+ ); +} diff --git a/dashboard/src/pages/TreeCompare/components/CompareStatusDisplay.tsx b/dashboard/src/pages/TreeCompare/components/CompareStatusDisplay.tsx new file mode 100644 index 000000000..6ac91be1e --- /dev/null +++ b/dashboard/src/pages/TreeCompare/components/CompareStatusDisplay.tsx @@ -0,0 +1,88 @@ +import type { JSX } from 'react'; + +import { FormattedMessage } from 'react-intl'; + +import { GroupedTestStatus } from '@/components/Status/Status'; + +import type { CompareStatusCounts } from '@/types/tree/TreeCompare'; + +import { cn } from '@/lib/utils'; + +function formatSigned(n: number): string { + if (n === 0) { + return '0'; + } + return n > 0 ? `+${n.toLocaleString()}` : n.toLocaleString(); +} + +export function StatusCountsDisplay({ + counts, + hideInconclusive = false, +}: { + counts: CompareStatusCounts; + hideInconclusive?: boolean; +}): JSX.Element { + return ( + + ); +} + +export function DeltaPair({ + passDelta, + failDelta, + className, +}: { + passDelta: number; + failDelta: number; + className?: string; +}): JSX.Element { + const passColor = + passDelta === 0 + ? 'text-dim-gray' + : passDelta > 0 + ? 'text-dark-green' + : 'text-red'; + const failColor = + failDelta === 0 + ? 'text-dim-gray' + : failDelta > 0 + ? 'text-red' + : 'text-dark-green'; + + return ( +
+ + :{' '} + {formatSigned(passDelta)} + + + :{' '} + {formatSigned(failDelta)} + +
+ ); +} + +export function CountsWithLabel({ + label, + counts, +}: { + label: string; + counts: CompareStatusCounts; +}): JSX.Element { + return ( +
+ + {label} + + +
+ ); +} diff --git a/dashboard/src/pages/TreeCompare/components/CompareSummary.tsx b/dashboard/src/pages/TreeCompare/components/CompareSummary.tsx new file mode 100644 index 000000000..1614a599c --- /dev/null +++ b/dashboard/src/pages/TreeCompare/components/CompareSummary.tsx @@ -0,0 +1,95 @@ +import type { JSX } from 'react'; + +import { FormattedMessage } from 'react-intl'; + +import type { CompareEntitySummary } from '@/types/tree/TreeCompare'; + +import { + CountsWithLabel, + DeltaPair, + StatusCountsDisplay, +} from './CompareStatusDisplay'; + +interface CompareSummaryCardProps { + titleId: 'global.builds' | 'global.boots' | 'global.tests'; + summary: CompareEntitySummary; +} + +function CompareSummaryCard({ + titleId, + summary, +}: CompareSummaryCardProps): JSX.Element { + const hasChange = summary.delta.pass !== 0 || summary.delta.fail !== 0; + + return ( +
+

+ +

+ +
+ + +
+ + + + +
+
+ +
+
+ + : + + + → + + : + + +
+ {hasChange && ( + + + + )} +
+
+ ); +} + +interface CompareSummaryProps { + builds: CompareEntitySummary; + boots: CompareEntitySummary; + tests: CompareEntitySummary; +} + +export function CompareSummary({ + builds, + boots, + tests, +}: CompareSummaryProps): JSX.Element { + return ( +
+

+ +

+
+ + + +
+
+ ); +} diff --git a/dashboard/src/pages/TreeCompare/components/RevisionSelector.tsx b/dashboard/src/pages/TreeCompare/components/RevisionSelector.tsx new file mode 100644 index 000000000..b5d938719 --- /dev/null +++ b/dashboard/src/pages/TreeCompare/components/RevisionSelector.tsx @@ -0,0 +1,161 @@ +import type { JSX } from 'react'; + +import { ArrowLeftRight } from 'lucide-react'; +import { FormattedMessage } from 'react-intl'; + +import { Button } from '@/components/ui/button'; +import { + Select, + SelectContent, + SelectItem, + SelectTrigger, + SelectValue, +} from '@/components/ui/select'; + +import type { CompareRevision } from '@/types/tree/TreeCompare'; + +import { cn } from '@/lib/utils'; + +type RevisionSide = 'A' | 'B'; + +function RevisionCard({ + side, + selectedHash, + revisions, + onSelect, +}: { + side: RevisionSide; + selectedHash: string; + revisions: CompareRevision[]; + onSelect: (hash: string) => void; +}): JSX.Element { + const selected = revisions.find(r => r.hash === selectedHash); + + return ( +
+
+ + {side} + + + + +
+ + + + {selected && ( +
+ {selected.commitName && ( +

+ {selected.commitName} +

+ )} +

{selected.shortHash}

+

{selected.date}

+
+ )} +
+ ); +} + +interface RevisionSelectorBarProps { + hashA: string; + hashB: string; + revisions: CompareRevision[]; + onHashAChange: (hash: string) => void; + onHashBChange: (hash: string) => void; + onSuggestion: (suggestion: 'previous' | 'branchHead' | 'swap') => void; +} + +export function RevisionSelectorBar({ + hashA, + hashB, + revisions, + onHashAChange, + onHashBChange, + onSuggestion, +}: RevisionSelectorBarProps): JSX.Element { + return ( +
+
+ + +
+
+ +
+
+ + +
+ +
+ + : + + + + +
+
+ ); +} diff --git a/dashboard/src/pages/TreeDetails/TreeDetails.tsx b/dashboard/src/pages/TreeDetails/TreeDetails.tsx index 2374dbe4b..be4020be2 100644 --- a/dashboard/src/pages/TreeDetails/TreeDetails.tsx +++ b/dashboard/src/pages/TreeDetails/TreeDetails.tsx @@ -69,6 +69,8 @@ import { isEmptyObject } from '@/utils/utils'; import { sanitizeTreeinfo } from '@/utils/treeDetails'; +import { TreeCompareLink } from '@/pages/TreeCompare/TreeCompareLink'; + import TreeDetailsFilter from './TreeDetailsFilter'; import TreeDetailsTab from './Tabs/TreeDetailsTab'; @@ -411,7 +413,7 @@ const TreeDetails = ({ -
+
+ {sanitizedTreeInfo.treeName && sanitizedTreeInfo.gitBranch && ( +
+ +
+ )}
MainalternativesBBuildIdRouteRoute, } as any) +const MainTreeTreeNameBranchCompareRouteRoute = + MainTreeTreeNameBranchCompareRouteRouteImport.update({ + id: '/$treeName/$branch/compare', + path: '/$treeName/$branch/compare', + getParentRoute: () => MainTreeRouteRoute, + } as any) const MainTreeTreeNameBranchHashRouteRoute = MainTreeTreeNameBranchHashRouteRouteImport.update({ id: '/$treeName/$branch/$hash', path: '/$treeName/$branch/$hash', getParentRoute: () => MainTreeRouteRoute, } as any) +const MainTreeTreeNameBranchCompareIndexRoute = + MainTreeTreeNameBranchCompareIndexRouteImport.update({ + id: '/', + path: '/', + getParentRoute: () => MainTreeTreeNameBranchCompareRouteRoute, + } as any) const MainTreeTreeNameBranchHashIndexRoute = MainTreeTreeNameBranchHashIndexRouteImport.update({ id: '/', @@ -324,6 +338,7 @@ export interface FileRoutesByFullPath { '/test/$testId/': typeof MainTestTestIdIndexRoute '/tree/$treeId/': typeof MainTreeTreeIdIndexRoute '/tree/$treeName/$branch/$hash': typeof MainTreeTreeNameBranchHashRouteRouteWithChildren + '/tree/$treeName/$branch/compare': typeof MainTreeTreeNameBranchCompareRouteRouteWithChildren '/b/$buildId/': typeof MainalternativesBBuildIdIndexRoute '/i/$issueId/': typeof MainalternativesIIssueIdIndexRoute '/t/$testId/': typeof MainalternativesTTestIdIndexRoute @@ -340,6 +355,7 @@ export interface FileRoutesByFullPath { '/tree/$treeId/build/$buildId/': typeof MainTreeTreeIdBuildBuildIdIndexRoute '/tree/$treeId/test/$testId/': typeof MainTreeTreeIdTestTestIdIndexRoute '/tree/$treeName/$branch/$hash/': typeof MainTreeTreeNameBranchHashIndexRoute + '/tree/$treeName/$branch/compare/': typeof MainTreeTreeNameBranchCompareIndexRoute '/c/$treeName/$branch/$hash/': typeof MainalternativesCTreeNameBranchHashIndexRoute } export interface FileRoutesByTo { @@ -372,6 +388,7 @@ export interface FileRoutesByTo { '/tree/$treeId/build/$buildId': typeof MainTreeTreeIdBuildBuildIdIndexRoute '/tree/$treeId/test/$testId': typeof MainTreeTreeIdTestTestIdIndexRoute '/tree/$treeName/$branch/$hash': typeof MainTreeTreeNameBranchHashIndexRoute + '/tree/$treeName/$branch/compare': typeof MainTreeTreeNameBranchCompareIndexRoute '/c/$treeName/$branch/$hash': typeof MainalternativesCTreeNameBranchHashIndexRoute } export interface FileRoutesById { @@ -403,6 +420,7 @@ export interface FileRoutesById { '/_main/test/$testId/': typeof MainTestTestIdIndexRoute '/_main/tree/$treeId/': typeof MainTreeTreeIdIndexRoute '/_main/tree/$treeName/$branch/$hash': typeof MainTreeTreeNameBranchHashRouteRouteWithChildren + '/_main/tree/$treeName/$branch/compare': typeof MainTreeTreeNameBranchCompareRouteRouteWithChildren '/_main/(alternatives)/b/$buildId/': typeof MainalternativesBBuildIdIndexRoute '/_main/(alternatives)/i/$issueId/': typeof MainalternativesIIssueIdIndexRoute '/_main/(alternatives)/t/$testId/': typeof MainalternativesTTestIdIndexRoute @@ -419,6 +437,7 @@ export interface FileRoutesById { '/_main/tree/$treeId/build/$buildId/': typeof MainTreeTreeIdBuildBuildIdIndexRoute '/_main/tree/$treeId/test/$testId/': typeof MainTreeTreeIdTestTestIdIndexRoute '/_main/tree/$treeName/$branch/$hash/': typeof MainTreeTreeNameBranchHashIndexRoute + '/_main/tree/$treeName/$branch/compare/': typeof MainTreeTreeNameBranchCompareIndexRoute '/_main/(alternatives)/c/$treeName/$branch/$hash/': typeof MainalternativesCTreeNameBranchHashIndexRoute } export interface FileRouteTypes { @@ -450,6 +469,7 @@ export interface FileRouteTypes { | '/test/$testId/' | '/tree/$treeId/' | '/tree/$treeName/$branch/$hash' + | '/tree/$treeName/$branch/compare' | '/b/$buildId/' | '/i/$issueId/' | '/t/$testId/' @@ -466,6 +486,7 @@ export interface FileRouteTypes { | '/tree/$treeId/build/$buildId/' | '/tree/$treeId/test/$testId/' | '/tree/$treeName/$branch/$hash/' + | '/tree/$treeName/$branch/compare/' | '/c/$treeName/$branch/$hash/' fileRoutesByTo: FileRoutesByTo to: @@ -498,6 +519,7 @@ export interface FileRouteTypes { | '/tree/$treeId/build/$buildId' | '/tree/$treeId/test/$testId' | '/tree/$treeName/$branch/$hash' + | '/tree/$treeName/$branch/compare' | '/c/$treeName/$branch/$hash' id: | '__root__' @@ -528,6 +550,7 @@ export interface FileRouteTypes { | '/_main/test/$testId/' | '/_main/tree/$treeId/' | '/_main/tree/$treeName/$branch/$hash' + | '/_main/tree/$treeName/$branch/compare' | '/_main/(alternatives)/b/$buildId/' | '/_main/(alternatives)/i/$issueId/' | '/_main/(alternatives)/t/$testId/' @@ -544,6 +567,7 @@ export interface FileRouteTypes { | '/_main/tree/$treeId/build/$buildId/' | '/_main/tree/$treeId/test/$testId/' | '/_main/tree/$treeName/$branch/$hash/' + | '/_main/tree/$treeName/$branch/compare/' | '/_main/(alternatives)/c/$treeName/$branch/$hash/' fileRoutesById: FileRoutesById } @@ -793,6 +817,13 @@ declare module '@tanstack/react-router' { preLoaderRoute: typeof MainalternativesBBuildIdIndexRouteImport parentRoute: typeof MainalternativesBBuildIdRouteRoute } + '/_main/tree/$treeName/$branch/compare': { + id: '/_main/tree/$treeName/$branch/compare' + path: '/$treeName/$branch/compare' + fullPath: '/tree/$treeName/$branch/compare' + preLoaderRoute: typeof MainTreeTreeNameBranchCompareRouteRouteImport + parentRoute: typeof MainTreeRouteRoute + } '/_main/tree/$treeName/$branch/$hash': { id: '/_main/tree/$treeName/$branch/$hash' path: '/$treeName/$branch/$hash' @@ -800,6 +831,13 @@ declare module '@tanstack/react-router' { preLoaderRoute: typeof MainTreeTreeNameBranchHashRouteRouteImport parentRoute: typeof MainTreeRouteRoute } + '/_main/tree/$treeName/$branch/compare/': { + id: '/_main/tree/$treeName/$branch/compare/' + path: '/' + fullPath: '/tree/$treeName/$branch/compare/' + preLoaderRoute: typeof MainTreeTreeNameBranchCompareIndexRouteImport + parentRoute: typeof MainTreeTreeNameBranchCompareRouteRoute + } '/_main/tree/$treeName/$branch/$hash/': { id: '/_main/tree/$treeName/$branch/$hash/' path: '/' @@ -962,10 +1000,26 @@ const MainTreeTreeNameBranchHashRouteRouteWithChildren = MainTreeTreeNameBranchHashRouteRouteChildren, ) +interface MainTreeTreeNameBranchCompareRouteRouteChildren { + MainTreeTreeNameBranchCompareIndexRoute: typeof MainTreeTreeNameBranchCompareIndexRoute +} + +const MainTreeTreeNameBranchCompareRouteRouteChildren: MainTreeTreeNameBranchCompareRouteRouteChildren = + { + MainTreeTreeNameBranchCompareIndexRoute: + MainTreeTreeNameBranchCompareIndexRoute, + } + +const MainTreeTreeNameBranchCompareRouteRouteWithChildren = + MainTreeTreeNameBranchCompareRouteRoute._addFileChildren( + MainTreeTreeNameBranchCompareRouteRouteChildren, + ) + interface MainTreeRouteRouteChildren { MainTreeTreeIdRouteRoute: typeof MainTreeTreeIdRouteRouteWithChildren MainTreeIndexRoute: typeof MainTreeIndexRoute MainTreeTreeNameBranchHashRouteRoute: typeof MainTreeTreeNameBranchHashRouteRouteWithChildren + MainTreeTreeNameBranchCompareRouteRoute: typeof MainTreeTreeNameBranchCompareRouteRouteWithChildren MainTreeTreeNameBranchIndexRoute: typeof MainTreeTreeNameBranchIndexRoute } @@ -974,6 +1028,8 @@ const MainTreeRouteRouteChildren: MainTreeRouteRouteChildren = { MainTreeIndexRoute: MainTreeIndexRoute, MainTreeTreeNameBranchHashRouteRoute: MainTreeTreeNameBranchHashRouteRouteWithChildren, + MainTreeTreeNameBranchCompareRouteRoute: + MainTreeTreeNameBranchCompareRouteRouteWithChildren, MainTreeTreeNameBranchIndexRoute: MainTreeTreeNameBranchIndexRoute, } diff --git a/dashboard/src/routes/_main/tree/$treeName/$branch/compare/index.tsx b/dashboard/src/routes/_main/tree/$treeName/$branch/compare/index.tsx new file mode 100644 index 000000000..79f114e00 --- /dev/null +++ b/dashboard/src/routes/_main/tree/$treeName/$branch/compare/index.tsx @@ -0,0 +1,9 @@ +import { createFileRoute } from '@tanstack/react-router'; + +import TreeComparePage from '@/pages/TreeCompare/TreeComparePage'; + +export const Route = createFileRoute( + '/_main/tree/$treeName/$branch/compare/', +)({ + component: TreeComparePage, +}); diff --git a/dashboard/src/routes/_main/tree/$treeName/$branch/compare/route.tsx b/dashboard/src/routes/_main/tree/$treeName/$branch/compare/route.tsx new file mode 100644 index 000000000..eb23dbb98 --- /dev/null +++ b/dashboard/src/routes/_main/tree/$treeName/$branch/compare/route.tsx @@ -0,0 +1,13 @@ +import { createFileRoute, stripSearchParams } from '@tanstack/react-router'; + +import { + compareDefaultValues, + compareSearchSchema, +} from '@/types/tree/TreeCompare'; + +export const Route = createFileRoute( + '/_main/tree/$treeName/$branch/compare', +)({ + validateSearch: compareSearchSchema, + search: { middlewares: [stripSearchParams(compareDefaultValues)] }, +}); diff --git a/dashboard/src/types/tree/TreeCompare.ts b/dashboard/src/types/tree/TreeCompare.ts new file mode 100644 index 000000000..7a7692cb1 --- /dev/null +++ b/dashboard/src/types/tree/TreeCompare.ts @@ -0,0 +1,76 @@ +import { z } from 'zod'; + +import { possibleTabs } from '@/types/tree/TreeDetails'; + +export type CompareStatusCounts = { + pass: number; + fail: number; + inconclusive: number; +}; + +export type CompareDelta = { + pass: number; + fail: number; +}; + +export type CompareEntitySummary = { + sideA: CompareStatusCounts; + sideB: CompareStatusCounts; + delta: CompareDelta; +}; + +export type CompareGroupRow = { + id: string; + label: string; + sideA: CompareStatusCounts; + sideB: CompareStatusCounts; + delta: CompareDelta; +}; + +export type CompareRevision = { + hash: string; + shortHash: string; + commitName: string; + date: string; +}; + +export type TreeCompareData = { + treeName: string; + branch: string; + gitUrl: string; + summary: { + builds: CompareEntitySummary; + boots: CompareEntitySummary; + tests: CompareEntitySummary; + }; + groups: { + builds: CompareGroupRow[]; + boots: CompareGroupRow[]; + tests: CompareGroupRow[]; + }; +}; + +export const compareDefaultValues = { + hashA: '', + hashB: '', + origin: 'maestro', + currentPageTab: 'global.builds' as const, +}; + +export const compareSearchSchema = z.object({ + hashA: z.string().catch(''), + hashB: z.string().catch(''), + origin: z + .string() + .default(compareDefaultValues.origin) + .catch(compareDefaultValues.origin), + currentPageTab: z + .enum(possibleTabs) + .default(compareDefaultValues.currentPageTab) + .catch(compareDefaultValues.currentPageTab), +}); + +export type CompareSearch = z.infer; + +export const compareRouteName = '/_main/tree/$treeName/$branch/compare'; +export const compareNavigateFrom = '/tree/$treeName/$branch/compare'; diff --git a/dashboard/src/types/tree/TreeDetails.tsx b/dashboard/src/types/tree/TreeDetails.tsx index b3afc6702..acfef4f22 100644 --- a/dashboard/src/types/tree/TreeDetails.tsx +++ b/dashboard/src/types/tree/TreeDetails.tsx @@ -186,7 +186,9 @@ export type PaginatedCommitHistoryByTree = { export type Commit = { git_commit_hash: string; - earliest_checkout: string; + git_commit_name?: string | null; + earliest_checkout?: string; + last_checkout?: string; }; export type BuildCountsResponse = { From 67cd6e4fe680df2301598f9394dedda461205755 Mon Sep 17 00:00:00 2001 From: Felipe Bergamin Date: Tue, 21 Jul 2026 16:33:00 -0300 Subject: [PATCH 4/8] feat: add AB comparison details screens Signed-off-by: Felipe Bergamin --- dashboard/src/locales/messages/index.ts | 23 ++ .../TreeCompare/TreeCompareFailuresPage.tsx | 255 ++++++++++++++++++ .../src/pages/TreeCompare/TreeComparePage.tsx | 8 + .../components/CompareChangeDisplay.tsx | 82 ++++++ .../components/CompareChangeStats.tsx | 111 ++++++++ .../components/CompareFailuresTables.tsx | 218 +++++++++++++++ .../TreeCompare/components/CompareSummary.tsx | 89 ++++-- .../TreeCompare/mock/compareFailuresMock.ts | 230 ++++++++++++++++ dashboard/src/routeTree.gen.ts | 23 ++ .../$treeName/$branch/compare/failures.tsx | 9 + dashboard/src/types/tree/TreeCompare.ts | 65 +++++ 11 files changed, 1097 insertions(+), 16 deletions(-) create mode 100644 dashboard/src/pages/TreeCompare/TreeCompareFailuresPage.tsx create mode 100644 dashboard/src/pages/TreeCompare/components/CompareChangeDisplay.tsx create mode 100644 dashboard/src/pages/TreeCompare/components/CompareChangeStats.tsx create mode 100644 dashboard/src/pages/TreeCompare/components/CompareFailuresTables.tsx create mode 100644 dashboard/src/pages/TreeCompare/mock/compareFailuresMock.ts create mode 100644 dashboard/src/routes/_main/tree/$treeName/$branch/compare/failures.tsx diff --git a/dashboard/src/locales/messages/index.ts b/dashboard/src/locales/messages/index.ts index 6ec841520..d3fbbe692 100644 --- a/dashboard/src/locales/messages/index.ts +++ b/dashboard/src/locales/messages/index.ts @@ -382,11 +382,33 @@ export const messages = { 'treeCompare.breadcrumb': 'Compare', 'treeCompare.breakdownTitle': 'Grouped breakdown', 'treeCompare.changed': 'Changed', + 'treeCompare.change.fixed': 'Fixed', + 'treeCompare.change.newFailure': 'New failure', + 'treeCompare.change.newFailures': 'New failures', + 'treeCompare.change.newPass': 'New pass', + 'treeCompare.change.newPasses': 'New passes', + 'treeCompare.change.regression': 'Regression', + 'treeCompare.change.regressions': 'Regressions', + 'treeCompare.change.stillFailing': 'Still failing', + 'treeCompare.changeFilter.all': 'All changes', 'treeCompare.delta': 'Delta', 'treeCompare.deltaFail': 'Δ fail', 'treeCompare.deltaPass': 'Δ pass', 'treeCompare.description': 'Compare pass/fail counts between two revisions on the same tree and branch.', + 'treeCompare.drilldownHint': + 'Click “View failures” to inspect individual builds, boots, and tests that changed between Side A and Side B.', + 'treeCompare.failures.backToCompare': '← Back to compare', + 'treeCompare.failures.buildsTitle': 'Build failures', + 'treeCompare.failures.bootsTitle': 'Boot failures', + 'treeCompare.failures.change': 'Change', + 'treeCompare.failures.configArch': 'Config / Arch', + 'treeCompare.failures.description': + 'Individual results that differ between {hashA} (Side A) and {hashB} (Side B).', + 'treeCompare.failures.mockNotice': + 'Sketch: showing mocked failure data. Backend wiring comes later.', + 'treeCompare.failures.pathHardware': 'Path / Hardware', + 'treeCompare.failures.testsTitle': 'Test failures', 'treeCompare.group.boots': 'Platform', 'treeCompare.group.builds': 'Config', 'treeCompare.group.tests': 'Path', @@ -399,6 +421,7 @@ export const messages = { 'treeCompare.suggestion.swap': 'Swap sides', 'treeCompare.suggestions': 'Suggestions', 'treeCompare.summaryTitle': 'Tree summary', + 'treeCompare.viewFailures': 'View failures →', 'treeDetails.bootsHistory': 'Boots History', 'treeDetails.branch': 'Branch', 'treeDetails.buildsHistory': 'Builds History', diff --git a/dashboard/src/pages/TreeCompare/TreeCompareFailuresPage.tsx b/dashboard/src/pages/TreeCompare/TreeCompareFailuresPage.tsx new file mode 100644 index 000000000..563911b78 --- /dev/null +++ b/dashboard/src/pages/TreeCompare/TreeCompareFailuresPage.tsx @@ -0,0 +1,255 @@ +import { useCallback, useMemo, type JSX } from 'react'; + +import { + Link, + useNavigate, + useParams, + useSearch, +} from '@tanstack/react-router'; +import { FormattedMessage, useIntl } from 'react-intl'; + +import { + Breadcrumb, + BreadcrumbItem, + BreadcrumbLink, + BreadcrumbList, + BreadcrumbPage, + BreadcrumbSeparator, +} from '@/components/Breadcrumb/Breadcrumb'; +import PageWithTitle from '@/components/PageWithTitle'; +import Tabs from '@/components/Tabs/Tabs'; +import type { ITabItem } from '@/components/Tabs/Tabs'; +import ColoredCircle from '@/components/ColoredCircle/ColoredCircle'; + +import { + compareFailuresNavigateFrom, + compareFailuresRouteName, + type CompareChangeFilter, + type CompareChangeStats, +} from '@/types/tree/TreeCompare'; +import type { PossibleTabs } from '@/types/tree/TreeDetails'; + +import { COMPARE_FAILURES_MOCK } from './mock/compareFailuresMock'; +import { + CompareChangeFilterBar, + CompareChangeStatsCards, +} from './components/CompareChangeStats'; +import { + CompareBootsFailuresTable, + CompareBuildsFailuresTable, + CompareTestsFailuresTable, +} from './components/CompareFailuresTables'; + +const SHORT_HASH_LENGTH = 7; + +function failCount(stats: CompareChangeStats): number { + return stats.regression + stats.newFailure + stats.stillFailing; +} + +const TreeCompareFailuresPage = (): JSX.Element => { + const { formatMessage } = useIntl(); + const { treeName, branch } = useParams({ from: compareFailuresRouteName }); + const { hashA, hashB, currentPageTab, changeFilter } = useSearch({ + from: compareFailuresRouteName, + }); + const navigate = useNavigate({ from: compareFailuresNavigateFrom }); + + const shortA = hashA.slice(0, SHORT_HASH_LENGTH) || '—'; + const shortB = hashB.slice(0, SHORT_HASH_LENGTH) || '—'; + + const mock = COMPARE_FAILURES_MOCK; + + const updateSearch = useCallback( + (updates: { + currentPageTab?: PossibleTabs; + changeFilter?: CompareChangeFilter; + }) => { + navigate({ + search: previous => ({ + ...previous, + currentPageTab: + updates.currentPageTab ?? + previous.currentPageTab ?? + 'global.builds', + changeFilter: updates.changeFilter ?? previous.changeFilter ?? 'all', + }), + params: { treeName, branch }, + }); + }, + [navigate, treeName, branch], + ); + + const activeStats = + currentPageTab === 'global.boots' + ? mock.stats.boots + : currentPageTab === 'global.tests' + ? mock.stats.tests + : mock.stats.builds; + + const tabs: ITabItem[] = useMemo(() => { + const applyFilter = (rows: T[]): T[] => { + if (changeFilter === 'all') { + return rows; + } + return rows.filter(row => row.change === changeFilter); + }; + + return [ + { + name: 'global.builds', + rightElement: ( + + ), + content: ( + + ), + }, + { + name: 'global.boots', + rightElement: ( + + ), + content: , + }, + { + name: 'global.tests', + rightElement: ( + + ), + content: , + }, + ]; + }, [changeFilter, mock]); + + const entityTitleId = + currentPageTab === 'global.boots' + ? 'treeCompare.failures.bootsTitle' + : currentPageTab === 'global.tests' + ? 'treeCompare.failures.testsTitle' + : 'treeCompare.failures.buildsTitle'; + + const pageTitle = formatMessage({ id: entityTitleId }); + + return ( + +
+ + + + s}> + + + + + + s} + > + + + + + + ({ + hashA: previous.hashA ?? hashA, + hashB: previous.hashB ?? hashB, + origin: previous.origin, + })} + state={s => s} + > + + + + + + + + + + + + +
+
+

{pageTitle}

+

+ +

+
+ ({ + hashA: previous.hashA ?? hashA, + hashB: previous.hashB ?? hashB, + origin: previous.origin, + })} + className="text-blue text-sm font-medium hover:underline" + state={s => s} + > + + +
+ +
+
+ + + A + + {shortA} + + + + B + + {shortB} + +
+ + {treeName} · {branch} + +
+ +
+ +
+ + + + updateSearch({ changeFilter: value })} + /> + + + updateSearch({ currentPageTab: value as PossibleTabs }) + } + /> +
+ + ); +}; + +export default TreeCompareFailuresPage; diff --git a/dashboard/src/pages/TreeCompare/TreeComparePage.tsx b/dashboard/src/pages/TreeCompare/TreeComparePage.tsx index 5728a0184..a6e21392f 100644 --- a/dashboard/src/pages/TreeCompare/TreeComparePage.tsx +++ b/dashboard/src/pages/TreeCompare/TreeComparePage.tsx @@ -254,12 +254,20 @@ const TreeComparePage = (): JSX.Element => { builds={compareQuery.data.summary.builds} boots={compareQuery.data.summary.boots} tests={compareQuery.data.summary.tests} + treeName={treeName} + branch={branch} + hashA={resolvedHashA} + hashB={resolvedHashB} + origin={origin} />

+
+ +
= { + PASS: 'bg-light-green text-dark-green', + FAIL: 'bg-light-red text-red', + INCONCLUSIVE: 'bg-medium-gray text-dim-gray', + '—': 'bg-medium-gray text-dim-gray', +}; + +export function CompareStatusChip({ + status, +}: { + status: CompareItemStatus; +}): JSX.Element { + return ( + + {status} + + ); +} + +const CHANGE_STYLES: Record = { + regression: 'bg-light-red text-red', + fixed: 'bg-light-green text-dark-green', + newFailure: 'bg-orange-100 text-orange-700', + stillFailing: 'bg-medium-gray text-dim-gray', + newPass: 'bg-light-blue text-dark-blue', +}; + +const CHANGE_MESSAGE_IDS: Record< + CompareChangeType, + | 'treeCompare.change.regression' + | 'treeCompare.change.fixed' + | 'treeCompare.change.newFailure' + | 'treeCompare.change.stillFailing' + | 'treeCompare.change.newPass' +> = { + regression: 'treeCompare.change.regression', + fixed: 'treeCompare.change.fixed', + newFailure: 'treeCompare.change.newFailure', + stillFailing: 'treeCompare.change.stillFailing', + newPass: 'treeCompare.change.newPass', +}; + +export function CompareChangeBadge({ + change, +}: { + change: CompareChangeType; +}): JSX.Element { + return ( + + + + ); +} + +export function isFailureHighlight(change: CompareChangeType): boolean { + return ( + change === 'regression' || + change === 'newFailure' || + change === 'stillFailing' + ); +} diff --git a/dashboard/src/pages/TreeCompare/components/CompareChangeStats.tsx b/dashboard/src/pages/TreeCompare/components/CompareChangeStats.tsx new file mode 100644 index 000000000..8ee2d2f87 --- /dev/null +++ b/dashboard/src/pages/TreeCompare/components/CompareChangeStats.tsx @@ -0,0 +1,111 @@ +import type { JSX } from 'react'; + +import { FormattedMessage } from 'react-intl'; + +import type { + CompareChangeFilter, + CompareChangeStats, + CompareChangeType, +} from '@/types/tree/TreeCompare'; +import { compareChangeTypes } from '@/types/tree/TreeCompare'; + +import type { MessagesKey } from '@/locales/messages'; + +import { cn } from '@/lib/utils'; +import { Button } from '@/components/ui/button'; + +const STAT_STYLES: Record = { + regression: 'bg-light-red text-red', + fixed: 'bg-light-green text-dark-green', + newFailure: 'bg-orange-100 text-orange-700', + stillFailing: 'bg-medium-gray text-dim-gray', + newPass: 'bg-light-blue text-dark-blue', +}; + +const STAT_LABELS: Record = { + regression: 'treeCompare.change.regressions', + fixed: 'treeCompare.change.fixed', + newFailure: 'treeCompare.change.newFailures', + stillFailing: 'treeCompare.change.stillFailing', + newPass: 'treeCompare.change.newPasses', +}; + +export function CompareChangeStatsCards({ + stats, +}: { + stats: CompareChangeStats; +}): JSX.Element { + return ( +
+ {compareChangeTypes.map(type => ( +
+

+ +

+
+ + {stats[type]} + +
+
+ ))} +
+ ); +} + +const FILTER_OPTIONS: { + value: CompareChangeFilter; + labelId: MessagesKey; +}[] = [ + { value: 'all', labelId: 'treeCompare.changeFilter.all' }, + { value: 'regression', labelId: 'treeCompare.change.regressions' }, + { value: 'fixed', labelId: 'treeCompare.change.fixed' }, + { value: 'newFailure', labelId: 'treeCompare.change.newFailures' }, + { value: 'stillFailing', labelId: 'treeCompare.change.stillFailing' }, +]; + +export function CompareChangeFilterBar({ + value, + onChange, +}: { + value: CompareChangeFilter; + onChange: (value: CompareChangeFilter) => void; +}): JSX.Element { + return ( +
+ + + +
+ {FILTER_OPTIONS.map(option => { + const selected = value === option.value; + return ( + + ); + })} +
+
+ ); +} diff --git a/dashboard/src/pages/TreeCompare/components/CompareFailuresTables.tsx b/dashboard/src/pages/TreeCompare/components/CompareFailuresTables.tsx new file mode 100644 index 000000000..9a855fa90 --- /dev/null +++ b/dashboard/src/pages/TreeCompare/components/CompareFailuresTables.tsx @@ -0,0 +1,218 @@ +import type { JSX } from 'react'; + +import { MoreHorizontal } from 'lucide-react'; +import { FormattedMessage } from 'react-intl'; + +import type { + CompareBootFailureRow, + CompareBuildFailureRow, + CompareTestFailureRow, +} from '@/types/tree/TreeCompare'; + +import { + Table, + TableBody, + TableCell, + TableHead, + TableHeader, + TableRow, +} from '@/components/ui/table'; + +import { cn } from '@/lib/utils'; + +import { + CompareChangeBadge, + CompareStatusChip, + isFailureHighlight, +} from './CompareChangeDisplay'; + +function DetailsPlaceholder(): JSX.Element { + return ( + + ); +} + +function SideCells({ + sideA, + sideB, +}: { + sideA: CompareBuildFailureRow['sideA']; + sideB: CompareBuildFailureRow['sideB']; +}): JSX.Element { + return ( + <> + +
+ +
+ + → + +
+ +
+ + + ); +} + +export function CompareBuildsFailuresTable({ + rows, +}: { + rows: CompareBuildFailureRow[]; +}): JSX.Element { + return ( +
+ + + + + + + + + + + + + + + + + + + + + + + + + + + {rows.map(row => ( + + +
{row.config}
+
+ {row.arch} · {row.compiler} +
+ + + +
+ +
+ + {row.buildErrors} + {row.date} + + + + + ))} + +
+
+ ); +} + +function PathHardwareTable({ + rows, + pathLabelId, +}: { + rows: Array; + pathLabelId: 'treeCompare.failures.pathHardware'; +}): JSX.Element { + return ( +
+ + + + + + + + + + + + + + + + + + + + + + + + + + + {rows.map(row => ( + + +
{row.path}
+
{row.hardware}
+ + + +
+ +
+ + + {row.duration} + + {row.date} + + + + + ))} + +
+
+ ); +} + +export function CompareBootsFailuresTable({ + rows, +}: { + rows: CompareBootFailureRow[]; +}): JSX.Element { + return ( + + ); +} + +export function CompareTestsFailuresTable({ + rows, +}: { + rows: CompareTestFailureRow[]; +}): JSX.Element { + return ( + + ); +} diff --git a/dashboard/src/pages/TreeCompare/components/CompareSummary.tsx b/dashboard/src/pages/TreeCompare/components/CompareSummary.tsx index 1614a599c..84c403cc1 100644 --- a/dashboard/src/pages/TreeCompare/components/CompareSummary.tsx +++ b/dashboard/src/pages/TreeCompare/components/CompareSummary.tsx @@ -1,8 +1,10 @@ import type { JSX } from 'react'; +import { Link } from '@tanstack/react-router'; import { FormattedMessage } from 'react-intl'; import type { CompareEntitySummary } from '@/types/tree/TreeCompare'; +import type { PossibleTabs } from '@/types/tree/TreeDetails'; import { CountsWithLabel, @@ -13,11 +15,23 @@ import { interface CompareSummaryCardProps { titleId: 'global.builds' | 'global.boots' | 'global.tests'; summary: CompareEntitySummary; + treeName: string; + branch: string; + hashA: string; + hashB: string; + origin?: string; + tab: PossibleTabs; } function CompareSummaryCard({ titleId, summary, + treeName, + branch, + hashA, + hashB, + origin, + tab, }: CompareSummaryCardProps): JSX.Element { const hasChange = summary.delta.pass !== 0 || summary.delta.fail !== 0; @@ -28,14 +42,8 @@ function CompareSummaryCard({
- - + +
@@ -59,11 +67,23 @@ function CompareSummaryCard({
- {hasChange && ( - - - - )} + {hasChange ? ( + s} + > + + + ) : null}
); @@ -73,12 +93,22 @@ interface CompareSummaryProps { builds: CompareEntitySummary; boots: CompareEntitySummary; tests: CompareEntitySummary; + treeName: string; + branch: string; + hashA: string; + hashB: string; + origin?: string; } export function CompareSummary({ builds, boots, tests, + treeName, + branch, + hashA, + hashB, + origin, }: CompareSummaryProps): JSX.Element { return (
@@ -86,9 +116,36 @@ export function CompareSummary({
- - - + + +
); diff --git a/dashboard/src/pages/TreeCompare/mock/compareFailuresMock.ts b/dashboard/src/pages/TreeCompare/mock/compareFailuresMock.ts new file mode 100644 index 000000000..6af514c61 --- /dev/null +++ b/dashboard/src/pages/TreeCompare/mock/compareFailuresMock.ts @@ -0,0 +1,230 @@ +import type { TreeCompareFailuresData } from '@/types/tree/TreeCompare'; + +/** Sketch-only mock for compare failure drill-down. Replace with API later. */ +export const COMPARE_FAILURES_MOCK: TreeCompareFailuresData = { + stats: { + builds: { + regression: 1, + fixed: 0, + newFailure: 1, + stillFailing: 0, + newPass: 14, + }, + boots: { + regression: 0, + fixed: 0, + newFailure: 0, + stillFailing: 0, + newPass: 11, + }, + tests: { + regression: 12, + fixed: 5, + newFailure: 41, + stillFailing: 31, + newPass: 368, + }, + }, + builds: [ + { + id: 'build-1', + config: 'defconfig+allmodconfig', + arch: 'arm64', + compiler: 'clang-17', + sideA: 'PASS', + sideB: 'FAIL', + change: 'regression', + buildErrors: 1, + date: 'Jul 13, 12:19 UTC', + }, + { + id: 'build-2', + config: 'defconfig+CONFIG_RANDOMIZE_BASE=y', + arch: 'x86_64', + compiler: 'gcc-12', + sideA: '—', + sideB: 'FAIL', + change: 'newFailure', + buildErrors: 1, + date: 'Jul 13, 12:20 UTC', + }, + { + id: 'build-3', + config: 'imx_v6_v7_defconfig+allmodconfig', + arch: 'arm', + compiler: 'gcc-12', + sideA: '—', + sideB: 'PASS', + change: 'newPass', + buildErrors: 0, + date: 'Jul 13, 12:18 UTC', + }, + { + id: 'build-4', + config: 'defconfig', + arch: 'arm64', + compiler: 'gcc-12', + sideA: 'PASS', + sideB: 'PASS', + change: 'newPass', + buildErrors: 0, + date: 'Jul 13, 12:17 UTC', + }, + { + id: 'build-5', + config: 'multi_v7_defconfig', + arch: 'arm', + compiler: 'gcc-11', + sideA: 'PASS', + sideB: 'PASS', + change: 'newPass', + buildErrors: 0, + date: 'Jul 13, 12:16 UTC', + }, + { + id: 'build-6', + config: 'x86_64_defconfig+allmodconfig', + arch: 'x86_64', + compiler: 'clang-17', + sideA: '—', + sideB: 'PASS', + change: 'newPass', + buildErrors: 0, + date: 'Jul 13, 12:15 UTC', + }, + ], + boots: [ + { + id: 'boot-1', + path: 'boot.boot_nfs', + hardware: 'rk3399-rock-pi-4b · lab-collabora', + sideA: '—', + sideB: 'PASS', + change: 'newPass', + duration: '38.2s', + date: 'Jul 13, 12:22 UTC', + }, + { + id: 'boot-2', + path: 'boot.boot_smc', + hardware: 'qemu-arm64 · lab-broonie', + sideA: '—', + sideB: 'PASS', + change: 'newPass', + duration: '12.1s', + date: 'Jul 13, 12:22 UTC', + }, + { + id: 'boot-3', + path: 'boot.boot', + hardware: 'bcm2711-rpi-4-b · lab-collabora', + sideA: '—', + sideB: 'PASS', + change: 'newPass', + duration: '45.0s', + date: 'Jul 13, 12:22 UTC', + }, + { + id: 'boot-4', + path: 'boot.boot_nfs', + hardware: 'sun50i-h6-pine-h64 · lab-baylibre', + sideA: '—', + sideB: 'PASS', + change: 'newPass', + duration: '52.4s', + date: 'Jul 13, 12:22 UTC', + }, + { + id: 'boot-5', + path: 'boot.boot', + hardware: 'x86_64 · lab-collabora', + sideA: '—', + sideB: 'PASS', + change: 'newPass', + duration: '28.7s', + date: 'Jul 13, 12:22 UTC', + }, + ], + tests: [ + { + id: 'test-1', + path: 'kselftest.mm.compaction_test', + hardware: 'qemu-arm64 · lab-broonie', + sideA: 'PASS', + sideB: 'FAIL', + change: 'regression', + duration: '8.1s', + date: 'Jul 13, 12:30 UTC', + }, + { + id: 'test-2', + path: 'ltp.syscalls.fanotify14', + hardware: 'x86_64 · lab-collabora', + sideA: 'PASS', + sideB: 'FAIL', + change: 'regression', + duration: '14.2s', + date: 'Jul 13, 12:30 UTC', + }, + { + id: 'test-3', + path: 'kunit.lib.list_test', + hardware: 'qemu-x86_64 · lab-broonie', + sideA: '—', + sideB: 'FAIL', + change: 'newFailure', + duration: '2.4s', + date: 'Jul 13, 12:30 UTC', + }, + { + id: 'test-4', + path: 'kselftest.bpf.verifier_and', + hardware: 'qemu-arm64 · lab-collabora', + sideA: 'FAIL', + sideB: 'PASS', + change: 'fixed', + duration: '22.0s', + date: 'Jul 13, 12:30 UTC', + }, + { + id: 'test-5', + path: 'ltp.fs.ftest06', + hardware: 'rk3399-rock-pi-4b · lab-collabora', + sideA: 'FAIL', + sideB: 'FAIL', + change: 'stillFailing', + duration: '31.5s', + date: 'Jul 13, 12:30 UTC', + }, + { + id: 'test-6', + path: 'kselftest.net.tls', + hardware: 'x86_64 · lab-broonie', + sideA: '—', + sideB: 'PASS', + change: 'newPass', + duration: '5.3s', + date: 'Jul 13, 12:30 UTC', + }, + { + id: 'test-7', + path: 'kunit.time.time64_to_tm_test', + hardware: 'qemu-arm64 · lab-baylibre', + sideA: 'PASS', + sideB: 'FAIL', + change: 'regression', + duration: '1.1s', + date: 'Jul 13, 12:30 UTC', + }, + { + id: 'test-8', + path: 'baseline.dmesg.crit', + hardware: 'bcm2711-rpi-4-b · lab-collabora', + sideA: '—', + sideB: 'FAIL', + change: 'newFailure', + duration: '0.8s', + date: 'Jul 13, 12:30 UTC', + }, + ], +}; diff --git a/dashboard/src/routeTree.gen.ts b/dashboard/src/routeTree.gen.ts index 2352d3a7d..34e589d05 100644 --- a/dashboard/src/routeTree.gen.ts +++ b/dashboard/src/routeTree.gen.ts @@ -54,6 +54,7 @@ import { Route as MainHardwareHardwareIdBuildBuildIdIndexRouteImport } from './r import { Route as MainHardwareHardwareIdBootBootIdIndexRouteImport } from './routes/_main/hardware/$hardwareId/boot/$bootId/index' import { Route as MainCheckoutTreeNameBranchHashIndexRouteImport } from './routes/_main/checkout/$treeName/$branch/$hash/index' import { Route as MainalternativesCTreeNameBranchIndexRouteImport } from './routes/_main/(alternatives)/c/$treeName/$branch/index' +import { Route as MainTreeTreeNameBranchCompareFailuresRouteImport } from './routes/_main/tree/$treeName/$branch/compare/failures' import { Route as MainalternativesCTreeNameBranchHashIndexRouteImport } from './routes/_main/(alternatives)/c/$treeName/$branch/$hash/index' const LogViewerRoute = LogViewerRouteImport.update({ @@ -304,6 +305,12 @@ const MainalternativesCTreeNameBranchIndexRoute = path: '/c/$treeName/$branch/', getParentRoute: () => MainRouteRoute, } as any) +const MainTreeTreeNameBranchCompareFailuresRoute = + MainTreeTreeNameBranchCompareFailuresRouteImport.update({ + id: '/failures', + path: '/failures', + getParentRoute: () => MainTreeTreeNameBranchCompareRouteRoute, + } as any) const MainalternativesCTreeNameBranchHashIndexRoute = MainalternativesCTreeNameBranchHashIndexRouteImport.update({ id: '/(alternatives)/c/$treeName/$branch/$hash/', @@ -347,6 +354,7 @@ export interface FileRoutesByFullPath { '/hardware/$hardwareId/build/': typeof MainHardwareHardwareIdBuildIndexRoute '/hardware/$hardwareId/test/': typeof MainHardwareHardwareIdTestIndexRoute '/tree/$treeName/$branch/': typeof MainTreeTreeNameBranchIndexRoute + '/tree/$treeName/$branch/compare/failures': typeof MainTreeTreeNameBranchCompareFailuresRoute '/c/$treeName/$branch/': typeof MainalternativesCTreeNameBranchIndexRoute '/checkout/$treeName/$branch/$hash/': typeof MainCheckoutTreeNameBranchHashIndexRoute '/hardware/$hardwareId/boot/$bootId/': typeof MainHardwareHardwareIdBootBootIdIndexRoute @@ -380,6 +388,7 @@ export interface FileRoutesByTo { '/hardware/$hardwareId/build': typeof MainHardwareHardwareIdBuildIndexRoute '/hardware/$hardwareId/test': typeof MainHardwareHardwareIdTestIndexRoute '/tree/$treeName/$branch': typeof MainTreeTreeNameBranchIndexRoute + '/tree/$treeName/$branch/compare/failures': typeof MainTreeTreeNameBranchCompareFailuresRoute '/c/$treeName/$branch': typeof MainalternativesCTreeNameBranchIndexRoute '/checkout/$treeName/$branch/$hash': typeof MainCheckoutTreeNameBranchHashIndexRoute '/hardware/$hardwareId/boot/$bootId': typeof MainHardwareHardwareIdBootBootIdIndexRoute @@ -429,6 +438,7 @@ export interface FileRoutesById { '/_main/hardware/$hardwareId/build/': typeof MainHardwareHardwareIdBuildIndexRoute '/_main/hardware/$hardwareId/test/': typeof MainHardwareHardwareIdTestIndexRoute '/_main/tree/$treeName/$branch/': typeof MainTreeTreeNameBranchIndexRoute + '/_main/tree/$treeName/$branch/compare/failures': typeof MainTreeTreeNameBranchCompareFailuresRoute '/_main/(alternatives)/c/$treeName/$branch/': typeof MainalternativesCTreeNameBranchIndexRoute '/_main/checkout/$treeName/$branch/$hash/': typeof MainCheckoutTreeNameBranchHashIndexRoute '/_main/hardware/$hardwareId/boot/$bootId/': typeof MainHardwareHardwareIdBootBootIdIndexRoute @@ -478,6 +488,7 @@ export interface FileRouteTypes { | '/hardware/$hardwareId/build/' | '/hardware/$hardwareId/test/' | '/tree/$treeName/$branch/' + | '/tree/$treeName/$branch/compare/failures' | '/c/$treeName/$branch/' | '/checkout/$treeName/$branch/$hash/' | '/hardware/$hardwareId/boot/$bootId/' @@ -511,6 +522,7 @@ export interface FileRouteTypes { | '/hardware/$hardwareId/build' | '/hardware/$hardwareId/test' | '/tree/$treeName/$branch' + | '/tree/$treeName/$branch/compare/failures' | '/c/$treeName/$branch' | '/checkout/$treeName/$branch/$hash' | '/hardware/$hardwareId/boot/$bootId' @@ -559,6 +571,7 @@ export interface FileRouteTypes { | '/_main/hardware/$hardwareId/build/' | '/_main/hardware/$hardwareId/test/' | '/_main/tree/$treeName/$branch/' + | '/_main/tree/$treeName/$branch/compare/failures' | '/_main/(alternatives)/c/$treeName/$branch/' | '/_main/checkout/$treeName/$branch/$hash/' | '/_main/hardware/$hardwareId/boot/$bootId/' @@ -894,6 +907,13 @@ declare module '@tanstack/react-router' { preLoaderRoute: typeof MainalternativesCTreeNameBranchIndexRouteImport parentRoute: typeof MainRouteRoute } + '/_main/tree/$treeName/$branch/compare/failures': { + id: '/_main/tree/$treeName/$branch/compare/failures' + path: '/failures' + fullPath: '/tree/$treeName/$branch/compare/failures' + preLoaderRoute: typeof MainTreeTreeNameBranchCompareFailuresRouteImport + parentRoute: typeof MainTreeTreeNameBranchCompareRouteRoute + } '/_main/(alternatives)/c/$treeName/$branch/$hash/': { id: '/_main/(alternatives)/c/$treeName/$branch/$hash/' path: '/c/$treeName/$branch/$hash' @@ -1001,11 +1021,14 @@ const MainTreeTreeNameBranchHashRouteRouteWithChildren = ) interface MainTreeTreeNameBranchCompareRouteRouteChildren { + MainTreeTreeNameBranchCompareFailuresRoute: typeof MainTreeTreeNameBranchCompareFailuresRoute MainTreeTreeNameBranchCompareIndexRoute: typeof MainTreeTreeNameBranchCompareIndexRoute } const MainTreeTreeNameBranchCompareRouteRouteChildren: MainTreeTreeNameBranchCompareRouteRouteChildren = { + MainTreeTreeNameBranchCompareFailuresRoute: + MainTreeTreeNameBranchCompareFailuresRoute, MainTreeTreeNameBranchCompareIndexRoute: MainTreeTreeNameBranchCompareIndexRoute, } diff --git a/dashboard/src/routes/_main/tree/$treeName/$branch/compare/failures.tsx b/dashboard/src/routes/_main/tree/$treeName/$branch/compare/failures.tsx new file mode 100644 index 000000000..a0e99aaf1 --- /dev/null +++ b/dashboard/src/routes/_main/tree/$treeName/$branch/compare/failures.tsx @@ -0,0 +1,9 @@ +import { createFileRoute } from '@tanstack/react-router'; + +import TreeCompareFailuresPage from '@/pages/TreeCompare/TreeCompareFailuresPage'; + +export const Route = createFileRoute( + '/_main/tree/$treeName/$branch/compare/failures', +)({ + component: TreeCompareFailuresPage, +}); diff --git a/dashboard/src/types/tree/TreeCompare.ts b/dashboard/src/types/tree/TreeCompare.ts index 7a7692cb1..194c2e8cf 100644 --- a/dashboard/src/types/tree/TreeCompare.ts +++ b/dashboard/src/types/tree/TreeCompare.ts @@ -50,11 +50,68 @@ export type TreeCompareData = { }; }; +/** Side status for an individual compared item (sketch). */ +export type CompareItemStatus = 'PASS' | 'FAIL' | 'INCONCLUSIVE' | '—'; + +export const compareChangeTypes = [ + 'regression', + 'fixed', + 'newFailure', + 'stillFailing', + 'newPass', +] as const; + +export type CompareChangeType = (typeof compareChangeTypes)[number]; + +export const compareChangeFilters = ['all', ...compareChangeTypes] as const; +export type CompareChangeFilter = (typeof compareChangeFilters)[number]; + +export type CompareChangeStats = Record; + +type CompareFailureRowBase = { + id: string; + change: CompareChangeType; + sideA: CompareItemStatus; + sideB: CompareItemStatus; + date: string; +}; + +export type CompareBuildFailureRow = CompareFailureRowBase & { + config: string; + arch: string; + compiler: string; + buildErrors: number; +}; + +export type CompareBootFailureRow = CompareFailureRowBase & { + path: string; + hardware: string; + duration: string; +}; + +export type CompareTestFailureRow = CompareFailureRowBase & { + path: string; + hardware: string; + duration: string; +}; + +export type TreeCompareFailuresData = { + stats: { + builds: CompareChangeStats; + boots: CompareChangeStats; + tests: CompareChangeStats; + }; + builds: CompareBuildFailureRow[]; + boots: CompareBootFailureRow[]; + tests: CompareTestFailureRow[]; +}; + export const compareDefaultValues = { hashA: '', hashB: '', origin: 'maestro', currentPageTab: 'global.builds' as const, + changeFilter: 'all' as const, }; export const compareSearchSchema = z.object({ @@ -68,9 +125,17 @@ export const compareSearchSchema = z.object({ .enum(possibleTabs) .default(compareDefaultValues.currentPageTab) .catch(compareDefaultValues.currentPageTab), + changeFilter: z + .enum(compareChangeFilters) + .default(compareDefaultValues.changeFilter) + .catch(compareDefaultValues.changeFilter), }); export type CompareSearch = z.infer; export const compareRouteName = '/_main/tree/$treeName/$branch/compare'; export const compareNavigateFrom = '/tree/$treeName/$branch/compare'; +export const compareFailuresRouteName = + '/_main/tree/$treeName/$branch/compare/failures'; +export const compareFailuresNavigateFrom = + '/tree/$treeName/$branch/compare/failures'; From f4016c246a06c9ea938fad003a58698ec67f514e Mon Sep 17 00:00:00 2001 From: Felipe Bergamin Date: Wed, 22 Jul 2026 17:54:03 -0300 Subject: [PATCH 5/8] fixup! feat: add AB comparison details screens Signed-off-by: Felipe Bergamin --- dashboard/src/locales/messages/index.ts | 1 + .../TreeCompare/TreeCompareFailuresPage.tsx | 194 ++++++++++++++-- .../components/CompareDetailSheet.tsx | 210 ++++++++++++++++++ .../components/CompareFailuresTables.tsx | 56 ++++- .../TreeCompare/mock/compareFailuresMock.ts | 114 +++++++++- dashboard/src/types/tree/TreeCompare.ts | 27 +++ 6 files changed, 570 insertions(+), 32 deletions(-) create mode 100644 dashboard/src/pages/TreeCompare/components/CompareDetailSheet.tsx diff --git a/dashboard/src/locales/messages/index.ts b/dashboard/src/locales/messages/index.ts index d3fbbe692..618f38ca1 100644 --- a/dashboard/src/locales/messages/index.ts +++ b/dashboard/src/locales/messages/index.ts @@ -409,6 +409,7 @@ export const messages = { 'Sketch: showing mocked failure data. Backend wiring comes later.', 'treeCompare.failures.pathHardware': 'Path / Hardware', 'treeCompare.failures.testsTitle': 'Test failures', + 'treeCompare.detail.viewFullLog': 'View full log', 'treeCompare.group.boots': 'Platform', 'treeCompare.group.builds': 'Config', 'treeCompare.group.tests': 'Path', diff --git a/dashboard/src/pages/TreeCompare/TreeCompareFailuresPage.tsx b/dashboard/src/pages/TreeCompare/TreeCompareFailuresPage.tsx index 563911b78..6c3fd2b6a 100644 --- a/dashboard/src/pages/TreeCompare/TreeCompareFailuresPage.tsx +++ b/dashboard/src/pages/TreeCompare/TreeCompareFailuresPage.tsx @@ -1,4 +1,4 @@ -import { useCallback, useMemo, type JSX } from 'react'; +import { useCallback, useMemo, useState, type JSX } from 'react'; import { Link, @@ -24,12 +24,18 @@ import ColoredCircle from '@/components/ColoredCircle/ColoredCircle'; import { compareFailuresNavigateFrom, compareFailuresRouteName, + type CompareBootFailureRow, + type CompareBuildFailureRow, type CompareChangeFilter, type CompareChangeStats, + type CompareTestFailureRow, } from '@/types/tree/TreeCompare'; import type { PossibleTabs } from '@/types/tree/TreeDetails'; -import { COMPARE_FAILURES_MOCK } from './mock/compareFailuresMock'; +import { + COMPARE_FAILURES_MOCK, + getCompareDetailMock, +} from './mock/compareFailuresMock'; import { CompareChangeFilterBar, CompareChangeStatsCards, @@ -39,6 +45,7 @@ import { CompareBuildsFailuresTable, CompareTestsFailuresTable, } from './components/CompareFailuresTables'; +import { CompareDetailSheet } from './components/CompareDetailSheet'; const SHORT_HASH_LENGTH = 7; @@ -46,6 +53,16 @@ function failCount(stats: CompareChangeStats): number { return stats.regression + stats.newFailure + stats.stillFailing; } +function applyChangeFilter( + rows: T[], + changeFilter: CompareChangeFilter, +): T[] { + if (changeFilter === 'all') { + return rows; + } + return rows.filter(row => row.change === changeFilter); +} + const TreeCompareFailuresPage = (): JSX.Element => { const { formatMessage } = useIntl(); const { treeName, branch } = useParams({ from: compareFailuresRouteName }); @@ -54,6 +71,8 @@ const TreeCompareFailuresPage = (): JSX.Element => { }); const navigate = useNavigate({ from: compareFailuresNavigateFrom }); + const [selectedId, setSelectedId] = useState(null); + const shortA = hashA.slice(0, SHORT_HASH_LENGTH) || '—'; const shortB = hashB.slice(0, SHORT_HASH_LENGTH) || '—'; @@ -86,15 +105,102 @@ const TreeCompareFailuresPage = (): JSX.Element => { ? mock.stats.tests : mock.stats.builds; - const tabs: ITabItem[] = useMemo(() => { - const applyFilter = (rows: T[]): T[] => { - if (changeFilter === 'all') { - return rows; - } - return rows.filter(row => row.change === changeFilter); - }; + const filteredBuilds = useMemo( + () => applyChangeFilter(mock.builds, changeFilter), + [changeFilter, mock.builds], + ); + const filteredBoots = useMemo( + () => applyChangeFilter(mock.boots, changeFilter), + [changeFilter, mock.boots], + ); + const filteredTests = useMemo( + () => applyChangeFilter(mock.tests, changeFilter), + [changeFilter, mock.tests], + ); + + const activeRows = useMemo(() => { + if (currentPageTab === 'global.boots') { + return filteredBoots; + } + if (currentPageTab === 'global.tests') { + return filteredTests; + } + return filteredBuilds; + }, [currentPageTab, filteredBoots, filteredBuilds, filteredTests]); + + const selectedIndex = useMemo( + () => activeRows.findIndex(row => row.id === selectedId), + [activeRows, selectedId], + ); + + const selectedDetail = useMemo(() => { + if (selectedIndex < 0) { + return null; + } + const row = activeRows[selectedIndex]; + if (!row) { + return null; + } + + if (currentPageTab === 'global.boots') { + const boot = row as CompareBootFailureRow; + return getCompareDetailMock({ + row: boot, + kind: 'boot', + title: boot.path, + subtitle: boot.hardware, + hashA, + hashB, + }); + } + + if (currentPageTab === 'global.tests') { + const test = row as CompareTestFailureRow; + return getCompareDetailMock({ + row: test, + kind: 'test', + title: test.path, + subtitle: test.hardware, + hashA, + hashB, + }); + } + + const build = row as CompareBuildFailureRow; + return getCompareDetailMock({ + row: build, + kind: 'build', + title: build.config, + subtitle: `${build.arch} · ${build.compiler}`, + hashA, + hashB, + }); + }, [activeRows, currentPageTab, hashA, hashB, selectedIndex]); - return [ + const openRow = useCallback((id: string) => { + setSelectedId(id); + }, []); + + const closeSheet = useCallback(() => { + setSelectedId(null); + }, []); + + const goPrevious = useCallback(() => { + if (selectedIndex <= 0) { + return; + } + setSelectedId(activeRows[selectedIndex - 1]?.id ?? null); + }, [activeRows, selectedIndex]); + + const goNext = useCallback(() => { + if (selectedIndex < 0 || selectedIndex >= activeRows.length - 1) { + return; + } + setSelectedId(activeRows[selectedIndex + 1]?.id ?? null); + }, [activeRows, selectedIndex]); + + const tabs: ITabItem[] = useMemo( + () => [ { name: 'global.builds', rightElement: ( @@ -104,7 +210,11 @@ const TreeCompareFailuresPage = (): JSX.Element => { /> ), content: ( - + ), }, { @@ -115,7 +225,13 @@ const TreeCompareFailuresPage = (): JSX.Element => { backgroundClassName="bg-light-red" /> ), - content: , + content: ( + + ), }, { name: 'global.tests', @@ -125,10 +241,26 @@ const TreeCompareFailuresPage = (): JSX.Element => { backgroundClassName="bg-light-red" /> ), - content: , + content: ( + + ), }, - ]; - }, [changeFilter, mock]); + ], + [ + filteredBoots, + filteredBuilds, + filteredTests, + mock.stats.boots, + mock.stats.builds, + mock.stats.tests, + openRow, + selectedId, + ], + ); const entityTitleId = currentPageTab === 'global.boots' @@ -228,7 +360,7 @@ const TreeCompareFailuresPage = (): JSX.Element => {
-
+
@@ -236,18 +368,40 @@ const TreeCompareFailuresPage = (): JSX.Element => { updateSearch({ changeFilter: value })} + onChange={value => { + setSelectedId(null); + updateSearch({ changeFilter: value }); + }} /> - updateSearch({ currentPageTab: value as PossibleTabs }) - } + onValueChange={value => { + setSelectedId(null); + updateSearch({ currentPageTab: value as PossibleTabs }); + }} />
+ + { + if (!open) { + closeSheet(); + } + }} + onPrevious={goPrevious} + onNext={goNext} + hasPrevious={selectedIndex > 0} + hasNext={ + selectedIndex >= 0 && selectedIndex < activeRows.length - 1 + } + /> ); }; diff --git a/dashboard/src/pages/TreeCompare/components/CompareDetailSheet.tsx b/dashboard/src/pages/TreeCompare/components/CompareDetailSheet.tsx new file mode 100644 index 000000000..9afa054dd --- /dev/null +++ b/dashboard/src/pages/TreeCompare/components/CompareDetailSheet.tsx @@ -0,0 +1,210 @@ +import { useEffect, useMemo, useState, type JSX } from 'react'; + +import { FormattedMessage } from 'react-intl'; + +import { Sheet } from '@/components/Sheet'; +import { WrapperSheetContent } from '@/components/Sheet/WrapperSheetContent'; +import { LogExcerpt } from '@/components/Log/LogExcerpt'; +import { Button } from '@/components/ui/button'; + +import type { CompareDetailItem, CompareItemStatus } from '@/types/tree/TreeCompare'; + +import { cn } from '@/lib/utils'; + +import { + CompareChangeBadge, + CompareStatusChip, +} from './CompareChangeDisplay'; + +type LogSide = 'A' | 'B'; + +function defaultSide(item: CompareDetailItem): LogSide { + if (item.sideB.status === 'FAIL') { + return 'B'; + } + if (item.sideA.status === 'FAIL') { + return 'A'; + } + return 'B'; +} + +function SideTab({ + label, + status, + active, + onClick, +}: { + label: string; + status: CompareItemStatus; + active: boolean; + onClick: () => void; +}): JSX.Element { + return ( + + ); +} + +interface CompareDetailSheetProps { + open: boolean; + item: CompareDetailItem | null; + treeName: string; + branch: string; + onOpenChange: (open: boolean) => void; + onPrevious: () => void; + onNext: () => void; + hasPrevious: boolean; + hasNext: boolean; +} + +export function CompareDetailSheet({ + open, + item, + treeName, + branch, + onOpenChange, + onPrevious, + onNext, + hasPrevious, + hasNext, +}: CompareDetailSheetProps): JSX.Element { + const [logSide, setLogSide] = useState('B'); + + useEffect(() => { + if (item) { + setLogSide(defaultSide(item)); + } + }, [item]); + + const activeSide = useMemo(() => { + if (!item) { + return null; + } + return logSide === 'A' ? item.sideA : item.sideB; + }, [item, logSide]); + + return ( + { + if (!next) { + onOpenChange(false); + } + }} + > + + + + } + > + {item && activeSide && ( +
+
+

{item.title}

+

{item.subtitle}

+
+ + → + + +
+
+ +
+ setLogSide('A')} + /> + setLogSide('B')} + /> +
+ +
+
+

+ {treeName}/{branch} ·{' '} + + {activeSide.hash.slice(0, 7) || '—'} + +

+ + + +
+
+ + + {item.title} + +
+
+ + + + {item.issues.length > 0 && ( +
+

+ +

+
    + {item.issues.map(issue => ( +
  • +

    + {issue.comment} +

    +

    + maestro · first seen {issue.firstSeen} +

    +
    • + ))} +
+
+ )} +
+ )} + + + ); +} diff --git a/dashboard/src/pages/TreeCompare/components/CompareFailuresTables.tsx b/dashboard/src/pages/TreeCompare/components/CompareFailuresTables.tsx index 9a855fa90..15ff9d7e4 100644 --- a/dashboard/src/pages/TreeCompare/components/CompareFailuresTables.tsx +++ b/dashboard/src/pages/TreeCompare/components/CompareFailuresTables.tsx @@ -26,13 +26,16 @@ import { isFailureHighlight, } from './CompareChangeDisplay'; -function DetailsPlaceholder(): JSX.Element { +function DetailsButton({ onClick }: { onClick: () => void }): JSX.Element { return ( @@ -63,11 +66,18 @@ function SideCells({ ); } +type TableSelectionProps = { + selectedId?: string | null; + onRowClick: (id: string) => void; +}; + export function CompareBuildsFailuresTable({ rows, + selectedId, + onRowClick, }: { rows: CompareBuildFailureRow[]; -}): JSX.Element { +} & TableSelectionProps): JSX.Element { return (
@@ -99,7 +109,13 @@ export function CompareBuildsFailuresTable({ {rows.map(row => ( onRowClick(row.id)} >
{row.config}
@@ -113,10 +129,12 @@ export function CompareBuildsFailuresTable({ - {row.buildErrors} + + {row.buildErrors} + {row.date} - + onRowClick(row.id)} /> ))} @@ -129,10 +147,12 @@ export function CompareBuildsFailuresTable({ function PathHardwareTable({ rows, pathLabelId, + selectedId, + onRowClick, }: { rows: Array; pathLabelId: 'treeCompare.failures.pathHardware'; -}): JSX.Element { +} & TableSelectionProps): JSX.Element { return (
@@ -164,7 +184,13 @@ function PathHardwareTable({ {rows.map(row => ( onRowClick(row.id)} >
{row.path}
@@ -181,7 +207,7 @@ function PathHardwareTable({ {row.date} - + onRowClick(row.id)} /> ))} @@ -193,26 +219,34 @@ function PathHardwareTable({ export function CompareBootsFailuresTable({ rows, + selectedId, + onRowClick, }: { rows: CompareBootFailureRow[]; -}): JSX.Element { +} & TableSelectionProps): JSX.Element { return ( ); } export function CompareTestsFailuresTable({ rows, + selectedId, + onRowClick, }: { rows: CompareTestFailureRow[]; -}): JSX.Element { +} & TableSelectionProps): JSX.Element { return ( ); } diff --git a/dashboard/src/pages/TreeCompare/mock/compareFailuresMock.ts b/dashboard/src/pages/TreeCompare/mock/compareFailuresMock.ts index 6af514c61..1279de9ac 100644 --- a/dashboard/src/pages/TreeCompare/mock/compareFailuresMock.ts +++ b/dashboard/src/pages/TreeCompare/mock/compareFailuresMock.ts @@ -1,4 +1,8 @@ -import type { TreeCompareFailuresData } from '@/types/tree/TreeCompare'; +import type { + CompareDetailItem, + CompareItemStatus, + TreeCompareFailuresData, +} from '@/types/tree/TreeCompare'; /** Sketch-only mock for compare failure drill-down. Replace with API later. */ export const COMPARE_FAILURES_MOCK: TreeCompareFailuresData = { @@ -228,3 +232,111 @@ export const COMPARE_FAILURES_MOCK: TreeCompareFailuresData = { }, ], }; + +const PASS_LOG = `[ 12.001] Build completed successfully +[ 12.002] Linking vmlinux +[ 12.450] Kernel: arch/arm64/boot/Image.gz is ready +[ 12.451] DONE`; + +const FAIL_BUILD_LOG = `[ 142.318] make[2]: *** [scripts/Makefile.build:480: drivers/net] Error 1 +[ 142.319] make[1]: *** [Makefile:2011: drivers] Error 2 +[ 142.320] make: *** [Makefile:350: __build_one_by_one] Error 2 + +error: implicit declaration of function 'foo_bar' + CC drivers/net/ethernet/foo.o +#include + ^~~~~~~~~~~~~ +Build failed after 142s`; + +const FAIL_TEST_LOG = `[ 8.102] # selftests: mm:compaction_test +[ 8.110] not ok 1 compaction_test +[ 8.111] # Assertion failed: pages_moved > 0 +[ 8.112] # FAIL: Unexpected page migration count +[ 8.200] Tests failed: 1`; + +const PASS_TEST_LOG = `[ 5.001] # selftests: mm:compaction_test +[ 5.050] ok 1 compaction_test +[ 5.051] Tests passed: 1`; + +const FAIL_BOOT_LOG = `[ 0.000] Booting Linux on physical CPU 0x000 +[ 12.400] Kernel panic - not syncing: Attempted to kill init! +[ 12.401] ---[ end Kernel panic ]---`; + +const PASS_BOOT_LOG = `[ 0.000] Booting Linux on physical CPU 0x000 +[ 18.200] Freeing unused kernel memory +[ 18.201] Run /sbin/init as init process +[ 22.000] login:`; + +/** Sketch-only per-row detail payloads for the compare Log Viewer sheet. */ +export function getCompareDetailMock(args: { + row: { + id: string; + change: TreeCompareFailuresData['builds'][number]['change']; + sideA: CompareItemStatus; + sideB: CompareItemStatus; + }; + kind: 'build' | 'boot' | 'test'; + title: string; + subtitle: string; + hashA: string; + hashB: string; +}): CompareDetailItem { + const failLog = + args.kind === 'build' + ? FAIL_BUILD_LOG + : args.kind === 'boot' + ? FAIL_BOOT_LOG + : FAIL_TEST_LOG; + const passLog = + args.kind === 'build' + ? PASS_LOG + : args.kind === 'boot' + ? PASS_BOOT_LOG + : PASS_TEST_LOG; + + const logFor = (status: typeof args.row.sideA): string => { + if (status === 'FAIL') { + return failLog; + } + if (status === 'PASS') { + return passLog; + } + return 'No log available for this side (item missing on this revision).'; + }; + + const issues = + args.row.sideB === 'FAIL' + ? [ + { + id: `mock-issue-${args.row.id}`, + version: 1, + comment: + args.kind === 'build' + ? 'net: foo: undeclared identifier in allmodconfig builds' + : args.kind === 'boot' + ? 'boot: kernel panic while killing init on this platform' + : 'selftest assertion failure on this hardware', + firstSeen: '2026-07-13', + }, + ] + : []; + + return { + id: args.row.id, + kind: args.kind, + title: args.title, + subtitle: args.subtitle, + change: args.row.change, + sideA: { + status: args.row.sideA, + hash: args.hashA, + logExcerpt: logFor(args.row.sideA), + }, + sideB: { + status: args.row.sideB, + hash: args.hashB, + logExcerpt: logFor(args.row.sideB), + }, + issues, + }; +} diff --git a/dashboard/src/types/tree/TreeCompare.ts b/dashboard/src/types/tree/TreeCompare.ts index 194c2e8cf..ecf132127 100644 --- a/dashboard/src/types/tree/TreeCompare.ts +++ b/dashboard/src/types/tree/TreeCompare.ts @@ -95,6 +95,33 @@ export type CompareTestFailureRow = CompareFailureRowBase & { duration: string; }; +export type CompareFailureRow = + | CompareBuildFailureRow + | CompareBootFailureRow + | CompareTestFailureRow; + +export type CompareDetailSide = { + status: CompareItemStatus; + hash: string; + logExcerpt: string; +}; + +export type CompareDetailItem = { + id: string; + kind: 'build' | 'boot' | 'test'; + title: string; + subtitle: string; + change: CompareChangeType; + sideA: CompareDetailSide; + sideB: CompareDetailSide; + issues: Array<{ + id: string; + version: number; + comment: string; + firstSeen: string; + }>; +}; + export type TreeCompareFailuresData = { stats: { builds: CompareChangeStats; From a85e85b3cc03cf67030f0cabb98b3e16b7616560 Mon Sep 17 00:00:00 2001 From: Felipe Bergamin Date: Wed, 22 Jul 2026 17:54:34 -0300 Subject: [PATCH 6/8] wip: backend endpoint to AB details Signed-off-by: Felipe Bergamin --- backend/kernelCI_app/helpers/treeCompare.py | 160 +++++++++++++++++- backend/kernelCI_app/queries/tree.py | 135 ++++++++++++++- .../queries/treeCompareQueries_test.py | 37 ++++ .../views/treeCompareBootsView_test.py | 134 +++++++++++++++ .../kernelCI_app/typeModels/treeCompare.py | 16 ++ backend/kernelCI_app/urls.py | 16 +- .../views/treeCompareBootsView.py | 74 ++++++++ backend/requests/tree-compare-boots-get.sh | 17 ++ 8 files changed, 581 insertions(+), 8 deletions(-) create mode 100644 backend/kernelCI_app/tests/unitTests/views/treeCompareBootsView_test.py create mode 100644 backend/kernelCI_app/views/treeCompareBootsView.py create mode 100644 backend/requests/tree-compare-boots-get.sh diff --git a/backend/kernelCI_app/helpers/treeCompare.py b/backend/kernelCI_app/helpers/treeCompare.py index be208ebd7..8835477ea 100644 --- a/backend/kernelCI_app/helpers/treeCompare.py +++ b/backend/kernelCI_app/helpers/treeCompare.py @@ -1,9 +1,10 @@ from collections import defaultdict from dataclasses import dataclass, field -from typing import Callable, Literal, Optional +from typing import TYPE_CHECKING, Callable, Literal, Optional -from kernelCI_app.constants.general import UNKNOWN_STRING +from kernelCI_app.constants.general import UNCATEGORIZED_STRING, UNKNOWN_STRING from kernelCI_app.constants.process_pending import ROLLUP_STATUS_FIELDS +from kernelCI_app.typeModels.databases import NULL_STATUS from kernelCI_app.typeModels.treeCompare import ( CompareDelta, CompareEntitySummary, @@ -14,9 +15,164 @@ TreeCompareResponse, ) +if TYPE_CHECKING: + from kernelCI_app.helpers.filters import FilterParams + BucketKey = Literal["pass", "fail", "inconclusive"] +def _boot_status_filter_clause( + statuses: set[str], + params: dict, +) -> str: + status_list = list(statuses) + include_null = NULL_STATUS in status_list + concrete = [status for status in status_list if status != NULL_STATUS] + if include_null and concrete: + params["boot_statuses"] = concrete + return "AND (t.status IS NULL OR t.status = ANY(%(boot_statuses)s))" + if include_null: + return "AND t.status IS NULL" + params["boot_statuses"] = concrete + return "AND t.status = ANY(%(boot_statuses)s)" + + +def _boot_issue_filter_clause( + issue_filters: set[tuple[str, Optional[int]]], + params: dict, +) -> str: + known_issues = [ + (issue_id, issue_version) + for issue_id, issue_version in issue_filters + if issue_id != UNCATEGORIZED_STRING + ] + has_uncategorized = any( + issue_id == UNCATEGORIZED_STRING for issue_id, _ in issue_filters + ) + issue_parts: list[str] = [] + if known_issues: + issue_tuples = [] + for index, (issue_id, issue_version) in enumerate(known_issues): + id_key = f"boot_issue_id_{index}" + version_key = f"boot_issue_version_{index}" + params[id_key] = issue_id + params[version_key] = issue_version + if issue_version is None: + issue_tuples.append( + f"(i.issue_id = %({id_key})s AND i.issue_version IS NULL)" + ) + else: + issue_tuples.append( + f"(i.issue_id = %({id_key})s" + f" AND i.issue_version = %({version_key})s)" + ) + issue_parts.append( + f"""EXISTS ( + SELECT 1 FROM incidents i + WHERE i.test_id = t.id + AND ({' OR '.join(issue_tuples)}) + )""" + ) + if has_uncategorized: + issue_parts.append( + """NOT EXISTS ( + SELECT 1 FROM incidents i + WHERE i.test_id = t.id + )""" + ) + if not issue_parts: + return "" + return f"AND ({' OR '.join(issue_parts)})" + + +def build_boot_compare_filter_clauses( # noqa: C901 - maps FilterParams fields to SQL AND clauses + filters: Optional["FilterParams"], +) -> tuple[str, dict]: + """Build optional WHERE fragments for boot compare rows (applied before A/B join).""" + if filters is None: + return "", {} + + clauses: list[str] = [] + params: dict = {} + + if filters.filterBootPath: + clauses.append("AND strpos(t.path, %(boot_path)s) > 0") + params["boot_path"] = filters.filterBootPath + + if filters.filterBootStatus: + clauses.append(_boot_status_filter_clause(filters.filterBootStatus, params)) + + if ( + filters.filterBootDurationMin is not None + or filters.filterBootDurationMax is not None + ): + clauses.append("AND t.duration IS NOT NULL") + if filters.filterBootDurationMin is not None: + clauses.append("AND t.duration >= %(boot_duration_min)s") + params["boot_duration_min"] = filters.filterBootDurationMin + if filters.filterBootDurationMax is not None: + clauses.append("AND t.duration <= %(boot_duration_max)s") + params["boot_duration_max"] = filters.filterBootDurationMax + + if filters.filterPlatforms["boot"]: + clauses.append( + "AND COALESCE(t.environment_misc->>'platform', %(unknown_string)s)" + " = ANY(%(boot_platforms)s)" + ) + params["boot_platforms"] = list(filters.filterPlatforms["boot"]) + params["unknown_string"] = UNKNOWN_STRING + + if filters.filter_boot_origin: + clauses.append("AND t.origin = ANY(%(boot_origins)s)") + params["boot_origins"] = list(filters.filter_boot_origin) + + if filters.filterConfigs: + clauses.append( + "AND COALESCE(b.config_name, %(unknown_string)s) = ANY(%(configs)s)" + ) + params["configs"] = list(filters.filterConfigs) + params["unknown_string"] = UNKNOWN_STRING + + if filters.filterArchitecture: + clauses.append( + "AND COALESCE(b.architecture, %(unknown_string)s) = ANY(%(architectures)s)" + ) + params["architectures"] = list(filters.filterArchitecture) + params["unknown_string"] = UNKNOWN_STRING + + if filters.filterCompiler: + clauses.append( + "AND COALESCE(b.compiler, %(unknown_string)s) = ANY(%(compilers)s)" + ) + params["compilers"] = list(filters.filterCompiler) + params["unknown_string"] = UNKNOWN_STRING + + if filters.filterHardware: + clauses.append( + """ + AND ( + t.environment_compatible && %(hardware)s::text[] + OR t.environment_misc->>'platform' = ANY(%(hardware)s::text[]) + ) + """ + ) + params["hardware"] = list(filters.filterHardware) + + if filters.filter_labs: + clauses.append( + "AND COALESCE(t.misc->>'runtime', %(unknown_string)s) = ANY(%(labs)s)" + ) + params["labs"] = list(filters.filter_labs) + params["unknown_string"] = UNKNOWN_STRING + + if filters.filterIssues["boot"]: + issue_clause = _boot_issue_filter_clause(filters.filterIssues["boot"], params) + if issue_clause: + clauses.append(issue_clause) + + return ("\n".join(clauses), params) + + @dataclass class _HashAccumulator: builds: CompareStatusCounts = field(default_factory=CompareStatusCounts) diff --git a/backend/kernelCI_app/queries/tree.py b/backend/kernelCI_app/queries/tree.py index 0e1d64b49..d31f2cd41 100644 --- a/backend/kernelCI_app/queries/tree.py +++ b/backend/kernelCI_app/queries/tree.py @@ -1,10 +1,12 @@ -from typing import Literal, Optional +from typing import TYPE_CHECKING, Literal, Optional from django.db import connection from django.db.models import Q from kernelCI_app.cache import get_query_cache, set_query_cache +from kernelCI_app.constants.general import UNKNOWN_STRING from kernelCI_app.helpers.database import dict_fetchall +from kernelCI_app.helpers.treeCompare import build_boot_compare_filter_clauses from kernelCI_app.helpers.treeDetails import create_checkouts_where_clauses from kernelCI_app.models import Checkouts from kernelCI_app.queries.duration import ( @@ -12,6 +14,9 @@ get_build_duration_clause, ) +if TYPE_CHECKING: + from kernelCI_app.helpers.filters import FilterParams + def _get_tree_listing_count_clause() -> str: build_count_clause = """ @@ -565,6 +570,9 @@ def get_tree_data( issues."_timestamp" DESC """ + print(query); + print(params); + with connection.cursor() as cursor: cursor.execute(query, params) rows = cursor.fetchall() @@ -1376,3 +1384,128 @@ def get_tree_compare_builds( return rows + +def get_tree_compare_boots( + *, + hash_a: str, + hash_b: str, + origin_param: str, + git_branch_param: Optional[str], + tree_name: Optional[str] = None, + filters: Optional["FilterParams"] = None, +) -> list[dict]: + """Return boot rows whose grouped status differs between two commits. + + Identity key: path + config_name + platform. + Statuses are bucketed to PASS / FAIL / INCONCLUSIVE before compare so + SKIP vs MISS is not treated as a difference. + """ + commit_hashes = [hash_a, hash_b] + filter_clauses, filter_params = build_boot_compare_filter_clauses(filters) + + cache_key = "treeCompareBoots" + params = { + "hash_a": hash_a, + "hash_b": hash_b, + "commit_hashes": commit_hashes, + "tree_name": tree_name, + "origin_param": origin_param, + "git_branch_param": git_branch_param, + "unknown_string": UNKNOWN_STRING, + **filter_params, + } + + rows = get_query_cache(cache_key, params) + if rows is not None: + return rows + + git_branch_clause, tree_name_full_clause = _get_compare_checkout_clauses( + git_branch_param=git_branch_param, + tree_name=tree_name, + ) + + query = f""" + WITH RELEVANT_CHECKOUTS AS ( + SELECT DISTINCT ON (c.git_commit_hash) + c.id AS checkout_id, + c.git_commit_hash + FROM + checkouts c + WHERE + c.git_commit_hash = ANY(%(commit_hashes)s) + {tree_name_full_clause} + AND {git_branch_clause} + AND c.origin = %(origin_param)s + ORDER BY + c.git_commit_hash, + c._timestamp DESC + ), + BOOT_ROWS AS ( + SELECT DISTINCT ON ( + rc.git_commit_hash, + t.path, + COALESCE(b.config_name, %(unknown_string)s), + COALESCE(t.environment_misc->>'platform', %(unknown_string)s) + ) + rc.git_commit_hash, + t.path, + COALESCE(b.config_name, %(unknown_string)s) AS config_name, + COALESCE( + t.environment_misc->>'platform', + %(unknown_string)s + ) AS platform, + CASE + WHEN UPPER(t.status) = 'PASS' THEN 'PASS' + WHEN UPPER(t.status) = 'FAIL' THEN 'FAIL' + ELSE 'INCONCLUSIVE' + END AS grouped_status + FROM + RELEVANT_CHECKOUTS rc + INNER JOIN builds b ON b.checkout_id = rc.checkout_id + INNER JOIN tests t ON t.build_id = b.id + AND (t.path = 'boot' OR t.path LIKE 'boot.%%') + WHERE + b.id NOT LIKE 'maestro:dummy_%%' + {filter_clauses} + ORDER BY + rc.git_commit_hash, + t.path, + COALESCE(b.config_name, %(unknown_string)s), + COALESCE(t.environment_misc->>'platform', %(unknown_string)s), + t.start_time DESC NULLS LAST, + t._timestamp DESC NULLS LAST + ), + SIDE_A AS ( + SELECT path, config_name, platform, grouped_status + FROM BOOT_ROWS + WHERE git_commit_hash = %(hash_a)s + ), + SIDE_B AS ( + SELECT path, config_name, platform, grouped_status + FROM BOOT_ROWS + WHERE git_commit_hash = %(hash_b)s + ) + SELECT + COALESCE(a.path, b.path) AS path, + COALESCE(a.config_name, b.config_name) AS config_name, + COALESCE(a.platform, b.platform) AS platform, + a.grouped_status AS status_a, + b.grouped_status AS status_b + FROM + SIDE_A a + FULL OUTER JOIN SIDE_B b ON ( + a.path = b.path + AND a.config_name = b.config_name + AND a.platform = b.platform + ) + WHERE + a.grouped_status IS DISTINCT FROM b.grouped_status + """ + + with connection.cursor() as cursor: + cursor.execute(query, params) + rows = dict_fetchall(cursor=cursor) + set_query_cache(key=cache_key, params=params, rows=rows) + + return rows + diff --git a/backend/kernelCI_app/tests/unitTests/queries/treeCompareQueries_test.py b/backend/kernelCI_app/tests/unitTests/queries/treeCompareQueries_test.py index 0d213e398..a7f4de6d2 100644 --- a/backend/kernelCI_app/tests/unitTests/queries/treeCompareQueries_test.py +++ b/backend/kernelCI_app/tests/unitTests/queries/treeCompareQueries_test.py @@ -3,6 +3,7 @@ from django.test import SimpleTestCase from kernelCI_app.queries.tree import ( + get_tree_compare_boots, get_tree_compare_builds, get_tree_compare_rollup, ) @@ -65,3 +66,39 @@ def test_compare_builds_aggregates_in_sql( self.assertIn("GROUP BY", executed_query) self.assertNotIn("known_issues", executed_query) self.assertNotIn("SELECT\n b.id AS build_id", executed_query) + + @patch("kernelCI_app.queries.tree.connection") + @patch("kernelCI_app.queries.tree.get_query_cache", return_value=None) + @patch("kernelCI_app.queries.tree.set_query_cache") + @patch("kernelCI_app.queries.tree.dict_fetchall", return_value=[]) + def test_compare_boots_joins_and_diffs_in_sql( + self, + mock_fetchall, + mock_set_cache, + mock_get_cache, + mock_connection, + ): + mock_cursor = MagicMock() + mock_connection.cursor.return_value.__enter__.return_value = mock_cursor + + get_tree_compare_boots( + hash_a="hash_a", + hash_b="hash_b", + origin_param="maestro", + git_branch_param="master", + tree_name="linux", + ) + + executed_query = mock_cursor.execute.call_args[0][0] + params = mock_cursor.execute.call_args[0][1] + self.assertIn("git_commit_hash = ANY(%(commit_hashes)s)", executed_query) + self.assertIn("DISTINCT ON (c.git_commit_hash)", executed_query) + self.assertIn("FULL OUTER JOIN", executed_query) + self.assertIn("IS DISTINCT FROM", executed_query) + self.assertIn("WHEN UPPER(t.status) = 'PASS' THEN 'PASS'", executed_query) + self.assertIn("WHEN UPPER(t.status) = 'FAIL' THEN 'FAIL'", executed_query) + self.assertIn("ELSE 'INCONCLUSIVE'", executed_query) + self.assertIn("t.path LIKE 'boot.%%'", executed_query) + self.assertNotIn("incidents", executed_query) + self.assertNotIn("issues", executed_query) + self.assertEqual(params["commit_hashes"], ["hash_a", "hash_b"]) diff --git a/backend/kernelCI_app/tests/unitTests/views/treeCompareBootsView_test.py b/backend/kernelCI_app/tests/unitTests/views/treeCompareBootsView_test.py new file mode 100644 index 000000000..d9ef86310 --- /dev/null +++ b/backend/kernelCI_app/tests/unitTests/views/treeCompareBootsView_test.py @@ -0,0 +1,134 @@ +from unittest.mock import MagicMock, patch + +from django.test import SimpleTestCase +from rest_framework.test import APIRequestFactory + +from kernelCI_app.helpers.treeCompare import build_boot_compare_filter_clauses +from kernelCI_app.views.treeCompareBootsView import TreeCompareBootsView + +HASH_A = "a" * 40 +HASH_B = "b" * 40 + +BOOT_DIFF_ROW = { + "path": "boot", + "config_name": "defconfig", + "platform": "qemu-arm64", + "status_a": "FAIL", + "status_b": "PASS", +} + + +class TestBuildBootCompareFilterClauses(SimpleTestCase): + def test_empty_filters_add_no_clauses(self): + clauses, params = build_boot_compare_filter_clauses(None) + self.assertEqual(clauses, "") + self.assertEqual(params, {}) + + def test_boot_path_and_status_filters(self): + filters = MagicMock() + filters.filterBootPath = "boot.login" + filters.filterBootStatus = {"FAIL", "NULL"} + filters.filterBootDurationMin = None + filters.filterBootDurationMax = None + filters.filterPlatforms = {"boot": set(), "test": set()} + filters.filter_boot_origin = set() + filters.filterConfigs = set() + filters.filterArchitecture = set() + filters.filterCompiler = set() + filters.filterHardware = set() + filters.filter_labs = set() + filters.filterIssues = {"boot": set(), "build": set(), "test": set()} + + clauses, params = build_boot_compare_filter_clauses(filters) + self.assertIn("strpos(t.path, %(boot_path)s) > 0", clauses) + self.assertIn("t.status IS NULL OR t.status = ANY(%(boot_statuses)s)", clauses) + self.assertEqual(params["boot_path"], "boot.login") + self.assertEqual(params["boot_statuses"], ["FAIL"]) + + +class TestTreeCompareBootsView(SimpleTestCase): + def setUp(self): + self.factory = APIRequestFactory() + self.view = TreeCompareBootsView.as_view() + self.url = "/api/tree/linux/master/compare/boots" + + @patch("kernelCI_app.views.treeCompareBootsView.get_tree_compare_boots") + def test_get_returns_boots_payload(self, mock_boots): + mock_boots.return_value = [BOOT_DIFF_ROW] + + request = self.factory.get( + self.url, + { + "hash_a": HASH_A, + "hash_b": HASH_B, + "origin": "maestro", + }, + ) + response = self.view(request, tree_name="linux", git_branch="master") + + self.assertEqual(response.status_code, 200) + self.assertEqual(response.data["boots"], [BOOT_DIFF_ROW]) + self.assertIn("path", response.data["boots"][0]) + self.assertIn("config_name", response.data["boots"][0]) + self.assertIn("status_a", response.data["boots"][0]) + self.assertIn("status_b", response.data["boots"][0]) + self.assertNotIn("configName", response.data["boots"][0]) + mock_boots.assert_called_once() + self.assertEqual(mock_boots.call_args.kwargs["hash_a"], HASH_A) + self.assertEqual(mock_boots.call_args.kwargs["hash_b"], HASH_B) + + def test_missing_hashes_returns_400(self): + request = self.factory.get( + self.url, + { + "hash_a": HASH_A, + "origin": "maestro", + }, + ) + response = self.view(request, tree_name="linux", git_branch="master") + self.assertEqual(response.status_code, 400) + + @patch("kernelCI_app.views.treeCompareBootsView.get_tree_compare_boots") + def test_skip_vs_miss_same_bucket_yields_empty_diff(self, mock_boots): + # SQL buckets SKIP and MISS both to INCONCLUSIVE, so they never appear + # as a diff row. The query layer returns [] for that case. + mock_boots.return_value = [] + + request = self.factory.get( + self.url, + { + "hash_a": HASH_A, + "hash_b": HASH_B, + "origin": "maestro", + }, + ) + response = self.view(request, tree_name="linux", git_branch="master") + + self.assertEqual(response.status_code, 200) + self.assertEqual(response.data["boots"], []) + + @patch("kernelCI_app.views.treeCompareBootsView.get_tree_compare_boots") + def test_one_sided_null_status(self, mock_boots): + mock_boots.return_value = [ + { + "path": "boot", + "config_name": "defconfig", + "platform": "qemu-arm64", + "status_a": "PASS", + "status_b": None, + } + ] + + request = self.factory.get( + self.url, + { + "hash_a": HASH_A, + "hash_b": HASH_B, + "origin": "maestro", + }, + ) + response = self.view(request, tree_name="linux", git_branch="master") + + self.assertEqual(response.status_code, 200) + self.assertEqual(response.data["boots"][0]["status_a"], "PASS") + self.assertIsNone(response.data["boots"][0]["status_b"]) diff --git a/backend/kernelCI_app/typeModels/treeCompare.py b/backend/kernelCI_app/typeModels/treeCompare.py index bd303e09d..e527646de 100644 --- a/backend/kernelCI_app/typeModels/treeCompare.py +++ b/backend/kernelCI_app/typeModels/treeCompare.py @@ -1,8 +1,12 @@ +from typing import Literal, Optional + from pydantic import BaseModel, ConfigDict, Field from kernelCI_app.constants.general import DEFAULT_ORIGIN from kernelCI_app.constants.localization import DocStrings +CompareGroupedStatus = Literal["PASS", "FAIL", "INCONCLUSIVE"] + class CompareStatusCounts(BaseModel): model_config = ConfigDict(populate_by_name=True) @@ -66,3 +70,15 @@ class TreeCompareQueryParameters(BaseModel): default=DEFAULT_ORIGIN, description=DocStrings.TREE_QUERY_ORIGIN_DESCRIPTION, ) + + +class CompareBootDiffRow(BaseModel): + path: str + config_name: str + platform: str + status_a: Optional[CompareGroupedStatus] = None + status_b: Optional[CompareGroupedStatus] = None + + +class TreeCompareBootsResponse(BaseModel): + boots: list[CompareBootDiffRow] diff --git a/backend/kernelCI_app/urls.py b/backend/kernelCI_app/urls.py index 57f5a37fa..04c22469b 100644 --- a/backend/kernelCI_app/urls.py +++ b/backend/kernelCI_app/urls.py @@ -62,6 +62,17 @@ def view_cache(view, timeout: int = settings.CACHE_TIMEOUT): view_cache(views.TreeCommitsListView), name="treeCommitsList", ), + # Compare routes must come before ...//boots (else commit_hash="compare") + path( + "tree///compare/boots", + view_cache(views.TreeCompareBootsView), + name="treeCompareBoots", + ), + path( + "tree///compare", + view_cache(views.TreeCompareView), + name="treeCompare", + ), path( "tree////commits", view_cache(views.TreeCommitsHistoryDirect), @@ -92,11 +103,6 @@ def view_cache(view, timeout: int = settings.CACHE_TIMEOUT): views.TreeDetailsTestsDirect.as_view(), name="treeDetailsTestsDirectView", ), - path( - "tree///compare", - view_cache(views.TreeCompareView), - name="treeCompare", - ), path( "tree//", view_cache(views.TreeLatest), diff --git a/backend/kernelCI_app/views/treeCompareBootsView.py b/backend/kernelCI_app/views/treeCompareBootsView.py new file mode 100644 index 000000000..84ab283a9 --- /dev/null +++ b/backend/kernelCI_app/views/treeCompareBootsView.py @@ -0,0 +1,74 @@ +from http import HTTPStatus + +from django.http import HttpRequest +from drf_spectacular.utils import extend_schema +from pydantic import ValidationError +from rest_framework.response import Response +from rest_framework.views import APIView + +from kernelCI_app.constants.general import DEFAULT_ORIGIN +from kernelCI_app.helpers.errorHandling import create_api_error_response +from kernelCI_app.helpers.filters import FilterParams +from kernelCI_app.queries.tree import get_tree_compare_boots +from kernelCI_app.typeModels.commonOpenApiParameters import ( + GIT_BRANCH_PATH_PARAM, + TREE_NAME_PATH_PARAM, +) +from kernelCI_app.typeModels.treeCompare import ( + TreeCompareBootsResponse, + TreeCompareQueryParameters, +) + + +class TreeCompareBootsView(APIView): + @extend_schema( + parameters=[ + TREE_NAME_PATH_PARAM, + GIT_BRANCH_PATH_PARAM, + TreeCompareQueryParameters, + ], + responses=TreeCompareBootsResponse, + ) + def get( + self, + request: HttpRequest, + tree_name: str, + git_branch: str, + ) -> Response: + try: + query_params = TreeCompareQueryParameters( + hash_a=request.GET.get("hash_a", ""), + hash_b=request.GET.get("hash_b", ""), + origin=request.GET.get("origin", DEFAULT_ORIGIN), + ) + except ValidationError as error: + return Response(data=error.json(), status=HTTPStatus.BAD_REQUEST) + + if not query_params.hash_a or not query_params.hash_b: + return create_api_error_response( + status_code=HTTPStatus.BAD_REQUEST, + error_message="hash_a and hash_b are required", + ) + + filters = FilterParams(request) + rows = get_tree_compare_boots( + hash_a=query_params.hash_a, + hash_b=query_params.hash_b, + origin_param=query_params.origin, + git_branch_param=git_branch, + tree_name=tree_name, + filters=filters, + ) + + try: + response = TreeCompareBootsResponse(boots=rows) + except ValidationError as error: + return Response( + data=error.json(), + status=HTTPStatus.INTERNAL_SERVER_ERROR, + ) + + return Response( + data=response.model_dump(), + status=HTTPStatus.OK, + ) diff --git a/backend/requests/tree-compare-boots-get.sh b/backend/requests/tree-compare-boots-get.sh new file mode 100644 index 000000000..6e3581001 --- /dev/null +++ b/backend/requests/tree-compare-boots-get.sh @@ -0,0 +1,17 @@ +http 'http://localhost:8000/api/tree/linux/master/compare/boots' \ + origin==maestro \ + hash_a==abc1234567890abcdef1234567890abcdef12 \ + hash_b==def5678901234abcdef5678901234abcdef56 + +# HTTP/1.1 200 OK +# { +# "boots": [ +# { +# "path": "boot", +# "config_name": "defconfig", +# "platform": "qemu-arm64", +# "status_a": "FAIL", +# "status_b": "PASS" +# } +# ] +# } From b5d03f2401fbd1c5ff32ce56c79876be3bee8775 Mon Sep 17 00:00:00 2001 From: Felipe Bergamin Date: Thu, 23 Jul 2026 18:22:08 -0300 Subject: [PATCH 7/8] feat(ab-compare): build details endpoint --- backend/kernelCI_app/helpers/treeCompare.py | 155 ++++++++++++++++++ backend/kernelCI_app/queries/tree.py | 127 +++++++++++++- .../queries/treeCompareQueries_test.py | 38 +++++ .../views/treeCompareBuildsView_test.py | 132 +++++++++++++++ .../kernelCI_app/typeModels/treeCompare.py | 12 ++ backend/kernelCI_app/urls.py | 7 +- .../views/treeCompareBuildsView.py | 74 +++++++++ backend/requests/tree-compare-builds-get.sh | 17 ++ 8 files changed, 560 insertions(+), 2 deletions(-) create mode 100644 backend/kernelCI_app/tests/unitTests/views/treeCompareBuildsView_test.py create mode 100644 backend/kernelCI_app/views/treeCompareBuildsView.py create mode 100644 backend/requests/tree-compare-builds-get.sh diff --git a/backend/kernelCI_app/helpers/treeCompare.py b/backend/kernelCI_app/helpers/treeCompare.py index 8835477ea..fda391f11 100644 --- a/backend/kernelCI_app/helpers/treeCompare.py +++ b/backend/kernelCI_app/helpers/treeCompare.py @@ -173,6 +173,161 @@ def build_boot_compare_filter_clauses( # noqa: C901 - maps FilterParams fields return ("\n".join(clauses), params) +def _build_status_filter_clause( + statuses: set[str], + params: dict, +) -> str: + status_list = list(statuses) + include_null = NULL_STATUS in status_list + concrete = [status.upper() for status in status_list if status != NULL_STATUS] + if include_null and concrete: + params["build_statuses"] = concrete + return ( + "AND (b.status IS NULL OR UPPER(b.status) = ANY(%(build_statuses)s))" + ) + if include_null: + return "AND b.status IS NULL" + params["build_statuses"] = concrete + return "AND UPPER(b.status) = ANY(%(build_statuses)s)" + + +def _build_issue_filter_clause( + issue_filters: set[tuple[str, Optional[int]]], + params: dict, +) -> str: + """Issue filters only apply to failing builds (FAIL/NULL), matching FilterParams.""" + known_issues = [ + (issue_id, issue_version) + for issue_id, issue_version in issue_filters + if issue_id != UNCATEGORIZED_STRING + ] + has_uncategorized = any( + issue_id == UNCATEGORIZED_STRING for issue_id, _ in issue_filters + ) + issue_parts: list[str] = [] + if known_issues: + issue_tuples = [] + for index, (issue_id, issue_version) in enumerate(known_issues): + id_key = f"build_issue_id_{index}" + version_key = f"build_issue_version_{index}" + params[id_key] = issue_id + params[version_key] = issue_version + if issue_version is None: + issue_tuples.append( + f"(i.issue_id = %({id_key})s AND i.issue_version IS NULL)" + ) + else: + issue_tuples.append( + f"(i.issue_id = %({id_key})s" + f" AND i.issue_version = %({version_key})s)" + ) + # Build-level incidents have test_id IS NULL + issue_parts.append( + f"""EXISTS ( + SELECT 1 FROM incidents i + WHERE i.build_id = b.id + AND i.test_id IS NULL + AND ({' OR '.join(issue_tuples)}) + )""" + ) + if has_uncategorized: + issue_parts.append( + """NOT EXISTS ( + SELECT 1 FROM incidents i + WHERE i.build_id = b.id + AND i.test_id IS NULL + AND i.issue_id IS NOT NULL + )""" + ) + if not issue_parts: + return "" + return ( + "AND (b.status IS NULL OR UPPER(b.status) = 'FAIL')\n" + f"AND ({' OR '.join(issue_parts)})" + ) + + +def build_build_compare_filter_clauses( # noqa: C901 - maps FilterParams fields to SQL AND clauses + filters: Optional["FilterParams"], +) -> tuple[str, dict]: + """Build optional WHERE fragments for build compare rows (applied before A/B join).""" + if filters is None: + return "", {} + + clauses: list[str] = [] + params: dict = {} + + if filters.filterBuildStatus: + clauses.append(_build_status_filter_clause(filters.filterBuildStatus, params)) + + if ( + filters.filterBuildDurationMin is not None + or filters.filterBuildDurationMax is not None + ): + clauses.append("AND b.duration IS NOT NULL") + if filters.filterBuildDurationMin is not None: + clauses.append("AND b.duration >= %(build_duration_min)s") + params["build_duration_min"] = filters.filterBuildDurationMin + if filters.filterBuildDurationMax is not None: + clauses.append("AND b.duration <= %(build_duration_max)s") + params["build_duration_max"] = filters.filterBuildDurationMax + + if filters.filter_build_origin: + clauses.append("AND b.origin = ANY(%(build_origins)s)") + params["build_origins"] = list(filters.filter_build_origin) + + if filters.filterConfigs: + clauses.append( + "AND COALESCE(b.config_name, %(unknown_string)s) = ANY(%(configs)s)" + ) + params["configs"] = list(filters.filterConfigs) + params["unknown_string"] = UNKNOWN_STRING + + if filters.filterArchitecture: + clauses.append( + "AND COALESCE(b.architecture, %(unknown_string)s) = ANY(%(architectures)s)" + ) + params["architectures"] = list(filters.filterArchitecture) + params["unknown_string"] = UNKNOWN_STRING + + if filters.filterCompiler: + clauses.append( + "AND COALESCE(b.compiler, %(unknown_string)s) = ANY(%(compilers)s)" + ) + params["compilers"] = list(filters.filterCompiler) + params["unknown_string"] = UNKNOWN_STRING + + if filters.filter_labs: + clauses.append( + "AND COALESCE(b.misc->>'lab', %(unknown_string)s) = ANY(%(labs)s)" + ) + params["labs"] = list(filters.filter_labs) + params["unknown_string"] = UNKNOWN_STRING + + if filters.filterHardware: + # Keep hardware filter as EXISTS so we don't multiply build rows by tests + clauses.append( + """ + AND EXISTS ( + SELECT 1 FROM tests ht + WHERE ht.build_id = b.id + AND ( + ht.environment_compatible && %(hardware)s::text[] + OR ht.environment_misc->>'platform' = ANY(%(hardware)s::text[]) + ) + ) + """ + ) + params["hardware"] = list(filters.filterHardware) + + if filters.filterIssues["build"]: + issue_clause = _build_issue_filter_clause(filters.filterIssues["build"], params) + if issue_clause: + clauses.append(issue_clause) + + return ("\n".join(clauses), params) + + @dataclass class _HashAccumulator: builds: CompareStatusCounts = field(default_factory=CompareStatusCounts) diff --git a/backend/kernelCI_app/queries/tree.py b/backend/kernelCI_app/queries/tree.py index d31f2cd41..99b747479 100644 --- a/backend/kernelCI_app/queries/tree.py +++ b/backend/kernelCI_app/queries/tree.py @@ -6,7 +6,10 @@ from kernelCI_app.cache import get_query_cache, set_query_cache from kernelCI_app.constants.general import UNKNOWN_STRING from kernelCI_app.helpers.database import dict_fetchall -from kernelCI_app.helpers.treeCompare import build_boot_compare_filter_clauses +from kernelCI_app.helpers.treeCompare import ( + build_boot_compare_filter_clauses, + build_build_compare_filter_clauses, +) from kernelCI_app.helpers.treeDetails import create_checkouts_where_clauses from kernelCI_app.models import Checkouts from kernelCI_app.queries.duration import ( @@ -1509,3 +1512,125 @@ def get_tree_compare_boots( return rows + +def get_tree_compare_builds_diff( + *, + hash_a: str, + hash_b: str, + origin_param: str, + git_branch_param: Optional[str], + tree_name: Optional[str] = None, + filters: Optional["FilterParams"] = None, +) -> list[dict]: + """Return build rows whose grouped status differs between two commits. + + Identity key: config_name + architecture + compiler. + Statuses are bucketed to PASS / FAIL / INCONCLUSIVE before compare so + SKIP vs MISS is not treated as a difference. + + Named *_diff to avoid colliding with get_tree_compare_builds (summary aggregates). + """ + commit_hashes = [hash_a, hash_b] + filter_clauses, filter_params = build_build_compare_filter_clauses(filters) + + cache_key = "treeCompareBuildsDiff" + params = { + "hash_a": hash_a, + "hash_b": hash_b, + "commit_hashes": commit_hashes, + "tree_name": tree_name, + "origin_param": origin_param, + "git_branch_param": git_branch_param, + "unknown_string": UNKNOWN_STRING, + **filter_params, + } + + rows = get_query_cache(cache_key, params) + if rows is not None: + return rows + + git_branch_clause, tree_name_full_clause = _get_compare_checkout_clauses( + git_branch_param=git_branch_param, + tree_name=tree_name, + ) + + query = f""" + WITH RELEVANT_CHECKOUTS AS ( + SELECT DISTINCT ON (c.git_commit_hash) + c.id AS checkout_id, + c.git_commit_hash + FROM + checkouts c + WHERE + c.git_commit_hash = ANY(%(commit_hashes)s) + {tree_name_full_clause} + AND {git_branch_clause} + AND c.origin = %(origin_param)s + ORDER BY + c.git_commit_hash, + c._timestamp DESC + ), + BUILD_ROWS AS ( + SELECT DISTINCT ON ( + rc.git_commit_hash, + COALESCE(b.config_name, %(unknown_string)s), + COALESCE(b.architecture, %(unknown_string)s), + COALESCE(b.compiler, %(unknown_string)s) + ) + rc.git_commit_hash, + COALESCE(b.config_name, %(unknown_string)s) AS config_name, + COALESCE(b.architecture, %(unknown_string)s) AS architecture, + COALESCE(b.compiler, %(unknown_string)s) AS compiler, + CASE + WHEN UPPER(b.status) = 'PASS' THEN 'PASS' + WHEN UPPER(b.status) = 'FAIL' THEN 'FAIL' + ELSE 'INCONCLUSIVE' + END AS grouped_status + FROM + RELEVANT_CHECKOUTS rc + INNER JOIN builds b ON b.checkout_id = rc.checkout_id + WHERE + b.id NOT LIKE 'maestro:dummy_%%' + {filter_clauses} + ORDER BY + rc.git_commit_hash, + COALESCE(b.config_name, %(unknown_string)s), + COALESCE(b.architecture, %(unknown_string)s), + COALESCE(b.compiler, %(unknown_string)s), + b.start_time DESC NULLS LAST, + b._timestamp DESC NULLS LAST + ), + SIDE_A AS ( + SELECT config_name, architecture, compiler, grouped_status + FROM BUILD_ROWS + WHERE git_commit_hash = %(hash_a)s + ), + SIDE_B AS ( + SELECT config_name, architecture, compiler, grouped_status + FROM BUILD_ROWS + WHERE git_commit_hash = %(hash_b)s + ) + SELECT + COALESCE(a.config_name, b.config_name) AS config_name, + COALESCE(a.architecture, b.architecture) AS architecture, + COALESCE(a.compiler, b.compiler) AS compiler, + a.grouped_status AS status_a, + b.grouped_status AS status_b + FROM + SIDE_A a + FULL OUTER JOIN SIDE_B b ON ( + a.config_name = b.config_name + AND a.architecture = b.architecture + AND a.compiler = b.compiler + ) + WHERE + a.grouped_status IS DISTINCT FROM b.grouped_status + """ + + with connection.cursor() as cursor: + cursor.execute(query, params) + rows = dict_fetchall(cursor=cursor) + set_query_cache(key=cache_key, params=params, rows=rows) + + return rows + diff --git a/backend/kernelCI_app/tests/unitTests/queries/treeCompareQueries_test.py b/backend/kernelCI_app/tests/unitTests/queries/treeCompareQueries_test.py index a7f4de6d2..a4df5e320 100644 --- a/backend/kernelCI_app/tests/unitTests/queries/treeCompareQueries_test.py +++ b/backend/kernelCI_app/tests/unitTests/queries/treeCompareQueries_test.py @@ -5,6 +5,7 @@ from kernelCI_app.queries.tree import ( get_tree_compare_boots, get_tree_compare_builds, + get_tree_compare_builds_diff, get_tree_compare_rollup, ) @@ -102,3 +103,40 @@ def test_compare_boots_joins_and_diffs_in_sql( self.assertNotIn("incidents", executed_query) self.assertNotIn("issues", executed_query) self.assertEqual(params["commit_hashes"], ["hash_a", "hash_b"]) + + @patch("kernelCI_app.queries.tree.connection") + @patch("kernelCI_app.queries.tree.get_query_cache", return_value=None) + @patch("kernelCI_app.queries.tree.set_query_cache") + @patch("kernelCI_app.queries.tree.dict_fetchall", return_value=[]) + def test_compare_builds_diff_joins_and_diffs_in_sql( + self, + mock_fetchall, + mock_set_cache, + mock_get_cache, + mock_connection, + ): + mock_cursor = MagicMock() + mock_connection.cursor.return_value.__enter__.return_value = mock_cursor + + get_tree_compare_builds_diff( + hash_a="hash_a", + hash_b="hash_b", + origin_param="maestro", + git_branch_param="master", + tree_name="linux", + ) + + executed_query = mock_cursor.execute.call_args[0][0] + params = mock_cursor.execute.call_args[0][1] + self.assertIn("git_commit_hash = ANY(%(commit_hashes)s)", executed_query) + self.assertIn("DISTINCT ON (c.git_commit_hash)", executed_query) + self.assertIn("FULL OUTER JOIN", executed_query) + self.assertIn("IS DISTINCT FROM", executed_query) + self.assertIn("WHEN UPPER(b.status) = 'PASS' THEN 'PASS'", executed_query) + self.assertIn("WHEN UPPER(b.status) = 'FAIL' THEN 'FAIL'", executed_query) + self.assertIn("ELSE 'INCONCLUSIVE'", executed_query) + self.assertIn("b.architecture", executed_query) + self.assertIn("b.compiler", executed_query) + self.assertNotIn("incidents", executed_query) + self.assertNotIn("FROM tests", executed_query) + self.assertEqual(params["commit_hashes"], ["hash_a", "hash_b"]) diff --git a/backend/kernelCI_app/tests/unitTests/views/treeCompareBuildsView_test.py b/backend/kernelCI_app/tests/unitTests/views/treeCompareBuildsView_test.py new file mode 100644 index 000000000..44ee4e823 --- /dev/null +++ b/backend/kernelCI_app/tests/unitTests/views/treeCompareBuildsView_test.py @@ -0,0 +1,132 @@ +from unittest.mock import MagicMock, patch + +from django.test import SimpleTestCase +from rest_framework.test import APIRequestFactory + +from kernelCI_app.helpers.treeCompare import build_build_compare_filter_clauses +from kernelCI_app.views.treeCompareBuildsView import TreeCompareBuildsView + +HASH_A = "a" * 40 +HASH_B = "b" * 40 + +BUILD_DIFF_ROW = { + "config_name": "defconfig", + "architecture": "arm64", + "compiler": "gcc-12", + "status_a": "PASS", + "status_b": "FAIL", +} + + +class TestBuildBuildCompareFilterClauses(SimpleTestCase): + def test_empty_filters_add_no_clauses(self): + clauses, params = build_build_compare_filter_clauses(None) + self.assertEqual(clauses, "") + self.assertEqual(params, {}) + + def test_build_status_and_config_filters(self): + filters = MagicMock() + filters.filterBuildStatus = {"FAIL", "NULL"} + filters.filterBuildDurationMin = None + filters.filterBuildDurationMax = None + filters.filter_build_origin = set() + filters.filterConfigs = {"defconfig"} + filters.filterArchitecture = set() + filters.filterCompiler = set() + filters.filter_labs = set() + filters.filterHardware = set() + filters.filterIssues = {"boot": set(), "build": set(), "test": set()} + + clauses, params = build_build_compare_filter_clauses(filters) + self.assertIn( + "b.status IS NULL OR UPPER(b.status) = ANY(%(build_statuses)s)", + clauses, + ) + self.assertIn("= ANY(%(configs)s)", clauses) + self.assertEqual(params["build_statuses"], ["FAIL"]) + self.assertEqual(params["configs"], ["defconfig"]) + + +class TestTreeCompareBuildsView(SimpleTestCase): + def setUp(self): + self.factory = APIRequestFactory() + self.view = TreeCompareBuildsView.as_view() + self.url = "/api/tree/linux/master/compare/builds" + + @patch("kernelCI_app.views.treeCompareBuildsView.get_tree_compare_builds_diff") + def test_get_returns_builds_payload(self, mock_builds): + mock_builds.return_value = [BUILD_DIFF_ROW] + + request = self.factory.get( + self.url, + { + "hash_a": HASH_A, + "hash_b": HASH_B, + "origin": "maestro", + }, + ) + response = self.view(request, tree_name="linux", git_branch="master") + + self.assertEqual(response.status_code, 200) + self.assertEqual(response.data["builds"], [BUILD_DIFF_ROW]) + self.assertIn("config_name", response.data["builds"][0]) + self.assertIn("architecture", response.data["builds"][0]) + self.assertIn("compiler", response.data["builds"][0]) + self.assertIn("status_a", response.data["builds"][0]) + self.assertNotIn("configName", response.data["builds"][0]) + mock_builds.assert_called_once() + self.assertEqual(mock_builds.call_args.kwargs["hash_a"], HASH_A) + + def test_missing_hashes_returns_400(self): + request = self.factory.get( + self.url, + { + "hash_a": HASH_A, + "origin": "maestro", + }, + ) + response = self.view(request, tree_name="linux", git_branch="master") + self.assertEqual(response.status_code, 400) + + @patch("kernelCI_app.views.treeCompareBuildsView.get_tree_compare_builds_diff") + def test_skip_vs_miss_same_bucket_yields_empty_diff(self, mock_builds): + mock_builds.return_value = [] + + request = self.factory.get( + self.url, + { + "hash_a": HASH_A, + "hash_b": HASH_B, + "origin": "maestro", + }, + ) + response = self.view(request, tree_name="linux", git_branch="master") + + self.assertEqual(response.status_code, 200) + self.assertEqual(response.data["builds"], []) + + @patch("kernelCI_app.views.treeCompareBuildsView.get_tree_compare_builds_diff") + def test_one_sided_null_status(self, mock_builds): + mock_builds.return_value = [ + { + "config_name": "defconfig", + "architecture": "arm64", + "compiler": "gcc-12", + "status_a": "PASS", + "status_b": None, + } + ] + + request = self.factory.get( + self.url, + { + "hash_a": HASH_A, + "hash_b": HASH_B, + "origin": "maestro", + }, + ) + response = self.view(request, tree_name="linux", git_branch="master") + + self.assertEqual(response.status_code, 200) + self.assertEqual(response.data["builds"][0]["status_a"], "PASS") + self.assertIsNone(response.data["builds"][0]["status_b"]) diff --git a/backend/kernelCI_app/typeModels/treeCompare.py b/backend/kernelCI_app/typeModels/treeCompare.py index e527646de..2e393bf9b 100644 --- a/backend/kernelCI_app/typeModels/treeCompare.py +++ b/backend/kernelCI_app/typeModels/treeCompare.py @@ -82,3 +82,15 @@ class CompareBootDiffRow(BaseModel): class TreeCompareBootsResponse(BaseModel): boots: list[CompareBootDiffRow] + + +class CompareBuildDiffRow(BaseModel): + config_name: str + architecture: str + compiler: str + status_a: Optional[CompareGroupedStatus] = None + status_b: Optional[CompareGroupedStatus] = None + + +class TreeCompareBuildsResponse(BaseModel): + builds: list[CompareBuildDiffRow] diff --git a/backend/kernelCI_app/urls.py b/backend/kernelCI_app/urls.py index 04c22469b..ca65d4e86 100644 --- a/backend/kernelCI_app/urls.py +++ b/backend/kernelCI_app/urls.py @@ -62,12 +62,17 @@ def view_cache(view, timeout: int = settings.CACHE_TIMEOUT): view_cache(views.TreeCommitsListView), name="treeCommitsList", ), - # Compare routes must come before ...//boots (else commit_hash="compare") + # Compare routes must come before ...//boots|builds (else commit_hash="compare") path( "tree///compare/boots", view_cache(views.TreeCompareBootsView), name="treeCompareBoots", ), + path( + "tree///compare/builds", + view_cache(views.TreeCompareBuildsView), + name="treeCompareBuilds", + ), path( "tree///compare", view_cache(views.TreeCompareView), diff --git a/backend/kernelCI_app/views/treeCompareBuildsView.py b/backend/kernelCI_app/views/treeCompareBuildsView.py new file mode 100644 index 000000000..516e314bc --- /dev/null +++ b/backend/kernelCI_app/views/treeCompareBuildsView.py @@ -0,0 +1,74 @@ +from http import HTTPStatus + +from django.http import HttpRequest +from drf_spectacular.utils import extend_schema +from pydantic import ValidationError +from rest_framework.response import Response +from rest_framework.views import APIView + +from kernelCI_app.constants.general import DEFAULT_ORIGIN +from kernelCI_app.helpers.errorHandling import create_api_error_response +from kernelCI_app.helpers.filters import FilterParams +from kernelCI_app.queries.tree import get_tree_compare_builds_diff +from kernelCI_app.typeModels.commonOpenApiParameters import ( + GIT_BRANCH_PATH_PARAM, + TREE_NAME_PATH_PARAM, +) +from kernelCI_app.typeModels.treeCompare import ( + TreeCompareBuildsResponse, + TreeCompareQueryParameters, +) + + +class TreeCompareBuildsView(APIView): + @extend_schema( + parameters=[ + TREE_NAME_PATH_PARAM, + GIT_BRANCH_PATH_PARAM, + TreeCompareQueryParameters, + ], + responses=TreeCompareBuildsResponse, + ) + def get( + self, + request: HttpRequest, + tree_name: str, + git_branch: str, + ) -> Response: + try: + query_params = TreeCompareQueryParameters( + hash_a=request.GET.get("hash_a", ""), + hash_b=request.GET.get("hash_b", ""), + origin=request.GET.get("origin", DEFAULT_ORIGIN), + ) + except ValidationError as error: + return Response(data=error.json(), status=HTTPStatus.BAD_REQUEST) + + if not query_params.hash_a or not query_params.hash_b: + return create_api_error_response( + status_code=HTTPStatus.BAD_REQUEST, + error_message="hash_a and hash_b are required", + ) + + filters = FilterParams(request) + rows = get_tree_compare_builds_diff( + hash_a=query_params.hash_a, + hash_b=query_params.hash_b, + origin_param=query_params.origin, + git_branch_param=git_branch, + tree_name=tree_name, + filters=filters, + ) + + try: + response = TreeCompareBuildsResponse(builds=rows) + except ValidationError as error: + return Response( + data=error.json(), + status=HTTPStatus.INTERNAL_SERVER_ERROR, + ) + + return Response( + data=response.model_dump(), + status=HTTPStatus.OK, + ) diff --git a/backend/requests/tree-compare-builds-get.sh b/backend/requests/tree-compare-builds-get.sh new file mode 100644 index 000000000..c00715119 --- /dev/null +++ b/backend/requests/tree-compare-builds-get.sh @@ -0,0 +1,17 @@ +http 'http://localhost:8000/api/tree/linux/master/compare/builds' \ + origin==maestro \ + hash_a==abc1234567890abcdef1234567890abcdef12 \ + hash_b==def5678901234abcdef5678901234abcdef56 + +# HTTP/1.1 200 OK +# { +# "builds": [ +# { +# "config_name": "defconfig", +# "architecture": "arm64", +# "compiler": "gcc-12", +# "status_a": "PASS", +# "status_b": "FAIL" +# } +# ] +# } From 245fdc10759c8c3094eb4dc7f68f53eb6690df24 Mon Sep 17 00:00:00 2001 From: Felipe Bergamin Date: Fri, 24 Jul 2026 19:03:54 -0300 Subject: [PATCH 8/8] feat: adjustments to ab summary screen - Layout changes - Counters for Regressions, fixed, still failing, new failures --- backend/kernelCI_app/helpers/treeCompare.py | 40 +- backend/kernelCI_app/queries/tree.py | 379 +++++++++++++++++- .../queries/treeCompareQueries_test.py | 44 ++ .../unitTests/views/treeCompareView_test.py | 73 +++- .../kernelCI_app/typeModels/treeCompare.py | 11 + backend/kernelCI_app/views/treeCompareView.py | 24 ++ dashboard/src/locales/messages/index.ts | 2 +- .../src/pages/TreeCompare/TreeComparePage.tsx | 40 +- .../TreeCompare/components/CompareSummary.tsx | 155 +++++-- .../components/RevisionSelector.tsx | 88 ++-- .../TreeCompare/mock/compareFailuresMock.ts | 10 +- dashboard/src/types/tree/TreeCompare.ts | 25 +- 12 files changed, 769 insertions(+), 122 deletions(-) diff --git a/backend/kernelCI_app/helpers/treeCompare.py b/backend/kernelCI_app/helpers/treeCompare.py index fda391f11..569a32294 100644 --- a/backend/kernelCI_app/helpers/treeCompare.py +++ b/backend/kernelCI_app/helpers/treeCompare.py @@ -6,6 +6,7 @@ from kernelCI_app.constants.process_pending import ROLLUP_STATUS_FIELDS from kernelCI_app.typeModels.databases import NULL_STATUS from kernelCI_app.typeModels.treeCompare import ( + CompareChangeCounts, CompareDelta, CompareEntitySummary, CompareGroupRow, @@ -431,12 +432,12 @@ def process_build_rows( for row in rows: commit_hash = row["git_commit_hash"] - config = row.get("config_name") or UNKNOWN_STRING + arch = row.get("architecture") or UNKNOWN_STRING count = row.get("count") or 0 acc = accumulators.setdefault(commit_hash, _HashAccumulator()) bucket = _status_to_bucket(row.get("status")) _increment_bucket(acc.builds, bucket, count) - _increment_bucket(acc.build_groups[config], bucket, count) + _increment_bucket(acc.build_groups[arch], bucket, count) return accumulators @@ -450,15 +451,30 @@ def _make_delta(side_a: CompareStatusCounts, side_b: CompareStatusCounts) -> Com ) +def change_counts_from_row(row: Optional[dict]) -> CompareChangeCounts: + """Map a SQL aggregate row (snake_case keys) into CompareChangeCounts.""" + if not row: + return CompareChangeCounts() + return CompareChangeCounts( + regression=row.get("regression") or 0, + fixed=row.get("fixed") or 0, + new_failure=row.get("new_failure") or 0, + still_failing=row.get("still_failing") or 0, + new_pass=row.get("new_pass") or 0, + ) + + def _make_entity_summary( *, side_a: CompareStatusCounts, side_b: CompareStatusCounts, + changes: Optional[CompareChangeCounts] = None, ) -> CompareEntitySummary: return CompareEntitySummary( sideA=side_a, sideB=side_b, delta=_make_delta(side_a, side_b), + changes=changes or CompareChangeCounts(), ) @@ -503,18 +519,32 @@ def build_compare_response( branch: str, git_url: str, accumulators: dict[str, _HashAccumulator], + changes: Optional[dict[str, CompareChangeCounts]] = None, ) -> TreeCompareResponse: acc_a = accumulators.get(hash_a, _HashAccumulator()) acc_b = accumulators.get(hash_b, _HashAccumulator()) + change_map = changes or {} return TreeCompareResponse( treeName=tree_name, branch=branch, gitUrl=git_url, summary=CompareSummary( - builds=_make_entity_summary(side_a=acc_a.builds, side_b=acc_b.builds), - boots=_make_entity_summary(side_a=acc_a.boots, side_b=acc_b.boots), - tests=_make_entity_summary(side_a=acc_a.tests, side_b=acc_b.tests), + builds=_make_entity_summary( + side_a=acc_a.builds, + side_b=acc_b.builds, + changes=change_map.get("builds"), + ), + boots=_make_entity_summary( + side_a=acc_a.boots, + side_b=acc_b.boots, + changes=change_map.get("boots"), + ), + tests=_make_entity_summary( + side_a=acc_a.tests, + side_b=acc_b.tests, + changes=change_map.get("tests"), + ), ), groups=CompareGroups( builds=_make_group_rows( diff --git a/backend/kernelCI_app/queries/tree.py b/backend/kernelCI_app/queries/tree.py index 99b747479..c0eb9dd5f 100644 --- a/backend/kernelCI_app/queries/tree.py +++ b/backend/kernelCI_app/queries/tree.py @@ -1364,19 +1364,19 @@ def get_tree_compare_builds( SELECT rc.git_commit_hash, rc.git_repository_url, - b.config_name, + b.architecture, b.status, COUNT(DISTINCT b.id) AS count FROM RELEVANT_CHECKOUTS rc INNER JOIN builds b ON b.checkout_id = rc.checkout_id WHERE - b.config_name IS NOT NULL + b.architecture IS NOT NULL AND b.id NOT LIKE 'maestro:dummy_%%' GROUP BY rc.git_commit_hash, rc.git_repository_url, - b.config_name, + b.architecture, b.status """ @@ -1634,3 +1634,376 @@ def get_tree_compare_builds_diff( return rows + +_CHANGE_COUNT_SELECT = """ + COUNT(*) FILTER ( + WHERE status_a = 'PASS' AND status_b = 'FAIL' + ) AS regression, + COUNT(*) FILTER ( + WHERE status_a = 'FAIL' AND status_b = 'PASS' + ) AS fixed, + COUNT(*) FILTER ( + WHERE status_a IS NULL AND status_b = 'FAIL' + ) AS new_failure, + COUNT(*) FILTER ( + WHERE status_a = 'FAIL' AND status_b = 'FAIL' + ) AS still_failing, + COUNT(*) FILTER ( + WHERE status_a IS NULL AND status_b = 'PASS' + ) AS new_pass +""" + + +def _empty_change_counts() -> dict: + return { + "regression": 0, + "fixed": 0, + "new_failure": 0, + "still_failing": 0, + "new_pass": 0, + } + + +def get_tree_compare_builds_change_counts( + *, + hash_a: str, + hash_b: str, + origin_param: str, + git_branch_param: Optional[str], + tree_name: Optional[str] = None, +) -> dict: + """Aggregate build A/B change categories (including still-failing).""" + commit_hashes = [hash_a, hash_b] + cache_key = "treeCompareBuildsChangeCounts" + params = { + "hash_a": hash_a, + "hash_b": hash_b, + "commit_hashes": commit_hashes, + "tree_name": tree_name, + "origin_param": origin_param, + "git_branch_param": git_branch_param, + "unknown_string": UNKNOWN_STRING, + } + + cached = get_query_cache(cache_key, params) + if cached is not None: + return cached[0] if cached else _empty_change_counts() + + git_branch_clause, tree_name_full_clause = _get_compare_checkout_clauses( + git_branch_param=git_branch_param, + tree_name=tree_name, + ) + + query = f""" + WITH RELEVANT_CHECKOUTS AS ( + SELECT DISTINCT ON (c.git_commit_hash) + c.id AS checkout_id, + c.git_commit_hash + FROM + checkouts c + WHERE + c.git_commit_hash = ANY(%(commit_hashes)s) + {tree_name_full_clause} + AND {git_branch_clause} + AND c.origin = %(origin_param)s + ORDER BY + c.git_commit_hash, + c._timestamp DESC + ), + BUILD_ROWS AS ( + SELECT DISTINCT ON ( + rc.git_commit_hash, + COALESCE(b.config_name, %(unknown_string)s), + COALESCE(b.architecture, %(unknown_string)s), + COALESCE(b.compiler, %(unknown_string)s) + ) + rc.git_commit_hash, + COALESCE(b.config_name, %(unknown_string)s) AS config_name, + COALESCE(b.architecture, %(unknown_string)s) AS architecture, + COALESCE(b.compiler, %(unknown_string)s) AS compiler, + CASE + WHEN UPPER(b.status) = 'PASS' THEN 'PASS' + WHEN UPPER(b.status) = 'FAIL' THEN 'FAIL' + ELSE 'INCONCLUSIVE' + END AS grouped_status + FROM + RELEVANT_CHECKOUTS rc + INNER JOIN builds b ON b.checkout_id = rc.checkout_id + WHERE + b.id NOT LIKE 'maestro:dummy_%%' + ORDER BY + rc.git_commit_hash, + COALESCE(b.config_name, %(unknown_string)s), + COALESCE(b.architecture, %(unknown_string)s), + COALESCE(b.compiler, %(unknown_string)s), + b.start_time DESC NULLS LAST, + b._timestamp DESC NULLS LAST + ), + SIDE_A AS ( + SELECT config_name, architecture, compiler, grouped_status + FROM BUILD_ROWS + WHERE git_commit_hash = %(hash_a)s + ), + SIDE_B AS ( + SELECT config_name, architecture, compiler, grouped_status + FROM BUILD_ROWS + WHERE git_commit_hash = %(hash_b)s + ), + PAIRED AS ( + SELECT + a.grouped_status AS status_a, + b.grouped_status AS status_b + FROM + SIDE_A a + FULL OUTER JOIN SIDE_B b ON ( + a.config_name = b.config_name + AND a.architecture = b.architecture + AND a.compiler = b.compiler + ) + ) + SELECT + {_CHANGE_COUNT_SELECT} + FROM + PAIRED + """ + + with connection.cursor() as cursor: + cursor.execute(query, params) + rows = dict_fetchall(cursor=cursor) + set_query_cache(key=cache_key, params=params, rows=rows) + + return rows[0] if rows else _empty_change_counts() + + +def get_tree_compare_boots_change_counts( + *, + hash_a: str, + hash_b: str, + origin_param: str, + git_branch_param: Optional[str], + tree_name: Optional[str] = None, +) -> dict: + """Aggregate boot A/B change categories (including still-failing).""" + commit_hashes = [hash_a, hash_b] + cache_key = "treeCompareBootsChangeCounts" + params = { + "hash_a": hash_a, + "hash_b": hash_b, + "commit_hashes": commit_hashes, + "tree_name": tree_name, + "origin_param": origin_param, + "git_branch_param": git_branch_param, + "unknown_string": UNKNOWN_STRING, + } + + cached = get_query_cache(cache_key, params) + if cached is not None: + return cached[0] if cached else _empty_change_counts() + + git_branch_clause, tree_name_full_clause = _get_compare_checkout_clauses( + git_branch_param=git_branch_param, + tree_name=tree_name, + ) + + query = f""" + WITH RELEVANT_CHECKOUTS AS ( + SELECT DISTINCT ON (c.git_commit_hash) + c.id AS checkout_id, + c.git_commit_hash + FROM + checkouts c + WHERE + c.git_commit_hash = ANY(%(commit_hashes)s) + {tree_name_full_clause} + AND {git_branch_clause} + AND c.origin = %(origin_param)s + ORDER BY + c.git_commit_hash, + c._timestamp DESC + ), + BOOT_ROWS AS ( + SELECT DISTINCT ON ( + rc.git_commit_hash, + t.path, + COALESCE(b.config_name, %(unknown_string)s), + COALESCE(t.environment_misc->>'platform', %(unknown_string)s) + ) + rc.git_commit_hash, + t.path, + COALESCE(b.config_name, %(unknown_string)s) AS config_name, + COALESCE( + t.environment_misc->>'platform', + %(unknown_string)s + ) AS platform, + CASE + WHEN UPPER(t.status) = 'PASS' THEN 'PASS' + WHEN UPPER(t.status) = 'FAIL' THEN 'FAIL' + ELSE 'INCONCLUSIVE' + END AS grouped_status + FROM + RELEVANT_CHECKOUTS rc + INNER JOIN builds b ON b.checkout_id = rc.checkout_id + INNER JOIN tests t ON t.build_id = b.id + AND (t.path = 'boot' OR t.path LIKE 'boot.%%') + WHERE + b.id NOT LIKE 'maestro:dummy_%%' + ORDER BY + rc.git_commit_hash, + t.path, + COALESCE(b.config_name, %(unknown_string)s), + COALESCE(t.environment_misc->>'platform', %(unknown_string)s), + t.start_time DESC NULLS LAST, + t._timestamp DESC NULLS LAST + ), + SIDE_A AS ( + SELECT path, config_name, platform, grouped_status + FROM BOOT_ROWS + WHERE git_commit_hash = %(hash_a)s + ), + SIDE_B AS ( + SELECT path, config_name, platform, grouped_status + FROM BOOT_ROWS + WHERE git_commit_hash = %(hash_b)s + ), + PAIRED AS ( + SELECT + a.grouped_status AS status_a, + b.grouped_status AS status_b + FROM + SIDE_A a + FULL OUTER JOIN SIDE_B b ON ( + a.path = b.path + AND a.config_name = b.config_name + AND a.platform = b.platform + ) + ) + SELECT + {_CHANGE_COUNT_SELECT} + FROM + PAIRED + """ + + with connection.cursor() as cursor: + cursor.execute(query, params) + rows = dict_fetchall(cursor=cursor) + set_query_cache(key=cache_key, params=params, rows=rows) + + return rows[0] if rows else _empty_change_counts() + + +def get_tree_compare_tests_change_counts( + *, + hash_a: str, + hash_b: str, + origin_param: str, + git_branch_param: Optional[str], + tree_name: Optional[str] = None, +) -> dict: + """Aggregate non-boot test A/B change categories (including still-failing).""" + commit_hashes = [hash_a, hash_b] + cache_key = "treeCompareTestsChangeCounts" + params = { + "hash_a": hash_a, + "hash_b": hash_b, + "commit_hashes": commit_hashes, + "tree_name": tree_name, + "origin_param": origin_param, + "git_branch_param": git_branch_param, + "unknown_string": UNKNOWN_STRING, + } + + cached = get_query_cache(cache_key, params) + if cached is not None: + return cached[0] if cached else _empty_change_counts() + + git_branch_clause, tree_name_full_clause = _get_compare_checkout_clauses( + git_branch_param=git_branch_param, + tree_name=tree_name, + ) + + query = f""" + WITH RELEVANT_CHECKOUTS AS ( + SELECT DISTINCT ON (c.git_commit_hash) + c.id AS checkout_id, + c.git_commit_hash + FROM + checkouts c + WHERE + c.git_commit_hash = ANY(%(commit_hashes)s) + {tree_name_full_clause} + AND {git_branch_clause} + AND c.origin = %(origin_param)s + ORDER BY + c.git_commit_hash, + c._timestamp DESC + ), + TEST_ROWS AS ( + SELECT DISTINCT ON ( + rc.git_commit_hash, + t.path, + COALESCE(b.config_name, %(unknown_string)s), + COALESCE(t.environment_misc->>'platform', %(unknown_string)s) + ) + rc.git_commit_hash, + t.path, + COALESCE(b.config_name, %(unknown_string)s) AS config_name, + COALESCE( + t.environment_misc->>'platform', + %(unknown_string)s + ) AS platform, + CASE + WHEN UPPER(t.status) = 'PASS' THEN 'PASS' + WHEN UPPER(t.status) = 'FAIL' THEN 'FAIL' + ELSE 'INCONCLUSIVE' + END AS grouped_status + FROM + RELEVANT_CHECKOUTS rc + INNER JOIN builds b ON b.checkout_id = rc.checkout_id + INNER JOIN tests t ON t.build_id = b.id + AND t.path IS DISTINCT FROM 'boot' + AND t.path NOT LIKE 'boot.%%' + WHERE + b.id NOT LIKE 'maestro:dummy_%%' + ORDER BY + rc.git_commit_hash, + t.path, + COALESCE(b.config_name, %(unknown_string)s), + COALESCE(t.environment_misc->>'platform', %(unknown_string)s), + t.start_time DESC NULLS LAST, + t._timestamp DESC NULLS LAST + ), + SIDE_A AS ( + SELECT path, config_name, platform, grouped_status + FROM TEST_ROWS + WHERE git_commit_hash = %(hash_a)s + ), + SIDE_B AS ( + SELECT path, config_name, platform, grouped_status + FROM TEST_ROWS + WHERE git_commit_hash = %(hash_b)s + ), + PAIRED AS ( + SELECT + a.grouped_status AS status_a, + b.grouped_status AS status_b + FROM + SIDE_A a + FULL OUTER JOIN SIDE_B b ON ( + a.path = b.path + AND a.config_name = b.config_name + AND a.platform = b.platform + ) + ) + SELECT + {_CHANGE_COUNT_SELECT} + FROM + PAIRED + """ + + with connection.cursor() as cursor: + cursor.execute(query, params) + rows = dict_fetchall(cursor=cursor) + set_query_cache(key=cache_key, params=params, rows=rows) + + return rows[0] if rows else _empty_change_counts() + diff --git a/backend/kernelCI_app/tests/unitTests/queries/treeCompareQueries_test.py b/backend/kernelCI_app/tests/unitTests/queries/treeCompareQueries_test.py index a4df5e320..accfb4727 100644 --- a/backend/kernelCI_app/tests/unitTests/queries/treeCompareQueries_test.py +++ b/backend/kernelCI_app/tests/unitTests/queries/treeCompareQueries_test.py @@ -5,6 +5,7 @@ from kernelCI_app.queries.tree import ( get_tree_compare_boots, get_tree_compare_builds, + get_tree_compare_builds_change_counts, get_tree_compare_builds_diff, get_tree_compare_rollup, ) @@ -65,6 +66,8 @@ def test_compare_builds_aggregates_in_sql( executed_query = mock_cursor.execute.call_args[0][0] self.assertIn("COUNT(DISTINCT b.id)", executed_query) self.assertIn("GROUP BY", executed_query) + self.assertIn("b.architecture", executed_query) + self.assertNotIn("b.config_name", executed_query) self.assertNotIn("known_issues", executed_query) self.assertNotIn("SELECT\n b.id AS build_id", executed_query) @@ -140,3 +143,44 @@ def test_compare_builds_diff_joins_and_diffs_in_sql( self.assertNotIn("incidents", executed_query) self.assertNotIn("FROM tests", executed_query) self.assertEqual(params["commit_hashes"], ["hash_a", "hash_b"]) + + @patch("kernelCI_app.queries.tree.connection") + @patch("kernelCI_app.queries.tree.get_query_cache", return_value=None) + @patch("kernelCI_app.queries.tree.set_query_cache") + @patch( + "kernelCI_app.queries.tree.dict_fetchall", + return_value=[ + { + "regression": 1, + "fixed": 0, + "new_failure": 2, + "still_failing": 3, + "new_pass": 4, + } + ], + ) + def test_compare_builds_change_counts_aggregates_categories( + self, + mock_fetchall, + mock_set_cache, + mock_get_cache, + mock_connection, + ): + mock_cursor = MagicMock() + mock_connection.cursor.return_value.__enter__.return_value = mock_cursor + + result = get_tree_compare_builds_change_counts( + hash_a="hash_a", + hash_b="hash_b", + origin_param="maestro", + git_branch_param="master", + tree_name="linux", + ) + + executed_query = mock_cursor.execute.call_args[0][0] + self.assertIn("COUNT(*) FILTER", executed_query) + self.assertIn("still_failing", executed_query) + self.assertIn("FULL OUTER JOIN", executed_query) + self.assertNotIn("IS DISTINCT FROM", executed_query) + self.assertEqual(result["regression"], 1) + self.assertEqual(result["still_failing"], 3) diff --git a/backend/kernelCI_app/tests/unitTests/views/treeCompareView_test.py b/backend/kernelCI_app/tests/unitTests/views/treeCompareView_test.py index b188375c2..7e16e4f16 100644 --- a/backend/kernelCI_app/tests/unitTests/views/treeCompareView_test.py +++ b/backend/kernelCI_app/tests/unitTests/views/treeCompareView_test.py @@ -5,9 +5,11 @@ from kernelCI_app.helpers.treeCompare import ( build_compare_response, + change_counts_from_row, process_build_rows, process_rollup_rows, ) +from kernelCI_app.typeModels.treeCompare import CompareChangeCounts from kernelCI_app.views.treeCompareView import TreeCompareView HASH_A = "a" * 40 @@ -53,7 +55,7 @@ BUILD_ROW_A = { "git_commit_hash": HASH_A, "git_repository_url": "https://git.kernel.org/linux.git", - "config_name": "arm64-defconfig", + "architecture": "arm64", "status": "PASS", "count": 3, } @@ -82,7 +84,7 @@ def test_process_rollup_rows_aggregates_summary_and_groups(self): self.assertIn("qemu-arm64", acc.boot_groups) self.assertIn("kselftest/x86_64", acc.test_groups) - def test_process_build_rows_groups_by_config(self): + def test_process_build_rows_groups_by_architecture(self): accumulators = process_build_rows( rows=[BUILD_ROW_A, BUILD_ROW_B], commit_hashes=[HASH_A, HASH_B], @@ -91,7 +93,7 @@ def test_process_build_rows_groups_by_config(self): self.assertEqual(accumulators[HASH_A].builds.pass_count, 3) self.assertEqual(accumulators[HASH_B].builds.fail_count, 2) self.assertEqual( - accumulators[HASH_A].build_groups["arm64-defconfig"].pass_count, 3 + accumulators[HASH_A].build_groups["arm64"].pass_count, 3 ) def test_build_compare_response_computes_deltas(self): @@ -120,6 +122,11 @@ def test_build_compare_response_computes_deltas(self): branch="master", git_url="https://git.kernel.org/linux.git", accumulators=accumulators, + changes={ + "builds": CompareChangeCounts(regression=1), + "boots": CompareChangeCounts(), + "tests": CompareChangeCounts(new_failure=2, still_failing=3), + }, ) payload = response.model_dump(by_alias=True) @@ -127,6 +134,23 @@ def test_build_compare_response_computes_deltas(self): self.assertEqual(payload["summary"]["tests"]["delta"]["fail"], 2) self.assertEqual(payload["summary"]["builds"]["delta"]["pass"], -3) self.assertEqual(payload["summary"]["builds"]["delta"]["fail"], 2) + self.assertEqual(payload["summary"]["builds"]["changes"]["regression"], 1) + self.assertEqual(payload["summary"]["tests"]["changes"]["newFailure"], 2) + self.assertEqual(payload["summary"]["tests"]["changes"]["stillFailing"], 3) + + def test_change_counts_from_row(self): + counts = change_counts_from_row( + { + "regression": 4, + "fixed": 1, + "new_failure": 2, + "still_failing": 7, + "new_pass": 9, + } + ) + self.assertEqual(counts.regression, 4) + self.assertEqual(counts.new_failure, 2) + self.assertEqual(counts.model_dump(by_alias=True)["stillFailing"], 7) class TestTreeCompareView(SimpleTestCase): @@ -135,15 +159,28 @@ def setUp(self): self.view = TreeCompareView.as_view() self.url = "/api/tree/linux/master/compare" + @patch("kernelCI_app.views.treeCompareView.get_tree_compare_tests_change_counts") + @patch("kernelCI_app.views.treeCompareView.get_tree_compare_boots_change_counts") + @patch("kernelCI_app.views.treeCompareView.get_tree_compare_builds_change_counts") @patch("kernelCI_app.views.treeCompareView.get_tree_compare_rollup") @patch("kernelCI_app.views.treeCompareView.get_tree_compare_builds") - def test_get_returns_compare_payload(self, mock_builds, mock_rollup): + def test_get_returns_compare_payload( + self, + mock_builds, + mock_rollup, + mock_build_changes, + mock_boot_changes, + mock_test_changes, + ): mock_builds.return_value = [BUILD_ROW_A, BUILD_ROW_B] mock_rollup.return_value = [ ROLLUP_BOOT_ROW, ROLLUP_TEST_ROW_A, ROLLUP_TEST_ROW_B, ] + mock_build_changes.return_value = {"regression": 1, "fixed": 0, "new_failure": 0, "still_failing": 0, "new_pass": 0} + mock_boot_changes.return_value = {"regression": 0, "fixed": 0, "new_failure": 0, "still_failing": 0, "new_pass": 0} + mock_test_changes.return_value = {"regression": 2, "fixed": 1, "new_failure": 3, "still_failing": 4, "new_pass": 5} request = self.factory.get( self.url, @@ -160,8 +197,13 @@ def test_get_returns_compare_payload(self, mock_builds, mock_rollup): self.assertEqual(response.data["branch"], "master") self.assertIn("summary", response.data) self.assertIn("groups", response.data) + self.assertEqual(response.data["summary"]["builds"]["changes"]["regression"], 1) + self.assertEqual(response.data["summary"]["tests"]["changes"]["stillFailing"], 4) mock_builds.assert_called_once() mock_rollup.assert_called_once() + mock_build_changes.assert_called_once() + mock_boot_changes.assert_called_once() + mock_test_changes.assert_called_once() self.assertNotIn("git_url_param", mock_builds.call_args.kwargs) def test_missing_hashes_returns_400(self): @@ -175,11 +217,31 @@ def test_missing_hashes_returns_400(self): response = self.view(request, tree_name="linux", git_branch="master") self.assertEqual(response.status_code, 400) + @patch("kernelCI_app.views.treeCompareView.get_tree_compare_tests_change_counts") + @patch("kernelCI_app.views.treeCompareView.get_tree_compare_boots_change_counts") + @patch("kernelCI_app.views.treeCompareView.get_tree_compare_builds_change_counts") @patch("kernelCI_app.views.treeCompareView.get_tree_compare_rollup") @patch("kernelCI_app.views.treeCompareView.get_tree_compare_builds") - def test_missing_checkout_returns_zero_counts(self, mock_builds, mock_rollup): + def test_missing_checkout_returns_zero_counts( + self, + mock_builds, + mock_rollup, + mock_build_changes, + mock_boot_changes, + mock_test_changes, + ): mock_builds.return_value = [] mock_rollup.return_value = [] + empty = { + "regression": 0, + "fixed": 0, + "new_failure": 0, + "still_failing": 0, + "new_pass": 0, + } + mock_build_changes.return_value = empty + mock_boot_changes.return_value = empty + mock_test_changes.return_value = empty request = self.factory.get( self.url, @@ -194,3 +256,4 @@ def test_missing_checkout_returns_zero_counts(self, mock_builds, mock_rollup): self.assertEqual(response.status_code, 200) self.assertEqual(response.data["summary"]["builds"]["sideA"]["pass"], 0) self.assertEqual(response.data["summary"]["tests"]["sideB"]["fail"], 0) + self.assertEqual(response.data["summary"]["builds"]["changes"]["regression"], 0) diff --git a/backend/kernelCI_app/typeModels/treeCompare.py b/backend/kernelCI_app/typeModels/treeCompare.py index 2e393bf9b..341acfe17 100644 --- a/backend/kernelCI_app/typeModels/treeCompare.py +++ b/backend/kernelCI_app/typeModels/treeCompare.py @@ -23,12 +23,23 @@ class CompareDelta(BaseModel): fail_count: int = Field(alias="fail", default=0) +class CompareChangeCounts(BaseModel): + model_config = ConfigDict(populate_by_name=True) + + regression: int = 0 + fixed: int = 0 + new_failure: int = Field(alias="newFailure", default=0) + still_failing: int = Field(alias="stillFailing", default=0) + new_pass: int = Field(alias="newPass", default=0) + + class CompareEntitySummary(BaseModel): model_config = ConfigDict(populate_by_name=True) side_a: CompareStatusCounts = Field(alias="sideA") side_b: CompareStatusCounts = Field(alias="sideB") delta: CompareDelta + changes: CompareChangeCounts = Field(default_factory=CompareChangeCounts) class CompareGroupRow(BaseModel): diff --git a/backend/kernelCI_app/views/treeCompareView.py b/backend/kernelCI_app/views/treeCompareView.py index 533695029..c33fd8f39 100644 --- a/backend/kernelCI_app/views/treeCompareView.py +++ b/backend/kernelCI_app/views/treeCompareView.py @@ -11,12 +11,16 @@ from kernelCI_app.helpers.treeCompare import ( _HashAccumulator, build_compare_response, + change_counts_from_row, process_build_rows, process_rollup_rows, ) from kernelCI_app.queries.tree import ( + get_tree_compare_boots_change_counts, get_tree_compare_builds, + get_tree_compare_builds_change_counts, get_tree_compare_rollup, + get_tree_compare_tests_change_counts, ) from kernelCI_app.typeModels.commonOpenApiParameters import ( GIT_BRANCH_PATH_PARAM, @@ -84,6 +88,13 @@ def get( "git_branch_param": git_branch, "tree_name": tree_name, } + change_kwargs = { + "hash_a": query_params.hash_a, + "hash_b": query_params.hash_b, + "origin_param": query_params.origin, + "git_branch_param": git_branch, + "tree_name": tree_name, + } build_rows = get_tree_compare_builds(**query_kwargs) builds_data = process_build_rows( @@ -104,6 +115,18 @@ def get( boots_tests_data=boots_tests_data, ) + changes = { + "builds": change_counts_from_row( + get_tree_compare_builds_change_counts(**change_kwargs) + ), + "boots": change_counts_from_row( + get_tree_compare_boots_change_counts(**change_kwargs) + ), + "tests": change_counts_from_row( + get_tree_compare_tests_change_counts(**change_kwargs) + ), + } + git_url = "" if build_rows: git_url = build_rows[0].get("git_repository_url") or "" @@ -115,6 +138,7 @@ def get( branch=git_branch, git_url=git_url, accumulators=accumulators, + changes=changes, ) return Response( diff --git a/dashboard/src/locales/messages/index.ts b/dashboard/src/locales/messages/index.ts index 618f38ca1..b1ae6ee9d 100644 --- a/dashboard/src/locales/messages/index.ts +++ b/dashboard/src/locales/messages/index.ts @@ -411,7 +411,7 @@ export const messages = { 'treeCompare.failures.testsTitle': 'Test failures', 'treeCompare.detail.viewFullLog': 'View full log', 'treeCompare.group.boots': 'Platform', - 'treeCompare.group.builds': 'Config', + 'treeCompare.group.builds': 'Architecture', 'treeCompare.group.tests': 'Path', 'treeCompare.openCompare': 'Compare revisions', 'treeCompare.selectRevision': 'Select a revision', diff --git a/dashboard/src/pages/TreeCompare/TreeComparePage.tsx b/dashboard/src/pages/TreeCompare/TreeComparePage.tsx index a6e21392f..9aeb5060c 100644 --- a/dashboard/src/pages/TreeCompare/TreeComparePage.tsx +++ b/dashboard/src/pages/TreeCompare/TreeComparePage.tsx @@ -105,30 +105,35 @@ const TreeComparePage = (): JSX.Element => { } }, [hashA, hashB, resolvedHashA, resolvedHashB, updateSearch]); - const handleSuggestion = useCallback( - (suggestion: 'previous' | 'branchHead' | 'swap') => { - if (suggestion === 'swap') { - updateSearch({ hashA: resolvedHashB, hashB: resolvedHashA }); - return; - } + const handleSwap = useCallback(() => { + updateSearch({ hashA: resolvedHashB, hashB: resolvedHashA }); + }, [resolvedHashA, resolvedHashB, updateSearch]); + + const handleSideAction = useCallback( + (side: 'A' | 'B', action: 'previous' | 'branchHead') => { + const currentHash = side === 'A' ? resolvedHashA : resolvedHashB; + const currentIndex = revisions.findIndex(r => r.hash === currentHash); - if (suggestion === 'previous') { - const currentIndex = revisions.findIndex(r => r.hash === resolvedHashB); + if (action === 'previous') { const previousIndex = Math.min( revisions.length - 1, Math.max(currentIndex, 0) + 1, ); - updateSearch({ - hashA: resolvedHashB, - hashB: revisions[previousIndex]?.hash ?? resolvedHashA, - }); + const nextHash = revisions[previousIndex]?.hash ?? currentHash; + if (side === 'A') { + updateSearch({ hashA: nextHash }); + } else { + updateSearch({ hashB: nextHash }); + } return; } - updateSearch({ - hashA: resolvedHashB, - hashB: revisions[0]?.hash ?? resolvedHashA, - }); + const headHash = revisions[0]?.hash ?? currentHash; + if (side === 'A') { + updateSearch({ hashA: headHash }); + } else { + updateSearch({ hashB: headHash }); + } }, [revisions, resolvedHashA, resolvedHashB, updateSearch], ); @@ -238,7 +243,8 @@ const TreeComparePage = (): JSX.Element => { revisions={revisions} onHashAChange={value => updateSearch({ hashA: value })} onHashBChange={value => updateSearch({ hashB: value })} - onSuggestion={handleSuggestion} + onSideAction={handleSideAction} + onSwap={handleSwap} /> diff --git a/dashboard/src/pages/TreeCompare/components/CompareSummary.tsx b/dashboard/src/pages/TreeCompare/components/CompareSummary.tsx index 84c403cc1..1b23cbe08 100644 --- a/dashboard/src/pages/TreeCompare/components/CompareSummary.tsx +++ b/dashboard/src/pages/TreeCompare/components/CompareSummary.tsx @@ -3,14 +3,71 @@ import type { JSX } from 'react'; import { Link } from '@tanstack/react-router'; import { FormattedMessage } from 'react-intl'; -import type { CompareEntitySummary } from '@/types/tree/TreeCompare'; +import type { + CompareChangeStats, + CompareEntitySummary, + CompareStatusCounts, +} from '@/types/tree/TreeCompare'; import type { PossibleTabs } from '@/types/tree/TreeDetails'; -import { - CountsWithLabel, - DeltaPair, - StatusCountsDisplay, -} from './CompareStatusDisplay'; +import type { MessagesKey } from '@/locales/messages'; + +import { cn } from '@/lib/utils'; + +const SUMMARY_METRICS: { + key: keyof Pick< + CompareChangeStats, + 'regression' | 'fixed' | 'newFailure' | 'stillFailing' + >; + labelId: MessagesKey; + activeClass: string; +}[] = [ + { + key: 'regression', + labelId: 'treeCompare.change.regressions', + activeClass: 'text-red', + }, + { + key: 'fixed', + labelId: 'treeCompare.change.fixed', + activeClass: 'text-dark-green', + }, + { + key: 'newFailure', + labelId: 'treeCompare.change.newFailures', + activeClass: 'text-orange-700', + }, + { + key: 'stillFailing', + labelId: 'treeCompare.change.stillFailing', + activeClass: 'text-dim-gray', + }, +]; + +function DotCounts({ counts }: { counts: CompareStatusCounts }): JSX.Element { + return ( + + + + + {counts.pass.toLocaleString()} + + + + + + {counts.fail.toLocaleString()} + + + + + + {counts.inconclusive.toLocaleString()} + + + + ); +} interface CompareSummaryCardProps { titleId: 'global.builds' | 'global.boots' | 'global.tests'; @@ -33,41 +90,20 @@ function CompareSummaryCard({ origin, tab, }: CompareSummaryCardProps): JSX.Element { - const hasChange = summary.delta.pass !== 0 || summary.delta.fail !== 0; + const { changes } = summary; + const hasFailures = + changes.regression > 0 || + changes.newFailure > 0 || + changes.stillFailing > 0 || + changes.fixed > 0; return ( -
-

- -

- -
- - -
- - - - -
-
- -
-
- - : - - - → - - : - - -
- {hasChange ? ( +
+
+

+ +

+ {hasFailures ? ( s} > ) : null}
+ +
+ {SUMMARY_METRICS.map(metric => { + const value = changes[metric.key]; + return ( +
+ 0 ? metric.activeClass : 'text-dim-gray', + )} + > + {value.toLocaleString()} + + + + +
+ ); + })} +
+ +
+
+ + A + + +
+ → +
+ + B + + +
+
); } @@ -112,7 +185,7 @@ export function CompareSummary({ }: CompareSummaryProps): JSX.Element { return (
-

+

diff --git a/dashboard/src/pages/TreeCompare/components/RevisionSelector.tsx b/dashboard/src/pages/TreeCompare/components/RevisionSelector.tsx index b5d938719..89292cee1 100644 --- a/dashboard/src/pages/TreeCompare/components/RevisionSelector.tsx +++ b/dashboard/src/pages/TreeCompare/components/RevisionSelector.tsx @@ -1,6 +1,6 @@ import type { JSX } from 'react'; -import { ArrowLeftRight } from 'lucide-react'; +import { ArrowLeftRight, GitBranch, History } from 'lucide-react'; import { FormattedMessage } from 'react-intl'; import { Button } from '@/components/ui/button'; @@ -23,11 +23,15 @@ function RevisionCard({ selectedHash, revisions, onSelect, + onPrevious, + onBranchHead, }: { side: RevisionSide; selectedHash: string; revisions: CompareRevision[]; onSelect: (hash: string) => void; + onPrevious: () => void; + onBranchHead: () => void; }): JSX.Element { const selected = revisions.find(r => r.hash === selectedHash); @@ -74,6 +78,35 @@ function RevisionCard({ +
+ + +
+ {selected && (
{selected.commitName && ( @@ -81,7 +114,6 @@ function RevisionCard({ {selected.commitName}

)} -

{selected.shortHash}

{selected.date}

)} @@ -95,7 +127,11 @@ interface RevisionSelectorBarProps { revisions: CompareRevision[]; onHashAChange: (hash: string) => void; onHashBChange: (hash: string) => void; - onSuggestion: (suggestion: 'previous' | 'branchHead' | 'swap') => void; + onSideAction: ( + side: RevisionSide, + action: 'previous' | 'branchHead', + ) => void; + onSwap: () => void; } export function RevisionSelectorBar({ @@ -104,7 +140,8 @@ export function RevisionSelectorBar({ revisions, onHashAChange, onHashBChange, - onSuggestion, + onSideAction, + onSwap, }: RevisionSelectorBarProps): JSX.Element { return (
@@ -114,12 +151,22 @@ export function RevisionSelectorBar({ selectedHash={hashA} revisions={revisions} onSelect={onHashAChange} + onPrevious={() => onSideAction('A', 'previous')} + onBranchHead={() => onSideAction('A', 'branchHead')} />
-
+
+ + + +
onSideAction('B', 'previous')} + onBranchHead={() => onSideAction('B', 'branchHead')} />
- -
- - : - - - - -
); } diff --git a/dashboard/src/pages/TreeCompare/mock/compareFailuresMock.ts b/dashboard/src/pages/TreeCompare/mock/compareFailuresMock.ts index 1279de9ac..e7a7d4d2b 100644 --- a/dashboard/src/pages/TreeCompare/mock/compareFailuresMock.ts +++ b/dashboard/src/pages/TreeCompare/mock/compareFailuresMock.ts @@ -9,10 +9,10 @@ export const COMPARE_FAILURES_MOCK: TreeCompareFailuresData = { stats: { builds: { regression: 1, - fixed: 0, + fixed: 1, newFailure: 1, stillFailing: 0, - newPass: 14, + newPass: 13, }, boots: { regression: 0, @@ -68,7 +68,7 @@ export const COMPARE_FAILURES_MOCK: TreeCompareFailuresData = { config: 'defconfig', arch: 'arm64', compiler: 'gcc-12', - sideA: 'PASS', + sideA: '—', sideB: 'PASS', change: 'newPass', buildErrors: 0, @@ -79,9 +79,9 @@ export const COMPARE_FAILURES_MOCK: TreeCompareFailuresData = { config: 'multi_v7_defconfig', arch: 'arm', compiler: 'gcc-11', - sideA: 'PASS', + sideA: 'FAIL', sideB: 'PASS', - change: 'newPass', + change: 'fixed', buildErrors: 0, date: 'Jul 13, 12:16 UTC', }, diff --git a/dashboard/src/types/tree/TreeCompare.ts b/dashboard/src/types/tree/TreeCompare.ts index ecf132127..059b28685 100644 --- a/dashboard/src/types/tree/TreeCompare.ts +++ b/dashboard/src/types/tree/TreeCompare.ts @@ -13,10 +13,23 @@ export type CompareDelta = { fail: number; }; +export const compareChangeTypes = [ + 'regression', + 'fixed', + 'newFailure', + 'stillFailing', + 'newPass', +] as const; + +export type CompareChangeType = (typeof compareChangeTypes)[number]; + +export type CompareChangeStats = Record; + export type CompareEntitySummary = { sideA: CompareStatusCounts; sideB: CompareStatusCounts; delta: CompareDelta; + changes: CompareChangeStats; }; export type CompareGroupRow = { @@ -53,21 +66,9 @@ export type TreeCompareData = { /** Side status for an individual compared item (sketch). */ export type CompareItemStatus = 'PASS' | 'FAIL' | 'INCONCLUSIVE' | '—'; -export const compareChangeTypes = [ - 'regression', - 'fixed', - 'newFailure', - 'stillFailing', - 'newPass', -] as const; - -export type CompareChangeType = (typeof compareChangeTypes)[number]; - export const compareChangeFilters = ['all', ...compareChangeTypes] as const; export type CompareChangeFilter = (typeof compareChangeFilters)[number]; -export type CompareChangeStats = Record; - type CompareFailureRowBase = { id: string; change: CompareChangeType;