0

PRESUBMIT: Fix AffectedSourceFile's mock.

Context:
--------

I was surprised @clamy was forced to use `DanglingUntriaged-notes` while
moving a file. This is unexpected. Especially given this behavior was
tested [0].

Root Cause:
-----------

The root cause is a divergence in between the depot_tool implementation
[1][2] using `include_deletes=False` and Chromes's mocks implementation
[3][4] using `include_deletes=True`.

The test was correctly written, but the environment not correctly
simulated ;-(
This patches reconcile the two.

[0]: https://source.chromium.org/chromium/chromium/src/+/main:PRESUBMIT_test.py;l=5144-5167
[1]: https://source.chromium.org/chromium/chromium/tools/depot_tools/+/main:presubmit_support.py;l=810-817
[2]: https://source.chromium.org/chromium/chromium/tools/depot_tools/+/main:presubmit_support.py;l=1377?q=%22def%20AffectedSourceFiles%22%20-f:third_party
[3]: https://source.chromium.org/chromium/chromium/src/+/main:PRESUBMIT_test_mocks.py;l=106-107
[4]: https://source.chromium.org/chromium/chromium/src/+/main:PRESUBMIT_test_mocks.py;l=273-275

Bug: chromium:1395297
Change-Id: I646adbe9e53dfe606bfcfacd41b4f794d49042be
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5447014
Reviewed-by: Dominic Battre <battre@chromium.org>
Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1291133}
This commit is contained in:
Arthur Sonzogni
2024-04-23 07:53:04 +00:00
committed by Chromium LUCI CQ
parent 5f29b7111c
commit c66e9c85b6
2 changed files with 6 additions and 6 deletions

@ -2452,11 +2452,11 @@ def CheckNoDISABLETypoInTests(input_api, output_api):
r'^\s*TEST[^(]*\([a-zA-Z0-9_]+,\s*DISABLE_[a-zA-Z0-9_]+\)',
input_api.re.MULTILINE)
for f in input_api.AffectedFiles(False):
for f in input_api.AffectedFiles(include_deletes=False):
if not 'test' in f.LocalPath() or not f.LocalPath().endswith('.cc'):
continue
# Search for MABYE_, DISABLE_ pairs.
# Search for MAYBE_, DISABLE_ pairs.
disable_lines = {} # Maps of test name to line number.
maybe_lines = {}
for line_num, line in f.ChangedContents():
@ -2511,7 +2511,7 @@ def CheckForgettingMAYBEInTests(input_api, output_api):
# Read the entire files. We can't just read the affected lines, forgetting to
# add MAYBE_ on a change would not show up otherwise.
for f in input_api.AffectedFiles(False):
for f in input_api.AffectedFiles(include_deletes=False):
if not 'test' in f.LocalPath() or not f.LocalPath().endswith('.cc'):
continue
contents = input_api.ReadFile(f)
@ -7233,7 +7233,7 @@ def CheckNoJsInIos(input_api, output_api):
deleted_files = []
# Collect filenames of all removed JS files.
for f in input_api.AffectedSourceFiles(_FilterFile):
for f in input_api.AffectedFiles(file_filter=_FilterFile):
local_path = f.LocalPath()
if input_api.os_path.splitext(local_path)[1] == '.js' and f.Action() == 'D':
@ -7331,7 +7331,7 @@ def CheckDanglingUntriaged(input_api, output_api):
)
count = 0
for f in input_api.AffectedSourceFiles(FilterFile):
for f in input_api.AffectedFiles(file_filter=FilterFile):
count -= sum([l.count("DanglingUntriaged") for l in f.OldContents()])
count += sum([l.count("DanglingUntriaged") for l in f.NewContents()])

@ -104,7 +104,7 @@ class MockInputApi(object):
yield (af, line[0], line[1])
def AffectedSourceFiles(self, file_filter=None):
return self.AffectedFiles(file_filter=file_filter)
return self.AffectedFiles(file_filter=file_filter, include_deletes=False)
def FilterSourceFile(self, file,
files_to_check=(), files_to_skip=()):