Filter file comparisons by flags

compare/commands/compare/compare.py

@@ -1,8 +1,16 @@
 from codecov.commands.base import BaseCommand
+from services.comparison import Comparison, ComparisonReport
 
 from .interactors.fetch_impacted_files import FetchImpactedFiles
 
 
 class CompareCommands(BaseCommand):
-    def fetch_impacted_files(self, comparsion, filters):
-        return self.get_interactor(FetchImpactedFiles).execute(comparsion, filters)
+    def fetch_impacted_files(
+        self,
+        comparison_report: ComparisonReport,
+        comparison: Comparison,
+        filters,
+    ):
+        return self.get_interactor(FetchImpactedFiles).execute(
+            comparison_report, comparison, filters
+        )

compare/commands/compare/interactors/fetch_impacted_files.py

@@ -1,7 +1,9 @@
 import enum
+from typing import List, Optional, Union
 
 from codecov.commands.base import BaseInteractor
-from services.comparison import ImpactedFile
+from services.comparison import Comparison, ComparisonReport, ImpactedFile
+from services.report import files_belonging_to_flags
 
 
 class ImpactedFileParameter(enum.Enum):
@@ -13,13 +15,28 @@ class ImpactedFileParameter(enum.Enum):
 
 
 class FetchImpactedFiles(BaseInteractor):
-    def _apply_filters(self, impacted_files, filters):
+    def _apply_filters(
+        self,
+        impacted_files: Optional[List[ImpactedFile]],
+        comparison: Comparison,
+        filters,
+    ):
         parameter = filters.get("ordering", {}).get("parameter")
         direction = filters.get("ordering", {}).get("direction")
         if parameter and direction:
             impacted_files = self.sort_impacted_files(
                 impacted_files, parameter, direction
             )
+        flags = filters.get("flags", [])
+        if flags and comparison:
+            head_commit_report = comparison.head_report
+            if set(flags) & set(head_commit_report.flags):
+                files = files_belonging_to_flags(
+                    commit_report=head_commit_report, flags=flags
+                )
+                impacted_files = [
+                    file for file in impacted_files if file.head_name in files
+                ]
         return impacted_files
 
     def get_attribute(
@@ -65,7 +82,12 @@ def sort_impacted_files(self, impacted_files, parameter, direction):
         # Merge both lists together
         return files_with_coverage + files_without_coverage
 
-    def execute(self, comparison_report, filters):
+    def execute(
+        self,
+        comparison_report: ComparisonReport,
+        comparison: Comparison,
+        filters,
+    ):
         if filters is None:
             return comparison_report.impacted_files
 
@@ -79,4 +101,4 @@ def execute(self, comparison_report, filters):
         else:
             impacted_files = comparison_report.impacted_files
 
-        return self._apply_filters(impacted_files, filters)
+        return self._apply_filters(impacted_files, comparison, filters)

compare/commands/compare/interactors/tests/test_fetch_impacted_files.py

@@ -1,16 +1,18 @@
 import enum
-from unittest.mock import patch
+from unittest.mock import PropertyMock, patch
 
 from django.contrib.auth.models import AnonymousUser
 from django.test import TransactionTestCase
+from shared.reports.resources import Report, ReportFile, ReportLine
+from shared.utils.sessions import Session
 
 from codecov_auth.tests.factories import OwnerFactory
 from compare.commands.compare.interactors.fetch_impacted_files import (
     ImpactedFileParameter,
 )
 from compare.tests.factories import CommitComparisonFactory
-from core.tests.factories import CommitFactory
-from services.comparison import ComparisonReport
+from core.tests.factories import CommitFactory, PullFactory, RepositoryFactory
+from services.comparison import Comparison, ComparisonReport, PullRequestComparison
 
 from ..fetch_impacted_files import FetchImpactedFiles
 
@@ -274,7 +276,7 @@ def setUp(self):
             compare_commit=self.commit,
             report_storage_path="v4/test.json",
         )
-        self.comparison = ComparisonReport(self.commit_comparison)
+        self.comparison_report = ComparisonReport(self.commit_comparison)
 
     # helper to execute the interactor
     def execute(self, owner, *args):
@@ -287,7 +289,8 @@ def test_impacted_file_sort_function(self, read_file):
         parameter = ImpactedFileParameter.CHANGE_COVERAGE
         direction = OrderingDirection.ASC
         filters = {"ordering": {"parameter": parameter, "direction": direction}}
