Skip to content

Commit 49879db

Browse files
committed
fix(hardware): resolve details trees from hardware_status in window
Stop selecting absolute tree tips for hardware details. Use hardware_status to pick the latest checkout per tree that actually ran the board, so older checkouts with tests still appear. Closes #1264 Signed-off-by: Alan Peixinho <alan.peixinho@profusion.mobi>
1 parent 8e04ba5 commit 49879db

5 files changed

Lines changed: 114 additions & 133 deletions

File tree

backend/kernelCI_app/queries/hardware.py

Lines changed: 53 additions & 130 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
from datetime import datetime
22
from typing import Optional, TypedDict
33

4-
from django.db import connection
4+
from django.db import connection, connections
55

66
from kernelCI_app.cache import get_query_cache, set_query_cache
77
from kernelCI_app.helpers.database import dict_fetchall
@@ -12,44 +12,38 @@
1212
from kernelCI_app.typeModels.hardwareDetails import CommitHead, Tree
1313

1414

15-
def _get_hardware_tree_heads_clause(*, id_only: bool) -> str:
16-
"""Returns the tree_heads for the hardware queries,
17-
where the checkout is not filtered by origin.
15+
def _get_hardware_trees_from_status_query(*, fields: str) -> str:
16+
"""Latest checkout per tree in the window for this hardware via hardware_status.
1817
19-
This is done because tests from a origin can be
20-
related to checkouts from another origin."""
21-
if id_only is True:
22-
fields = "C.id"
23-
else:
24-
fields = """C.id,
25-
C.origin,
26-
C.tree_name,
27-
C.start_time,
28-
C.git_repository_branch,
29-
C.git_repository_url,
30-
C.git_commit_name,
31-
C.git_commit_hash,
32-
C.git_commit_tags"""
33-
34-
return f"""SELECT DISTINCT
35-
ON (
36-
C.tree_name,
37-
C.git_repository_branch,
38-
C.git_repository_url,
39-
C.origin
40-
)
41-
{fields}
42-
FROM
43-
checkouts C
44-
WHERE
45-
C.start_time >= %(start_date)s
46-
AND C.start_time <= %(end_date)s
47-
ORDER BY
48-
C.tree_name ASC,
49-
C.git_repository_branch ASC,
50-
C.git_repository_url ASC,
51-
C.origin ASC,
52-
C.start_time DESC"""
18+
Checkout origin is not filtered: tests from one origin can be related to
19+
checkouts from another origin.
20+
"""
21+
return f"""
22+
SELECT DISTINCT ON (
23+
C.tree_name,
24+
C.git_repository_branch,
25+
C.git_repository_url,
26+
C.origin
27+
)
28+
{fields}
29+
FROM
30+
hardware_status HS
31+
INNER JOIN checkouts C ON C.id = HS.checkout_id
32+
WHERE
33+
HS.test_origin = %(origin)s
34+
AND (
35+
HS.platform = %(hardware)s
36+
OR HS.compatibles @> ARRAY[%(hardware)s]::TEXT[]
37+
)
38+
AND HS.start_time >= %(start_date)s
39+
AND HS.start_time <= %(end_date)s
40+
ORDER BY
41+
C.tree_name ASC,
42+
C.git_repository_branch ASC,
43+
C.git_repository_url ASC,
44+
C.origin ASC,
45+
C.start_time DESC
46+
"""
5347

5448

5549
# TODO: unify with get_tree_listing_count_clause
@@ -756,11 +750,7 @@ def get_hardware_trees_head_commits(
756750
start_datetime: datetime,
757751
end_datetime: datetime,
758752
) -> list[tuple[str, str]]:
759-
760-
# similar to the get_hardware_trees_data, except we limit the information
761-
# being brought to the commit hash
762-
763-
cache_key = "hardwareTreesHeadCommits"
753+
cache_key = "hardwareTreesHeadCommitsFromStatus"
764754

