Skip to content

Commit

Permalink
Fix PRESUBMIT_test.py lookup when inherit-review-settings-ok is applied.
Browse files Browse the repository at this point in the history
When `git cl presubmit` is executed from a nested repository which
contains `inherit-review-settings-ok` file, PRESUBMIT_test.py lookup is
made via LocalPath() which doesn't include input_api.RepositoryRoot() into account. In this case `full_path` is pointing directly at input_api.PresubmitLocalPath() which leads to PRESUBMIT_test.py being run from a root directory instead of a nested one where a modified PRESUBMIT.py is actually located.

Reproduce case:
1. cd third_party/pdfium
2. create inherit-review-settings-ok file
3. run git cl presubmit --force -v --files PRESUBMIT.py
Without this patch this is printed:
  vpython3.bat c:\src\chromium\src\PRESUBMIT_test.py
With this patch this is printed:
  vpython3.bat c:\src\chromium\src\third_party\pdfium\PRESUBMIT_test.py

This CL changes the way PRESUBMIT_test.py is located by using absolute
path instead; the CL also improves the way PRESUBMIT.py files are found in the change.

Change-Id: I9e321ae0693242a9f3bdee94484da282d570954b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3679163
Reviewed-by: Bruce Dawson <brucedawson@chromium.org>
Auto-Submit: Aleksey Khoroshilov <akhoroshilov@brave.com>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1013502}
  • Loading branch information
goodov authored and Chromium LUCI CQ committed Jun 13, 2022
1 parent 8968239 commit 2978c94
Showing 1 changed file with 27 additions and 25 deletions.
52 changes: 27 additions & 25 deletions PRESUBMIT.py
Original file line number Diff line number Diff line change
Expand Up @@ -4663,31 +4663,33 @@ def ChecksCommon(input_api, output_api):
],
non_inclusive_terms=_NON_INCLUSIVE_TERMS))

for f in input_api.AffectedFiles():
path, name = input_api.os_path.split(f.LocalPath())
if name == 'PRESUBMIT.py':
full_path = input_api.os_path.join(input_api.PresubmitLocalPath(),
path)
test_file = input_api.os_path.join(path, 'PRESUBMIT_test.py')
if f.Action() != 'D' and input_api.os_path.exists(test_file):
# The PRESUBMIT.py file (and the directory containing it) might
# have been affected by being moved or removed, so only try to
# run the tests if they still exist.
use_python3 = False
with open(f.LocalPath()) as fp:
use_python3 = any(
line.startswith('USE_PYTHON3 = True')
for line in fp.readlines())

results.extend(
input_api.canned_checks.RunUnitTestsInDirectory(
input_api,
output_api,
full_path,
files_to_check=[r'^PRESUBMIT_test\.py$'],
run_on_python2=not use_python3,
run_on_python3=use_python3,
skip_shebang_check=True))
presubmit_py_filter = lambda f: input_api.FilterSourceFile(
f, files_to_check=[r'PRESUBMIT\.py$'])
for f in input_api.AffectedFiles(include_deletes=False,
file_filter=presubmit_py_filter):
full_path = input_api.os_path.dirname(f.AbsoluteLocalPath())
test_file = input_api.os_path.join(full_path, 'PRESUBMIT_test.py')
# The PRESUBMIT.py file (and the directory containing it) might have
# been affected by being moved or removed, so only try to run the tests
# if they still exist.
if not input_api.os_path.exists(test_file):
continue

use_python3 = False
with open(f.LocalPath()) as fp:
use_python3 = any(
line.startswith('USE_PYTHON3 = True')
for line in fp.readlines())

results.extend(
input_api.canned_checks.RunUnitTestsInDirectory(
input_api,
output_api,
full_path,
files_to_check=[r'^PRESUBMIT_test\.py$'],
run_on_python2=not use_python3,
run_on_python3=use_python3,
skip_shebang_check=True))
return results


Expand Down

0 comments on commit 2978c94

Please sign in to comment.