-        sorted_files = self.execute(None, self.comparison, filters)
+        comparison = None
+        sorted_files = self.execute(None, self.comparison_report, comparison, filters)
         assert [file.head_name for file in sorted_files] == ["fileA", "fileB"]
 
     @patch("services.archive.ArchiveService.read_file")
@@ -296,7 +299,8 @@ def test_impacted_file_sort_function_no_misses(self, read_file):
         parameter = ImpactedFileParameter.MISSES_COUNT
         direction = OrderingDirection.ASC
         filters = {"ordering": {"parameter": parameter, "direction": direction}}
-        sorted_files = self.execute(None, self.comparison, filters)
+        comparison = None
+        sorted_files = self.execute(None, self.comparison_report, comparison, filters)
         assert [file.head_name for file in sorted_files] == ["fileA", "fileB"]
 
     @patch("services.archive.ArchiveService.read_file")
@@ -305,9 +309,10 @@ def test_impacted_file_sort_function_error(self, read_file):
         parameter = "something else"
         direction = OrderingDirection.DESC
         filters = {"ordering": {"parameter": parameter, "direction": direction}}
+        comparison = None
 
         with self.assertRaises(ValueError) as ctx:
-            self.execute(None, self.comparison, filters)
+            self.execute(None, self.comparison_report, comparison, filters)
         self.assertEqual(
             "invalid impacted file parameter: something else", str(ctx.exception)
         )
@@ -321,7 +326,8 @@ def test_impacted_files_filtered_by_change_coverage_ascending(self, read_file):
                 "parameter": ImpactedFileParameter.CHANGE_COVERAGE,
             }
         }
-        impacted_files = self.execute(None, self.comparison, filters)
+        comparison = None
+        impacted_files = self.execute(None, self.comparison_report, comparison, filters)
         assert [file.head_name for file in impacted_files] == ["fileA", "fileB"]
 
     @patch("services.archive.ArchiveService.read_file")
@@ -333,7 +339,8 @@ def test_impacted_files_filtered_by_change_coverage_descending(self, read_file):
                 "parameter": ImpactedFileParameter.CHANGE_COVERAGE,
             }
         }
-        impacted_files = self.execute(None, self.comparison, filters)
+        comparison = None
+        impacted_files = self.execute(None, self.comparison_report, comparison, filters)
         assert [file.head_name for file in impacted_files] == ["fileB", "fileA"]
 
     @patch("services.archive.ArchiveService.read_file")
@@ -345,7 +352,8 @@ def test_impacted_files_filtered_by_head_coverage_ascending(self, read_file):
                 "parameter": ImpactedFileParameter.HEAD_COVERAGE,
             }
         }
-        impacted_files = self.execute(None, self.comparison, filters)
+        comparison = None
+        impacted_files = self.execute(None, self.comparison_report, comparison, filters)
         assert [file.head_name for file in impacted_files] == ["fileA", "fileB"]
 
     @patch("services.archive.ArchiveService.read_file")
@@ -357,7 +365,8 @@ def test_impacted_files_filtered_by_patch_coverage_ascending(self, read_file):
                 "parameter": ImpactedFileParameter.PATCH_COVERAGE,
             }
         }
-        impacted_files = self.execute(None, self.comparison, filters)
+        comparison = None
+        impacted_files = self.execute(None, self.comparison_report, comparison, filters)
         assert [file.head_name for file in impacted_files] == ["fileA", "fileB"]
 
     @patch("services.archive.ArchiveService.read_file")
@@ -369,7 +378,8 @@ def test_impacted_files_filtered_by_patch_coverage_descending(self, read_file):
                 "parameter": ImpactedFileParameter.PATCH_COVERAGE,
             }
         }
-        impacted_files = self.execute(None, self.comparison, filters)
+        comparison = None
+        impacted_files = self.execute(None, self.comparison_report, comparison, filters)
         assert [file.head_name for file in impacted_files] == ["fileB", "fileA"]
 
     @patch("services.archive.ArchiveService.read_file")
@@ -381,7 +391,8 @@ def test_impacted_files_filtered_by_head_coverage_descending(self, read_file):
                 "parameter": ImpactedFileParameter.HEAD_COVERAGE,
             }
         }