765755
cache_params = {
766756
"hardware": hardware_id,
@@ -774,54 +764,20 @@ def get_hardware_trees_head_commits(
774764
if trees:
775765
return trees
776766

777-
tree_head_clause = _get_hardware_tree_heads_clause(id_only=False)
778-
779-
# We need a subquery because if we filter by any hardware, it will get the
780-
# last head that has that hardware, but not the real head of the trees
781-
query = f"""
782-
WITH
783-
-- Selects the data of the latest checkout of all trees in the given period
784-
tree_heads AS (
785-
{tree_head_clause}
786-
)
787-
SELECT DISTINCT
788-
ON (
789-
TH.tree_name,
790-
TH.git_repository_branch,
791-
TH.git_repository_url,
792-
TH.git_commit_hash
793-
) TH.tree_name,
794-
TH.git_commit_hash
795-
FROM
796-
tests
797-
INNER JOIN builds ON tests.build_id = builds.id
798-
INNER JOIN tree_heads TH ON builds.checkout_id = TH.id
799-
WHERE
800-
(
801-
(
802-
tests.environment_compatible @> ARRAY[%(hardware)s]::TEXT[]
803-
OR tests.environment_misc ->> 'platform' = %(hardware)s
804-
)
805-
AND tests.origin = %(origin)s
806-
AND TH.start_time >= %(start_date)s
807-
AND TH.start_time <= %(end_date)s
808-
)
809-
ORDER BY
810-
TH.tree_name ASC,
811-
TH.git_repository_branch ASC,
812-
TH.git_repository_url ASC,
813-
TH.git_commit_hash ASC,
814-
TH.start_time DESC
815-
"""
767+
query = _get_hardware_trees_from_status_query(
768+
fields="""
769+
C.tree_name,
770+
C.git_commit_hash
771+
"""
772+
)
816773

817774
params = {
818775
"hardware": hardware_id,
819776
"origin": origin,
820777
"start_date": start_datetime,
821778
"end_date": end_datetime,
822779
}
823-
trees = []
824-
with connection.cursor() as cursor:
780+
with connections["default"].cursor() as cursor:
825781
cursor.execute(query, params)
826782
tree_records = dict_fetchall(cursor)
827783
trees = [
@@ -840,7 +796,7 @@ def get_hardware_trees_data(
840796
start_datetime: datetime,
841797
end_datetime: datetime,
842798
) -> list[Tree]:
843-
cache_key = "hardwareDetailsTreeData"
799+
cache_key = "hardwareDetailsTreeDataFromStatus"
844800

845801
params = {
846802
"hardware": hardware_id,
@@ -851,52 +807,19 @@ def get_hardware_trees_data(
851807

852808
trees: list[Tree] = get_query_cache(cache_key, params)
853809

854-
tree_head_clause = _get_hardware_tree_heads_clause(id_only=False)
855-
856810
if not trees:
857-
# We need a subquery because if we filter by any hardware, it will get the
858-
# last head that has that hardware, but not the real head of the trees
859-
query = f"""
860-
WITH
861-
-- Selects the data of the latest checkout of all trees in the given period
862-
tree_heads AS (
863-
{tree_head_clause}
864-
)
865-
SELECT DISTINCT
866-
ON (
867-
TH.tree_name,
868-
TH.git_repository_branch,
869-
TH.git_repository_url,
870-
TH.git_commit_hash
871-
) TH.tree_name,
872-
TH.origin,
873-
TH.git_repository_branch,
874-
TH.git_repository_url,
875-
TH.git_commit_name,
876-
TH.git_commit_hash,
877-
TH.git_commit_tags
878-
FROM
879-
tests
880-
INNER JOIN builds ON tests.build_id = builds.id
881-
INNER JOIN tree_heads TH ON builds.checkout_id = TH.id
882-
WHERE
883-
(
884-
(
885-
tests.environment_compatible @> ARRAY[%(hardware)s]::TEXT[]
886-
OR tests.environment_misc ->> 'platform' = %(hardware)s
887-
)
888-
AND tests.origin = %(origin)s
889-
AND TH.start_time >= %(start_date)s
890-
AND TH.start_time <= %(end_date)s
891-
)
892-
ORDER BY
893-
TH.tree_name ASC,
894-
TH.git_repository_branch ASC,
895-
TH.git_repository_url ASC,
896-
TH.git_commit_hash ASC,
897-
TH.start_time DESC
898-
"""
899-
with connection.cursor() as cursor:
811+
query = _get_hardware_trees_from_status_query(
812+
fields="""
813+
C.tree_name,
814+
C.origin,
815+
C.git_repository_branch,
816+
C.git_repository_url,
817+
C.git_commit_name,
818+
C.git_commit_hash,
819+
C.git_commit_tags
820+
"""
821+
)
822+
with connections["default"].cursor() as cursor:
900823
cursor.execute(query, params)
901824
tree_records = dict_fetchall(cursor)
902825

backend/kernelCI_app/tests/factories/mocks/fixtures/build_data.py

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,20 @@
144144
"status": "FAIL",
145145
"config_name": "defconfig",
146146
},
147+
"older_checkout_build": {
148+
"checkout_id": "older_checkout_with_tests",
149+
"origin": "maestro",
150+
"architecture": "arm64",
151+
"status": "PASS",
152+
"config_name": "defconfig",
153+
},
154+
"latest_checkout_build": {
155+
"checkout_id": "latest_checkout_without_tests",
156+
"origin": "maestro",
157+
"architecture": "arm64",
158+
"status": "PASS",
159+
"config_name": "defconfig",
160+
},
147161
# TODO: Add builds to test STATUS NULL + add integration tests for this case
148162
}
149163

backend/kernelCI_app/tests/factories/mocks/fixtures/tree_data.py

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,4 +167,23 @@
167167
), # failed_tests_build
168168
"hardware_platform": None,
169169
},
170+
# Older-checkout scenario (#1264): the older checkout ran "older-checkout-board",
171+
# but the latest checkout (latest_checkout_without_tests) did not. Details must
172+
# still resolve the older checkout as the tree head.
173+
"older_checkout_with_tests": {
174+
"origin": "maestro",
175+
"git_url": "https://git.kernel.org/pub/scm/linux/kernel/git/older-checkout/linux.git",
176+
"git_branch": "master",
177+
"tree_name": "older_checkout_mainline",
178+
"start_time": datetime.fromtimestamp(1741356000, timezone.utc),
179+
"hardware_platform": "older-checkout-board",
180+
},
181+
"latest_checkout_without_tests": {
182+
"origin": "maestro",
183+
"git_url": "https://git.kernel.org/pub/scm/linux/kernel/git/older-checkout/linux.git",
184+
"git_branch": "master",
185+
"tree_name": "older_checkout_mainline",
186+
"start_time": datetime.fromtimestamp(1741399200, timezone.utc),
187+
"hardware_platform": None,
188+
},
170189
}

