diff --git a/backend/kernelCI_app/helpers/filters.py b/backend/kernelCI_app/helpers/filters.py index 2ad2c815..079ee2a1 100644 --- a/backend/kernelCI_app/helpers/filters.py +++ b/backend/kernelCI_app/helpers/filters.py @@ -1,4 +1,4 @@ -from typing import Optional, Dict, List, TypedDict, Literal, Any +from typing import Optional, Dict, List, Tuple, TypedDict, Literal, Any from django.http import HttpResponseBadRequest import re from kernelCI_app.utils import getErrorResponseBody @@ -15,8 +15,32 @@ def is_known_issue(issue_id: Optional[str]) -> bool: return issue_id is not None and issue_id is not UNKNOWN_STRING -def is_issue_from_test(incident_test_id: Optional[str], is_known_issue: bool) -> bool: - return incident_test_id is not None or not is_known_issue +def is_exclusively_build_issue(issue_id: Optional[str], incident_test_id: Optional[str]) -> bool: + is_known_issue_result = is_known_issue(issue_id) + is_exclusively_build_issue_result = is_known_issue_result and incident_test_id is None + return is_exclusively_build_issue_result + + +def is_exclusively_test_issue( + *, issue_id: Optional[str], incident_test_id: Optional[str] +) -> bool: + is_known_issue_result = is_known_issue(issue_id) + is_exclusively_test_issue_result = is_known_issue_result and incident_test_id is not None + return is_exclusively_test_issue_result + + +def is_issue_from_test(*, incident_test_id: Optional[str], issue_id: Optional[str]) -> bool: + is_known_issue_result = is_known_issue(issue_id=issue_id) + is_possible_test_issue = incident_test_id is not None or not is_known_issue_result + + return is_possible_test_issue + + +def is_issue_from_build(*, issue_id: Optional[str], incident_test_id: Optional[str]) -> bool: + is_known_issue_result = is_known_issue(issue_id=issue_id) + is_possible_build_issue = incident_test_id is None or not is_known_issue_result + + return is_possible_build_issue # TODO: consider issue_version in the filter @@ -25,35 +49,96 @@ def is_issue_filtered_out(issue_id: Optional[str], issue_filters: set) -> bool: def should_filter_test_issue( - issue_filters: set, issue_id: Optional[str], incident_test_id: Optional[str] + issue_filters: set, issue_id: Optional[str], incident_test_id: Optional[str], test_status: Optional[str] ) -> bool: has_issue_filter = len(issue_filters) > 0 + if (not has_issue_filter): + return False - is_known_issue_result = is_known_issue(issue_id) - is_issue_from_tests_result = is_issue_from_test( - incident_test_id, is_known_issue_result + if (test_status != "FAIL"): + return True + + has_unknown_filter = UNKNOWN_STRING in issue_filters + + is_exclusively_build_issue_result = is_exclusively_build_issue( + issue_id=issue_id, incident_test_id=incident_test_id ) + if is_exclusively_build_issue_result and has_unknown_filter: + return False + if is_exclusively_build_issue_result: + issue_id = UNKNOWN_STRING + is_issue_filtered_out_result = is_issue_filtered_out(issue_id, issue_filters) - return ( - has_issue_filter and is_issue_from_tests_result and is_issue_filtered_out_result + return is_issue_filtered_out_result + + +def should_filter_build_issue( + *, + issue_filters: set, + issue_id: Optional[str], + incident_test_id: Optional[str], + build_valid: Optional[bool], +) -> bool: + has_issue_filter = len(issue_filters) > 0 + if (not has_issue_filter): + return False + + if (not is_build_invalid(build_valid)): + return True + + has_unknown_filter = UNKNOWN_STRING in issue_filters + + is_exclusively_test_issue_result = is_exclusively_test_issue( + issue_id=issue_id, incident_test_id=incident_test_id ) + if is_exclusively_test_issue_result and has_unknown_filter: + return False + if is_exclusively_test_issue_result: + issue_id = UNKNOWN_STRING + + is_issue_filtered_out_result = is_issue_filtered_out(issue_id, issue_filters) + + return is_issue_filtered_out_result + def should_increment_test_issue( issue_id: Optional[str], incident_test_id: Optional[str] -) -> bool: - is_known_issue_result = is_known_issue(issue_id=issue_id) - is_exclusively_build_issue = is_known_issue_result and incident_test_id is None - if is_exclusively_build_issue: - issue_id = UNKNOWN_STRING +) -> Tuple[str, bool]: + is_exclusively_build_issue_result = is_exclusively_build_issue( + issue_id=issue_id, incident_test_id=incident_test_id + ) + if is_exclusively_build_issue_result: + return (UNKNOWN_STRING, False) + + is_issue_from_test_result = is_issue_from_test( + incident_test_id=incident_test_id, issue_id=issue_id + ) - is_unknown_issue = issue_id is UNKNOWN_STRING - is_known_test_issue = incident_test_id is not None - is_issue_from_test = is_known_test_issue or is_unknown_issue + return (issue_id, is_issue_from_test_result) - return is_issue_from_test + +def should_increment_build_issue( + *, + issue_id: Optional[str], + incident_test_id: Optional[str], + build_valid: Optional[bool], +) -> Tuple[str, bool]: + is_exclusively_test_issue_result = is_exclusively_test_issue( + issue_id=issue_id, incident_test_id=incident_test_id + ) + if is_exclusively_test_issue_result: + return (UNKNOWN_STRING, False) + + is_issue_from_build_result = is_issue_from_build( + issue_id=issue_id, incident_test_id=incident_test_id + ) + + result = is_issue_from_build_result and is_build_invalid(build_valid) + + return (issue_id, result) def toIntOrDefault(value, default): @@ -320,6 +405,7 @@ def is_build_filtered_out( duration: Optional[int], valid: Optional[bool], issue_id: Optional[str], + incident_test_id: Optional[str], ) -> bool: return ( ( @@ -342,8 +428,12 @@ def is_build_filtered_out( and (toIntOrDefault(duration, 0) < self.filterBuildDurationMin) ) or ( - len(self.filterIssues["build"]) > 0 - and (issue_id not in self.filterIssues["build"] or valid is True) + should_filter_build_issue( + issue_filters=self.filterIssues["build"], + issue_id=issue_id, + incident_test_id=incident_test_id, + build_valid=valid, + ) ) ) @@ -422,7 +512,7 @@ def is_boot_filtered_out( and (toIntOrDefault(duration, 0) < self.filterBootDurationMin) ) or should_filter_test_issue( - self.filterIssues["boot"], issue_id, incident_test_id + self.filterIssues["boot"], issue_id, incident_test_id, status ) or ( len(self.filterPlatforms["boot"]) > 0 @@ -464,7 +554,7 @@ def is_test_filtered_out( and (toIntOrDefault(duration, 0) < self.filterTestDurationMin) ) or should_filter_test_issue( - self.filterIssues["test"], issue_id, incident_test_id + self.filterIssues["test"], issue_id, incident_test_id, status ) or ( len(self.filterPlatforms["test"]) > 0 diff --git a/backend/kernelCI_app/helpers/treeDetails.py b/backend/kernelCI_app/helpers/treeDetails.py index 9c3af490..003d1ba1 100644 --- a/backend/kernelCI_app/helpers/treeDetails.py +++ b/backend/kernelCI_app/helpers/treeDetails.py @@ -1,6 +1,7 @@ from collections.abc import Callable from typing import Any from kernelCI_app.helpers.filters import ( + should_increment_build_issue, should_increment_test_issue, UNKNOWN_STRING, ) @@ -103,6 +104,7 @@ def get_tree_details_data(request, commit_hash): builds_filter.builds_id = incidents.build_id LEFT JOIN issues ON incidents.issue_id = issues.id + AND incidents.issue_version = issues.version WHERE tests.origin = %(origin_param)s OR tests.origin IS NULL @@ -258,8 +260,15 @@ def process_builds_issue(instance, row_data): issue_report_url = row_data["issue_report_url"] issue_version = row_data["issue_version"] build_valid = row_data["build_valid"] + incident_test_id = row_data["incident_test_id"] + + (issue_id, can_insert_issue) = should_increment_build_issue( + issue_id=issue_id, + incident_test_id=incident_test_id, + build_valid=build_valid, + ) - if issue_id and (build_valid is False or build_valid is None): + if issue_id and issue_version is not None and can_insert_issue: current_issue = instance.processed_build_issues.get((issue_id, issue_version)) if current_issue: current_issue["incidents_info"]["incidentsCount"] += 1 @@ -276,16 +285,18 @@ def process_builds_issue(instance, row_data): def process_tests_issue(instance, row_data): - testStatus = row_data["test_status"] + test_status = row_data["test_status"] issue_id = row_data["issue_id"] issue_comment = row_data["issue_comment"] issue_version = row_data["issue_version"] issue_report_url = row_data["issue_report_url"] incident_test_id = row_data["incident_test_id"] - can_insert_issue = should_increment_test_issue(issue_id, incident_test_id) + (issue_id, can_insert_issue) = should_increment_test_issue( + issue_id, incident_test_id + ) - if issue_id and can_insert_issue: + if issue_id and issue_version is not None and can_insert_issue: currentIssue = instance.testIssuesTable.get((issue_id, issue_version)) if currentIssue: currentIssue["incidents_info"]["incidentsCount"] += 1 @@ -296,7 +307,7 @@ def process_tests_issue(instance, row_data): issue_comment=issue_comment, issue_report_url=issue_report_url, ) - elif testStatus == "FAIL": + elif test_status == "FAIL": instance.failedTestsWithUnknownIssues += 1 @@ -308,11 +319,11 @@ def process_boots_issue(instance, row_data): issue_report_url = row_data["issue_report_url"] incident_test_id = row_data["incident_test_id"] - can_insert_issue = should_increment_test_issue( + (issue_id, can_insert_issue) = should_increment_test_issue( issue_id=issue_id, incident_test_id=incident_test_id ) - if issue_id and can_insert_issue: + if issue_id and issue_version is not None and can_insert_issue: currentIssue = instance.bootsIssuesTable.get((issue_id, issue_version)) if currentIssue: currentIssue["incidents_info"]["incidentsCount"] += 1 @@ -331,9 +342,13 @@ def decide_if_is_build_filtered_out(instance, row_data): issue_id = row_data["issue_id"] build_valid = row_data["build_valid"] build_duration = row_data["build_duration"] + incident_test_id = row_data["incident_test_id"] is_build_filtered_out = instance.filters.is_build_filtered_out( - valid=build_valid, duration=build_duration, issue_id=issue_id + valid=build_valid, + duration=build_duration, + issue_id=issue_id, + incident_test_id=incident_test_id, ) return is_build_filtered_out diff --git a/backend/kernelCI_app/typeModels/issueDetails.py b/backend/kernelCI_app/typeModels/issueDetails.py index 9fd61d14..34807d98 100644 --- a/backend/kernelCI_app/typeModels/issueDetails.py +++ b/backend/kernelCI_app/typeModels/issueDetails.py @@ -1,6 +1,11 @@ +from typing import Dict, Tuple +from kernelCI_app.utils import Issue from pydantic import BaseModel class IssueDetailsPathParameters(BaseModel): issue_id: str version: int + + +type IssueDict = Dict[Tuple[str, str], Issue] diff --git a/backend/kernelCI_app/unitTests/treeDetails.test.py b/backend/kernelCI_app/unitTests/treeDetails.test.py new file mode 100644 index 00000000..c4f9c905 --- /dev/null +++ b/backend/kernelCI_app/unitTests/treeDetails.test.py @@ -0,0 +1,32 @@ +import unittest +from kernelCI_app.helpers.filters import should_filter_test_issue, UNKNOWN_STRING + + +# TODO: replace with pytest +class TestShouldFilterTestIssue(unittest.TestCase): + + def test_no_issue_filters(self): + self.assertFalse( + should_filter_test_issue(set(), UNKNOWN_STRING, "incident_test_1", "FAIL") + ) + + def test_unknown_filter_with_exclusively_build_issue(self): + self.assertTrue( + should_filter_test_issue( + {UNKNOWN_STRING}, "issue1", "incident_test_1", "PASS" + ) + ) + + def test_unknown_issue_but_not_from_test(self): + self.assertFalse( + should_filter_test_issue( + {UNKNOWN_STRING}, + "maestro:72697a4efbbd0eff7080781839b405bbf0902f79", + None, + "FAIL", + ) + ) + + +if __name__ == "__main__": + unittest.main() diff --git a/backend/kernelCI_app/views/hardwareDetailsView.py b/backend/kernelCI_app/views/hardwareDetailsView.py index f4c969f2..5bedb61c 100644 --- a/backend/kernelCI_app/views/hardwareDetailsView.py +++ b/backend/kernelCI_app/views/hardwareDetailsView.py @@ -3,7 +3,10 @@ from http import HTTPStatus import json from typing import Dict, List, Optional, Set, Literal -from kernelCI_app.helpers.filters import should_increment_test_issue, is_build_invalid +from kernelCI_app.helpers.filters import ( + should_increment_build_issue, + should_increment_test_issue, +) from kernelCI_app.helpers.errorHandling import create_error_response from kernelCI_app.helpers.logger import log_message from django.utils.decorators import method_decorator @@ -152,12 +155,16 @@ def update_issues( ) -> None: can_insert_issue = True if issue_from == "build": - can_insert_issue = is_build_invalid(build_valid) + (issue_id, can_insert_issue) = should_increment_build_issue( + issue_id=issue_id, + incident_test_id=incident_test_id, + build_valid=build_valid, + ) elif issue_from == "test": - can_insert_issue = should_increment_test_issue(issue_id, incident_test_id) + (issue_id, can_insert_issue) = should_increment_test_issue(issue_id, incident_test_id) if issue_id and issue_version is not None and can_insert_issue: - existing_issue = task["issues"].get(issue_id) + existing_issue = task["issues"].get((issue_id, issue_version)) if existing_issue: existing_issue["incidents_info"]["incidentsCount"] += 1 else: @@ -167,7 +174,7 @@ def update_issues( issue_comment=issue_comment, issue_report_url=issue_report_url, ) - task["issues"][issue_id] = new_issue + task["issues"][(issue_id, issue_version)] = new_issue elif is_failed_task: task["failedWithUnknownIssues"] += 1 @@ -221,16 +228,18 @@ def setup_filters(self): self.filterValid = self.filterParams.filterBuildValid self.filterIssues = self.filterParams.filterIssues - def is_build_filtered_in( + def build_in_filter( self, build: Dict, processed_builds: Set[str], + incident_test_id: Optional[str], ) -> bool: is_build_not_processed = build["id"] not in processed_builds is_build_filtered_out = self.filterParams.is_build_filtered_out( valid=build["valid"], duration=build["duration"], issue_id=build["issue_id"], + incident_test_id=incident_test_id, ) return is_build_not_processed and not is_build_filtered_out @@ -410,7 +419,11 @@ def sanitize_records(self, records, trees: List, is_all_selected: bool): processed_tests.add(record["id"]) self.handle_test(record, boots if is_record_boot else tests) - should_process_build = self.is_build_filtered_in(build, processed_builds) + should_process_build = self.build_in_filter( + build=build, + processed_builds=processed_builds, + incident_test_id=record["incidents__test_id"], + ) processed_builds.add(build_id) if should_process_build: diff --git a/backend/kernelCI_app/views/issuesView.py b/backend/kernelCI_app/views/issuesView.py index 327c9b2f..51506955 100644 --- a/backend/kernelCI_app/views/issuesView.py +++ b/backend/kernelCI_app/views/issuesView.py @@ -10,6 +10,7 @@ create_issue, ) from http import HTTPStatus +from kernelCI_app.typeModels.issueDetails import IssueDict class IssueView(View): @@ -22,16 +23,19 @@ def get_dict_record(self, row) -> Dict[str, str]: return record def sanitize_rows(self, rows) -> List[Issue]: - result = {} + + result: IssueDict = {} for row in rows: record = self.get_dict_record(row) - currentIssue = result.get(record["id"]) + issue_id = record["id"] + issue_version = record["version"] + currentIssue = result.get((issue_id, issue_version)) if currentIssue: currentIssue["incidents_info"]["incidentsCount"] += 1 else: - result[record["id"]] = create_issue( - issue_id=record["id"], - issue_version=record["version"], + result[(issue_id, issue_version)] = create_issue( + issue_id=issue_id, + issue_version=issue_version, issue_comment=record["comment"], issue_report_url=record["report_url"], ) @@ -48,6 +52,7 @@ def get_test_issues(self, test_id: str) -> List[Issue]: FROM incidents JOIN issues ON incidents.issue_id = issues.id + AND incidents.issue_version = issues.version WHERE incidents.test_id = %s """ with connection.cursor() as cursor: @@ -67,6 +72,7 @@ def get_build_issues(self, build_id: str) -> List[Issue]: FROM incidents JOIN issues ON incidents.issue_id = issues.id + AND incidents.issue_version = issues.version WHERE incidents.build_id = %s """ with connection.cursor() as cursor: diff --git a/backend/kernelCI_app/views/treeCommitsHistory.py b/backend/kernelCI_app/views/treeCommitsHistory.py index b8c69453..2dc80e0a 100644 --- a/backend/kernelCI_app/views/treeCommitsHistory.py +++ b/backend/kernelCI_app/views/treeCommitsHistory.py @@ -19,7 +19,7 @@ from rest_framework.views import APIView from rest_framework.response import Response from rest_framework import status -from typing import Dict, List +from typing import Dict, List, Optional # TODO Move this endpoint to a function so it doesn't @@ -106,10 +106,14 @@ def _process_builds_count( build_valid: bool, duration: int, commit_hash: str, - issue_id: str + issue_id: str, + incident_test_id: Optional[str], ) -> None: is_filtered_out = self.filterParams.is_build_filtered_out( - duration=duration, valid=build_valid, issue_id=issue_id + duration=duration, + valid=build_valid, + issue_id=issue_id, + incident_test_id=incident_test_id, ) if is_filtered_out: return @@ -262,7 +266,8 @@ def _process_builds(self, row: Dict) -> None: build_valid=row["build_valid"], duration=row["build_duration"], commit_hash=commit_hash, - issue_id=row['issue_id'] + issue_id=row['issue_id'], + incident_test_id=row['incidents_test_id'] ) def _is_record_in_time_period(self, start_time: datetime) -> bool: diff --git a/backend/kernelCI_app/views/treeDetailsView.py b/backend/kernelCI_app/views/treeDetailsView.py index 3f0d60ac..cca23691 100644 --- a/backend/kernelCI_app/views/treeDetailsView.py +++ b/backend/kernelCI_app/views/treeDetailsView.py @@ -1,4 +1,4 @@ -from typing import Dict, List, Tuple +from typing import List from django.http import JsonResponse from http import HTTPStatus from django.views import View @@ -32,7 +32,7 @@ from kernelCI_app.viewCommon import create_details_build_summary -type IssueDict = Dict[Tuple[str, str], Issue] +from kernelCI_app.typeModels.issueDetails import IssueDict class TreeDetails(View):