From 2978c9420a7ac3224c607eb191f10ba291f6c4b1 Mon Sep 17 00:00:00 2001 From: Aleksey Khoroshilov Date: Mon, 13 Jun 2022 16:14:12 +0000 Subject: [PATCH] Fix PRESUBMIT_test.py lookup when inherit-review-settings-ok is applied. 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 Auto-Submit: Aleksey Khoroshilov Commit-Queue: Bruce Dawson Cr-Commit-Position: refs/heads/main@{#1013502} --- PRESUBMIT.py | 52 +++++++++++++++++++++++++++------------------------- 1 file changed, 27 insertions(+), 25 deletions(-) diff --git a/PRESUBMIT.py b/PRESUBMIT.py index b5237cbc9988aa..54a4876c2dd5e8 100644 --- a/PRESUBMIT.py +++ b/PRESUBMIT.py @@ -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