Skip to content
This repository was archived by the owner on Jun 13, 2025. It is now read-only.

Commit a9b4f6a

Browse files
adrian-codecovadrian-codecov
authored
Improve performance time for ImpactedFiles resolver (#1181)
1 parent bd2fea7 commit a9b4f6a

File tree

6 files changed

+28
-158
lines changed

6 files changed

+28
-158
lines changed

‎compare/commands/compare/interactors/fetch_impacted_files.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ def _apply_filters(
4040
components_paths = []
4141
components_flags = []
4242

43-
head_commit_report = comparison.head_report
43+
head_commit_report = comparison.head_report_without_applied_diff
4444
if components_filter:
4545
all_components = components.commit_components(
4646
comparison.head_commit, comparison.user

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

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -564,7 +564,10 @@ def test_impacted_files_filtered_by_unintended_changes_set_to_false(
564564
impacted_files = self.execute(None, self.comparison_report, comparison, filters)
565565
assert [file.head_name for file in impacted_files] == ["fileA", "fileB"]
566566

567-
@patch("services.comparison.Comparison.head_report", new_callable=PropertyMock)
567+
@patch(
568+
"services.comparison.Comparison.head_report_without_applied_diff",
569+
new_callable=PropertyMock,
570+
)
568571
@patch("shared.api_archive.archive.ArchiveService.read_file")
569572
def test_impacted_files_filtered_by_flags_and_commit_comparison_for_pull(
570573
self, read_file, build_report_from_commit_mock
@@ -605,7 +608,10 @@ def test_impacted_files_filtered_by_flags_and_commit_comparison_for_pull(
605608
assert len(impacted_files) == 1
606609
assert impacted_files[0].head_name == "fileA"
607610

608-
@patch("services.comparison.Comparison.head_report", new_callable=PropertyMock)
611+
@patch(
612+
"services.comparison.Comparison.head_report_without_applied_diff",
613+
new_callable=PropertyMock,
614+
)
609615
@patch("shared.api_archive.archive.ArchiveService.read_file")
610616
def test_impacted_files_filtered_by_flags_and_commit_comparison_for_parent_commit(
611617
self, read_file, build_report_from_commit_mock
@@ -639,7 +645,10 @@ def test_impacted_files_filtered_by_flags_and_commit_comparison_for_parent_commi
639645
assert impacted_files[0].head_name == "fileA"
640646

641647
@patch("services.components.commit_components")
642-
@patch("services.comparison.Comparison.head_report", new_callable=PropertyMock)
648+
@patch(
649+
"services.comparison.Comparison.head_report_without_applied_diff",
650+
new_callable=PropertyMock,
651+
)
643652
@patch("shared.api_archive.archive.ArchiveService.read_file")
644653
def test_impacted_files_filtered_by_components_and_commit_comparison_for_parent_commit(
645654
self, read_file, build_report_from_commit_mock, commit_components_mock
@@ -682,7 +691,10 @@ def test_impacted_files_filtered_by_components_and_commit_comparison_for_parent_
682691
assert impacted_files[0].head_name == "fileA.py"
683692

684693
@patch("services.components.commit_components")
685-
@patch("services.comparison.Comparison.head_report", new_callable=PropertyMock)
694+
@patch(
695+
"services.comparison.Comparison.head_report_without_applied_diff",
696+
new_callable=PropertyMock,
697+
)
686698
@patch("shared.api_archive.archive.ArchiveService.read_file")
687699
def test_impacted_files_filtered_by_components_using_flags(
688700
self, read_file, build_report_from_commit_mock, commit_components_mock
@@ -731,7 +743,10 @@ def test_impacted_files_filtered_by_components_using_flags(
731743
assert impacted_files[1].head_name == "fileB.js"
732744

733745
@patch("services.components.commit_components")
734-
@patch("services.comparison.Comparison.head_report", new_callable=PropertyMock)
746+
@patch(
747+
"services.comparison.Comparison.head_report_without_applied_diff",
748+
new_callable=PropertyMock,
749+
)
735750
@patch("shared.api_archive.archive.ArchiveService.read_file")
736751
def test_impacted_files_filtered_by_components_and_flags_commit_comparison_for_parent_commit(
737752
self, read_file, build_report_from_commit_mock, commit_components_mock

‎graphql_api/tests/test_impacted_file.py‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,6 @@
4646
isNewFile
4747
isRenamedFile
4848
isDeletedFile
49-
isCriticalFile
5049
baseCoverage {
5150
percentCovered
5251
}
@@ -422,7 +421,6 @@ def test_fetch_impacted_files(self, read_file):
422421
"isNewFile": False,
423422
"isRenamedFile": False,
424423
"isDeletedFile": False,
425-
"isCriticalFile": False,
426424
"baseCoverage": {
427425
"percentCovered": 41.666666666666664
428426
},
@@ -440,7 +438,6 @@ def test_fetch_impacted_files(self, read_file):
440438
"isNewFile": False,
441439
"isRenamedFile": False,
442440
"isDeletedFile": False,
443-
"isCriticalFile": False,
444441
"baseCoverage": {
445442
"percentCovered": 41.666666666666664
446443
},

‎graphql_api/tests/test_pull_comparison.py‎

Lines changed: 7 additions & 140 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,13 @@ def setUp(self):
5757
self.head_report = self.head_report_patcher.start()
5858
self.head_report.return_value = None
5959
self.addCleanup(self.head_report_patcher.stop)
60+
self.head_report_without_diff_patcher = patch(
61+
"services.comparison.Comparison.head_report_without_applied_diff",
62+
new_callable=PropertyMock,
63+
)
64+
self.head_report_without_diff = self.head_report_without_diff_patcher.start()
65+
self.head_report_without_diff.return_value = None
66+
self.addCleanup(self.head_report_without_diff_patcher.stop)
6067
self.base_report_patcher = patch(
6168
"services.comparison.Comparison.base_report", new_callable=PropertyMock
6269
)
@@ -506,146 +513,6 @@ def test_pull_comparison_impacted_files(
506513
},
507514
}
508515

509-
@patch(
510-
"services.comparison.ComparisonReport.files",
511-
new_callable=PropertyMock,
512-
)
513-
def test_pull_comparison_is_critical_file(self, files_mock):
514-
TestImpactedFile = namedtuple("TestImpactedFile", ["base_name", "head_name"])
515-
516-
files_mock.return_value = [
517-
TestImpactedFile(
518-
base_name="foo.py",
519-
head_name="bar.py",
520-
),
521-
TestImpactedFile(
522-
base_name=None,
523-
head_name="baz.py",
524-
),
525-
]
526-
527-
query = """
528-
pullId
529-
compareWithBase {
530-
... on Comparison {
531-
impactedFiles(filters:{}) {
532-
... on ImpactedFiles {
533-
results {
534-
baseName
535-
headName
536-
isCriticalFile
537-
}
538-
}
539-
... on UnknownFlags {
540-
message
541-
}
542-
}
543-
}
544-
}
545-
"""
546-
547-
res = self._request(query)
548-
assert res == {
549-
"pullId": self.pull.pullid,
550-
"compareWithBase": {
551-
"impactedFiles": {
552-
"results": [
553-
{
554-
"baseName": "foo.py",
555-
"headName": "bar.py",
556-
"isCriticalFile": False,
557-
},
558-
{
559-
"baseName": None,
560-
"headName": "baz.py",
561-
"isCriticalFile": False,
562-
},
563-
]
564-
},
565-
},
566-
}
567-
568-
@patch(
569-
"services.comparison.ComparisonReport.files",
570-
new_callable=PropertyMock,
571-
)
572-
def test_pull_comparison_is_critical_file_returns_false_through_repositories(
573-
self, files_mock
574-
):
575-
TestImpactedFile = namedtuple("TestImpactedFile", ["base_name", "head_name"])
576-
577-
files_mock.return_value = [
578-
TestImpactedFile(
579-
base_name="foo.py",
580-
head_name="bar.py",
581-
),
582-
]
583-
584-
query = """
585-
query {
586-
me {
587-
owner {
588-
repositories (first: 1) {
589-
edges {
590-
node {
591-
pull (id: %s) {
592-
pullId
593-
compareWithBase {
594-
... on Comparison {
595-
impactedFiles(filters:{}) {
596-
... on ImpactedFiles {
597-
results {
598-
baseName
599-
headName
600-
isCriticalFile
601-
}
602-
}
603-
... on UnknownFlags {
604-
message
605-
}
606-
}
607-
}
608-
}
609-
}
610-
}
611-
}
612-
}
613-
}
614-
}
615-
}
616-
"""
617-
618-
data = self.gql_request(query % (self.pull.pullid), owner=self.owner)
619-
620-
assert data == {
621-
"me": {
622-
"owner": {
623-
"repositories": {
624-
"edges": [
625-
{
626-
"node": {
627-
"pull": {
628-
"pullId": 2,
629-
"compareWithBase": {
630-
"impactedFiles": {
631-
"results": [
632-
{
633-
"baseName": "foo.py",
634-
"headName": "bar.py",
635-
"isCriticalFile": False,
636-
}
637-
]
638-
},
639-
},
640-
}
641-
}
642-
}
643-
]
644-
}
645-
}
646-
}
647-
}
648-
649516
@patch(
650517
"services.comparison.PullRequestComparison.get_file_comparison",
651518
)

‎graphql_api/types/impacted_file/impacted_file.graphql‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ type ImpactedFile {
55
isNewFile: Boolean!
66
isRenamedFile: Boolean!
77
isDeletedFile: Boolean!
8-
isCriticalFile: Boolean! @deprecated(reason: "Impact Analysis is deprecated.")
98
baseCoverage: CoverageTotals
109
headCoverage: CoverageTotals
1110
patchCoverage: CoverageTotals

‎graphql_api/types/impacted_file/impacted_file.py‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -133,14 +133,6 @@ def resolve_misses_count(impacted_file: ImpactedFile, info) -> int:
133133
return impacted_file.misses_count
134134

135135

136-
@sentry_sdk.trace
137-
@impacted_file_bindable.field("isCriticalFile")
138-
@sync_to_async
139-
def resolve_is_critical_file(impacted_file: ImpactedFile, info) -> bool:
140-
"""DEPRECATED. Returning False for all responses."""
141-
return False
142-
143-
144136
impacted_files_result_bindable = UnionType("ImpactedFilesResult")
145137

146138

0 commit comments

Comments
 (0)