backend/kernelCI_app/tests/integrationTests/hardwareDetailsSummary_test.py

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,18 @@
6363
),
6464
}
6565

66+
# Older-checkout board (#1264): tested only on an older checkout, not on the latest one.
67+
OLDER_CHECKOUT_HARDWARE = {
68+
"id": "older-checkout-board",
69+
"body": HardwareDetailsPostBody(
70+
origin="maestro",
71+
startTimestampInSeconds=1741300000,
72+
endTimestampInSeconds=1741420000,
73+
selectedCommits={},
74+
filter={},
75+
),
76+
}
77+
6678

6779
client = HardwareClient()
6880

@@ -202,6 +214,18 @@ def test_no_filters(base_hardware, status_code, has_error_body):
202214
)
203215

204216

217+
def test_older_checkout_board_resolves_older_checkout():
218+
"""#1264 regression: a board tested only on an older checkout (not the latest
219+
one) must still resolve that older checkout as the tree head."""
220+
response, content = request_data(OLDER_CHECKOUT_HARDWARE)
221+
assert_status_code(response=response, status_code=HTTPStatus.OK)
222+
assert "error" not in content
223+
224+
head_hashes = {tree["head_git_commit_hash"] for tree in content["common"]["trees"]}
225+
assert "older_checkout_with_tests" in head_hashes
226+
assert "latest_checkout_without_tests" not in head_hashes
227+
228+
205229
def test_filter_test_status(test_status_input):
206230
"""
207231
Tests for the status filter for both boots and tests

backend/kernelCI_app/tests/unitTests/queries/hardware_queries_test.py

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -75,9 +75,9 @@ def test_get_hardware_trees_data_from_cache(self, mock_get_cache):
7575
@patch("kernelCI_app.queries.hardware.get_query_cache")
7676
@patch("kernelCI_app.queries.hardware.set_query_cache")
7777
@patch("kernelCI_app.queries.hardware.dict_fetchall")
78-
@patch("kernelCI_app.queries.hardware.connection")
78+
@patch("kernelCI_app.queries.hardware.connections")
7979
def test_get_hardware_trees_data_from_database(
80-
self, mock_connection, mock_dict_fetchall, mock_set_cache, mock_get_cache
80+
self, mock_connections, mock_dict_fetchall, mock_set_cache, mock_get_cache
8181
):
8282
tree_records = [
8383
{
@@ -92,7 +92,7 @@ def test_get_hardware_trees_data_from_database(
9292
]
9393
mock_get_cache.return_value = None
9494
mock_dict_fetchall.return_value = tree_records
95-
setup_mock_cursor(mock_connection)
95+
setup_mock_cursor(mock_connections.__getitem__.return_value)
9696

9797
result = get_hardware_trees_data(
9898
hardware_id="hardware",
@@ -104,6 +104,7 @@ def test_get_hardware_trees_data_from_database(
104104
assert len(result) == 1
105105
assert result[0].tree_name == "mainline"
106106
mock_set_cache.assert_called_once()
107+
mock_connections.__getitem__.assert_called_with("default")
107108

108109

109110
class TestGenerateQueryParams:

0 commit comments

Comments
 (0)