-        impacted_files = self.execute(None, self.comparison, filters)
+        comparison = None
+        impacted_files = self.execute(None, self.comparison_report, comparison, filters)
         assert [file.head_name for file in impacted_files] == ["fileB", "fileA"]
 
     @patch("services.archive.ArchiveService.read_file")
@@ -393,7 +404,8 @@ def test_impacted_files_filtered_by_head_name_ascending(self, read_file):
                 "parameter": ImpactedFileParameter.FILE_NAME,
             }
         }
-        impacted_files = self.execute(None, self.comparison, filters)
+        comparison = None
+        impacted_files = self.execute(None, self.comparison_report, comparison, filters)
         assert [file.head_name for file in impacted_files] == ["fileA", "fileB"]
 
     @patch("services.archive.ArchiveService.read_file")
@@ -405,7 +417,8 @@ def test_impacted_files_filtered_by_head_name_descending(self, read_file):
                 "parameter": ImpactedFileParameter.FILE_NAME,
             }
         }
-        impacted_files = self.execute(None, self.comparison, filters)
+        comparison = None
+        impacted_files = self.execute(None, self.comparison_report, comparison, filters)
         assert [file.head_name for file in impacted_files] == ["fileB", "fileA"]
 
     @patch("services.archive.ArchiveService.read_file")
@@ -417,7 +430,8 @@ def test_impacted_files_filtered_by_misses_count_ascending(self, read_file):
                 "parameter": ImpactedFileParameter.MISSES_COUNT,
             }
         }
-        impacted_files = self.execute(None, self.comparison, filters)
+        comparison = None
+        impacted_files = self.execute(None, self.comparison_report, comparison, filters)
         assert [file.head_name for file in impacted_files] == ["fileA", "fileB"]
 
     @patch("services.archive.ArchiveService.read_file")
@@ -429,14 +443,16 @@ def test_impacted_files_filtered_by_misses_count_descending(self, read_file):
                 "parameter": ImpactedFileParameter.MISSES_COUNT,
             }
         }
-        impacted_files = self.execute(None, self.comparison, filters)
+        comparison = None
+        impacted_files = self.execute(None, self.comparison_report, comparison, filters)
         assert [file.head_name for file in impacted_files] == ["fileB", "fileA"]
 
     @patch("services.archive.ArchiveService.read_file")
     def test_impacted_files_without_filters(self, read_file):
         read_file.return_value = mock_data_from_archive
         filters = {}
-        impacted_files = self.execute(None, self.comparison, filters)
+        comparison = None
+        impacted_files = self.execute(None, self.comparison_report, comparison, filters)
         assert [file.head_name for file in impacted_files] == ["fileA", "fileB"]
 
     @patch("services.archive.ArchiveService.read_file")
@@ -449,7 +465,8 @@ def test_impacted_files_filtered_by_unintended_changes(self, read_file):
                 "parameter": ImpactedFileParameter.FILE_NAME,
             },
         }
-        impacted_files = self.execute(None, self.comparison, filters)
+        comparison = None
+        impacted_files = self.execute(None, self.comparison_report, comparison, filters)
         assert [file.head_name for file in impacted_files] == ["fileA", "fileB"]
 
     @patch("services.archive.ArchiveService.read_file")
@@ -464,5 +481,79 @@ def test_impacted_files_filtered_by_unintended_changes_set_to_false(
                 "parameter": ImpactedFileParameter.FILE_NAME,
             },
         }
-        impacted_files = self.execute(None, self.comparison, filters)
+        comparison = None
+        impacted_files = self.execute(None, self.comparison_report, comparison, filters)
         assert [file.head_name for file in impacted_files] == ["fileA", "fileB"]
