Skip to content

Commit fe46d75

Browse files
Victor Hugo Vianna SilvaLUCI CQ
authored andcommitted
Add presubmit warning to update references to (re)moved OWNERS files
file:// references in OWNERS files can become invalid when the file is moved or deleted. Bug: 1385205 Change-Id: Icf1a65b3d96d6ad298eac6645d8f692cb09dc75a Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4029143 Auto-Submit: Victor Vianna <[email protected]> Reviewed-by: Josip Sokcevic <[email protected]> Commit-Queue: Victor Vianna <[email protected]>
1 parent fadcbfd commit fe46d75

File tree

3 files changed

+53
-1
lines changed

3 files changed

+53
-1
lines changed

presubmit_canned_checks.py

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2081,3 +2081,21 @@ def CheckForMatch(affected_file, line_num, line, term, message, error):
20812081
output_api.PresubmitError('Banned non-inclusive language was used.\n' +
20822082
'\n'.join(errors)))
20832083
return result
2084+
2085+
2086+
def CheckUpdateOwnersFileReferences(input_api, output_api):
2087+
"""Checks whether an OWNERS file is being (re)moved and if so asks the
2088+
contributor to update any file:// references to it."""
2089+
files = []
2090+
# AffectedFiles() includes owner files, not AffectedSourceFiles().
2091+
for f in input_api.AffectedFiles():
2092+
# Moved files appear here as one deletion and one addition.
2093+
if f.LocalPath().endswith('OWNERS') and f.Action() == 'D':
2094+
files.append(f.LocalPath())
2095+
if not files:
2096+
return []
2097+
return [
2098+
output_api.PresubmitPromptWarning(
2099+
'OWNERS files being moved/removed, please update any file:// ' +
2100+
'references to them in other OWNERS files', files)
2101+
]

presubmit_canned_checks_test.py

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -241,5 +241,39 @@ def testCheckDescriptionUsesColonInsteadOfEquals(self):
241241
self.assertEqual(0, len(errors))
242242

243243

244+
class CheckUpdateOwnersFileReferences(unittest.TestCase):
245+
def testShowsWarningIfDeleting(self):
246+
input_api = MockInputApi()
247+
input_api.files = [
248+
MockFile('foo/OWNERS', [], [], action='D'),
249+
]
250+
results = presubmit_canned_checks.CheckUpdateOwnersFileReferences(
251+
input_api, MockOutputApi())
252+
self.assertEqual(1, len(results))
253+
self.assertEqual('warning', results[0].type)
254+
self.assertEqual(1, len(results[0].items))
255+
256+
def testShowsWarningIfMoving(self):
257+
input_api = MockInputApi()
258+
input_api.files = [
259+
MockFile('new_directory/OWNERS', [], [], action='A'),
260+
MockFile('old_directory/OWNERS', [], [], action='D'),
261+
]
262+
results = presubmit_canned_checks.CheckUpdateOwnersFileReferences(
263+
input_api, MockOutputApi())
264+
self.assertEqual(1, len(results))
265+
self.assertEqual('warning', results[0].type)
266+
self.assertEqual(1, len(results[0].items))
267+
268+
def testNoWarningIfAdding(self):
269+
input_api = MockInputApi()
270+
input_api.files = [
271+
MockFile('foo/OWNERS', [], [], action='A'),
272+
]
273+
results = presubmit_canned_checks.CheckUpdateOwnersFileReferences(
274+
input_api, MockOutputApi())
275+
self.assertEqual(0, len(results))
276+
277+
244278
if __name__ == '__main__':
245279
unittest.main()

presubmit_canned_checks_test_mocks.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ def __init__(self):
7979
def CreateMockFileInPath(self, f_list):
8080
self.os_path.exists = lambda x: x in f_list
8181

82-
def AffectedFiles(self, file_filter=None, include_deletes=False):
82+
def AffectedFiles(self, file_filter=None, include_deletes=True):
8383
for file in self.files: # pylint: disable=redefined-builtin
8484
if file_filter and not file_filter(file):
8585
continue

0 commit comments

Comments
 (0)