Skip to content
This repository was archived by the owner on Jun 13, 2025. It is now read-only.
4 changes: 3 additions & 1 deletion graphql_api/tests/test_impacted_file.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import hashlib
from dataclasses import dataclass
from dataclasses import dataclass, field
from typing import Callable
from unittest.mock import PropertyMock, patch

from django.test import TransactionTestCase
Expand Down Expand Up @@ -211,6 +212,7 @@
class MockSegment:
has_diff_changes: bool = False
has_unintended_changes: bool = False
remove_unintended_changes: Callable[[], None] = field(default=lambda: None)


class MockFileComparison(object):
Expand Down
8 changes: 6 additions & 2 deletions graphql_api/types/impacted_file/impacted_file.py
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,12 @@ def resolve_segments(
# segments with no diff changes and at least 1 unintended change
segments = [segment for segment in segments if segment.has_unintended_changes]
elif filters.get("has_unintended_changes") is False:
# segments with at least 1 diff change
segments = [segment for segment in segments if segment.has_diff_changes]
new_segments = []
for segment in segments:
if segment.has_diff_changes:
segment.remove_unintended_changes()
new_segments.append(segment)
segments = new_segments

return SegmentComparisons(results=segments)

Expand Down
9 changes: 9 additions & 0 deletions services/comparison.py
Original file line number Diff line number Diff line change
Expand Up @@ -481,6 +481,15 @@
return True
return False

def remove_unintended_changes(self):
filtered = []
for line in self._lines:
base_cov = line.coverage["base"]
head_cov = line.coverage["head"]
if (line.added or line.removed) or (base_cov == head_cov):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This logic says if line is in the diff or coverage on the line has not changed, then it's unintended (coverage change on non-diff line) and we should filter it out. Correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct yep

filtered.append(line)
self._lines = filtered

Check warning on line 491 in services/comparison.py

View check run for this annotation

Codecov Notifications / codecov/patch

services/comparison.py#L488-L491

Added lines #L488 - L491 were not covered by tests


class FileComparison:
def __init__(
Expand Down
16 changes: 16 additions & 0 deletions services/tests/test_comparison.py
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Just want to confirm that these indirect changes will still show in the indirect changes tab?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Solid lgtm

Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
LineComparison,
MissingComparisonReport,
PullRequestComparison,
Segment,
)

# Pulled from shared.django_apps.core.tests.factories.CommitFactory files.
Expand Down Expand Up @@ -1784,6 +1785,21 @@ def test_file_has_changes(self):
)
assert file.has_changes is True

def test_remove_unintended_changes(self):
lines = [
LineComparison(1, 1, 1, 1, "line1", False),
LineComparison(1, 0, 2, 2, "line2", False),
LineComparison(0, 0, None, 3, "+line3", True),
LineComparison(0, 0, 4, None, "-line4", True),
LineComparison(1, 0, 5, 5, "line5", False),
]

segment = Segment(lines)
segment.remove_unintended_changes()

assert len(segment.lines) == 3
assert [line.value for line in segment.lines] == ["line1", "+line3", "-line4"]


class CommitComparisonTests(TestCase):
def setUp(self):
Expand Down
Loading