+
+    @patch("services.comparison.Comparison.head_report", new_callable=PropertyMock)
+    @patch("services.archive.ArchiveService.read_file")
+    def test_impacted_files_filtered_by_flags_and_commit_comparison_for_pull(
+        self, read_file, build_report_from_commit_mock
+    ):
+        read_file.return_value = mocked_files_with_direct_and_indirect_changes
+
+        commit_report = Report()
+        session_a_id, _ = commit_report.add_session(Session(flags=["flag-123"]))
+        session_b_id, _ = commit_report.add_session(Session(flags=["flag-456"]))
+        file_a = ReportFile("fileA")
+        file_a.append(1, ReportLine.create(coverage=1, sessions=[[session_a_id, 1]]))
+        commit_report.append(file_a)
+        file_b = ReportFile("fileB")
+        file_b.append(1, ReportLine.create(coverage=1, sessions=[[session_b_id, 1]]))
+        commit_report.append(file_b)
+        build_report_from_commit_mock.return_value = commit_report
+
+        flags = ["flag-123"]
+        filters = {"flags": flags}
+
+        owner = OwnerFactory()
+        repo = RepositoryFactory(author=owner)
+        base, head, compared_to = (
+            CommitFactory(repository=repo),
+            CommitFactory(repository=repo),
+            CommitFactory(repository=repo),
+        )
+        pull = PullFactory(
+            repository=repo,
+            base=base.commitid,
+            head=head.commitid,
+            compared_to=compared_to.commitid,
+        )
+        comparison = PullRequestComparison(user=owner, pull=pull)
+
+        impacted_files = self.execute(None, self.comparison_report, comparison, filters)
+        assert len(impacted_files) == 1
+        assert impacted_files[0].head_name == "fileA"
+
+    @patch("services.comparison.Comparison.head_report", new_callable=PropertyMock)
+    @patch("services.archive.ArchiveService.read_file")
+    def test_impacted_files_filtered_by_flags_and_commit_comparison_for_parent_commit(
+        self, read_file, build_report_from_commit_mock
+    ):
+        read_file.return_value = mocked_files_with_direct_and_indirect_changes
+
+        commit_report = Report()
+        session_a_id, _ = commit_report.add_session(Session(flags=["flag-123"]))
+        session_b_id, _ = commit_report.add_session(Session(flags=["flag-456"]))
+        file_a = ReportFile("fileA")
+        file_a.append(1, ReportLine.create(coverage=1, sessions=[[session_a_id, 1]]))
+        commit_report.append(file_a)
+        file_b = ReportFile("fileB")
+        file_b.append(1, ReportLine.create(coverage=1, sessions=[[session_b_id, 1]]))
+        commit_report.append(file_b)
+        build_report_from_commit_mock.return_value = commit_report
+
+        flags = ["flag-123"]
+        filters = {"flags": flags}
+
+        owner = OwnerFactory()
+        repo = RepositoryFactory(author=owner)
+        base, head = (
+            CommitFactory(repository=repo),
+            CommitFactory(repository=repo),
+        )
+        comparison = Comparison(user=owner, base_commit=base, head_commit=head)
+
+        impacted_files = self.execute(None, self.comparison_report, comparison, filters)
+        assert len(impacted_files) == 1
+        assert impacted_files[0].head_name == "fileA"

graphql_api/types/commit/commit.py

@@ -171,7 +171,7 @@ def resolve_path_contents(commit: Commit, info, path: str = None, filters=None):
         filters = {}
     search_value = filters.get("search_value")
     display_type = filters.get("display_type")
-    flags = filters.get("flags") or []
+    flags = filters.get("flags", [])
 
     if flags and not set(flags) & set(commit_report.flags):
         return UnknownFlags()

graphql_api/types/comparison/comparison.py

@@ -5,6 +5,7 @@
 
 import services.components as components_service
 from codecov.db import sync_to_async
+from compare.commands.compare.compare import CompareCommands
 from compare.models import ComponentComparison, FlagComparison
 from graphql_api.actions.flags import get_flag_comparisons
 from graphql_api.dataloader.commit import CommitLoader
@@ -35,10 +36,12 @@ def resolve_state(comparison: ComparisonReport, info) -> str:
 @convert_kwargs_to_snake_case
 @sync_to_async
 def resolve_impacted_files(
-    comparison: ComparisonReport, info, filters=None
+    comparison_report: ComparisonReport, info, filters=None
 ) -> List[ImpactedFile]:
-    command = info.context["executor"].get_command("compare")
-    return command.fetch_impacted_files(comparison, filters)
+    command: CompareCommands = info.context["executor"].get_command("compare")
+    comparison: Comparison = info.context.get("comparison", None)
+
+    return command.fetch_impacted_files(comparison_report, comparison, filters)
 
 
 @comparison_bindable.field("impactedFilesCount")

graphql_api/types/inputs/impacted_files_filters.graphql

@@ -1,6 +1,7 @@
 input ImpactedFilesFilters {
   ordering: ImpactedFilesOrdering
   hasUnintendedChanges: Boolean
+  flags: [String!]
 }
 
 input ImpactedFilesOrdering {