diff options
| -rw-r--r-- | PRESUBMIT.py | 35 | ||||
| -rwxr-xr-x | PRESUBMIT_test.py | 37 |
2 files changed, 58 insertions, 14 deletions
diff --git a/PRESUBMIT.py b/PRESUBMIT.py index 6558685..efb665a 100644 --- a/PRESUBMIT.py +++ b/PRESUBMIT.py @@ -727,6 +727,26 @@ def _CheckNoAbbreviationInPngFileName(input_api, output_api): return results +def _DepsFilesToCheck(re, changed_lines): + """Helper method for _CheckAddedDepsHaveTargetApprovals. Returns + a set of DEPS entries that we should look up.""" + results = set() + + # This pattern grabs the path without basename in the first + # parentheses, and the basename (if present) in the second. It + # relies on the simple heuristic that if there is a basename it will + # be a header file ending in ".h". + pattern = re.compile( + r"""['"]\+([^'"]+?)(/[a-zA-Z0-9_]+\.h)?['"].*""") + for changed_line in changed_lines: + m = pattern.match(changed_line) + if m: + path = m.group(1) + if not (path.startswith('grit/') or path == 'grit'): + results.add('%s/DEPS' % m.group(1)) + return results + + def _CheckAddedDepsHaveTargetApprovals(input_api, output_api): """When a dependency prefixed with + is added to a DEPS file, we want to make sure that the change is reviewed by an OWNER of the @@ -743,20 +763,7 @@ def _CheckAddedDepsHaveTargetApprovals(input_api, output_api): if not changed_lines: return [] - virtual_depended_on_files = set() - # This pattern grabs the path without basename in the first - # parentheses, and the basename (if present) in the second. It - # relies on the simple heuristic that if there is a basename it will - # be a header file ending in ".h". - pattern = input_api.re.compile( - r"""['"]\+([^'"]+?)(/[a-zA-Z0-9_]+\.h)?['"].*""") - for changed_line in changed_lines: - m = pattern.match(changed_line) - if m: - path = m.group(1) - if not path.startswith('grit/'): - virtual_depended_on_files.add('%s/DEPS' % m.group(1)) - + virtual_depended_on_files = _DepsFilesToCheck(input_api.re, changed_lines) if not virtual_depended_on_files: return [] diff --git a/PRESUBMIT_test.py b/PRESUBMIT_test.py index ed7e190..1f4a1ef 100755 --- a/PRESUBMIT_test.py +++ b/PRESUBMIT_test.py @@ -369,5 +369,42 @@ class InvalidOSMacroNamesTest(unittest.TestCase): self.assertEqual(0, len(errors)) +class CheckAddedDepsHaveTetsApprovalsTest(unittest.TestCase): + def testDepsFilesToCheck(self): + changed_lines = [ + '"+breakpad",', + '"+chrome/installer",', + '"+chrome/plugin/chrome_content_plugin_client.h",', + '"+chrome/utility/chrome_content_utility_client.h",', + '"+chromeos/chromeos_paths.h",', + '"+components/breakpad",', + '"+components/nacl/common",', + '"+content/public/browser/render_process_host.h",', + '"+grit", # For generated headers', + '"+grit/generated_resources.h",', + '"+grit/",', + '"+policy", # For generated headers and source', + '"+sandbox",', + '"+tools/memory_watcher",', + '"+third_party/lss/linux_syscall_support.h",', + ] + files_to_check = PRESUBMIT._DepsFilesToCheck(re, changed_lines) + expected = set([ + 'breakpad/DEPS', + 'chrome/installer/DEPS', + 'chrome/plugin/DEPS', + 'chrome/utility/DEPS', + 'chromeos/DEPS', + 'components/breakpad/DEPS', + 'components/nacl/common/DEPS', + 'content/public/browser/DEPS', + 'policy/DEPS', + 'sandbox/DEPS', + 'tools/memory_watcher/DEPS', + 'third_party/lss/DEPS', + ]) + self.assertEqual(expected, files_to_check); + + if __name__ == '__main__': unittest.main() |