Fixing the PRESUBMIT include check some more.

PRESUBMIT.py

 # Copyright (c) 2012 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 """Top-level presubmit script for Chromium.
 
 See http://dev.chromium.org/developers/how-tos/depottools/presubmit-scripts
 for more details about the presubmit API built into gcl.
 """
 
@@ skipping 536 matching lines @@
     previous_line = line
     previous_line_num = line_num
 
   warnings = []
   for (line_num, previous_line_num) in problem_linenums:
     if line_num in changed_linenums or previous_line_num in changed_linenums:
       warnings.append('    %s:%d' % (file_path, line_num))
   return warnings
 
 
-def _CheckIncludeOrderInFile(input_api, output_api, f, is_source,
-                             changed_linenums):
+def _CheckIncludeOrderInFile(input_api, f, is_source, changed_linenums):
   """Checks the #include order for the given file f."""
 
-  include_pattern = input_api.re.compile(r'\s*#include.*')
+  system_include_pattern = input_api.re.compile(r'\s*#include \<.*')
+  custom_include_pattern = input_api.re.compile(r'\s*#include "(?P<FILE>.*)"')
   if_pattern = input_api.re.compile(r'\s*#if.*')
   endif_pattern = input_api.re.compile(r'\s*#endif.*')
 
   contents = f.NewContents()
   warnings = []
   line_num = 0
 
-  # Handle the special first include for source files.
+  # Handle the special first include for source files. If the header file is
+  # some/path/file.h, the corresponding source file can be some/path/file.cc,
+  # some/other/path/file.cc, some/path/file_platform.cc etc. It's also possible
+  # that no special first include exists.
   if is_source:
     for line in contents:
       line_num += 1
-      if include_pattern.match(line):
-        # The file name for the first include needs to be the same as for the
-        # file checked; the path can differ.
-        expected_ending = (
-            input_api.os_path.basename(f.LocalPath()).replace('.cc', '.h') +
-            '"')
-        if not line.endswith(expected_ending):
-          # Maybe there was no special first include, and that's fine. Process
-          # the line again along with the normal includes.
+      if system_include_pattern.match(line):
+        # No special first include -> process the line again along with normal
+        # includes.
+        line_num -= 1
+        break
+      match = custom_include_pattern.match(line)
+      if match:
+        match_dict = match.groupdict()
+        header_basename = input_api.os_path.basename(
+            match_dict['FILE']).replace('.h', '')
+        if header_basename not in input_api.os_path.basename(f.LocalPath()):
+          # No special first include -> process the line again along with normal
+          # includes.
           line_num -= 1
         break
 
   # Split into scopes: Each region between #if and #endif is its own scope.
   scopes = []
   current_scope = []
   for line in contents[line_num:]:
     line_num += 1
     if if_pattern.match(line) or endif_pattern.match(line):
       scopes.append(current_scope)
       current_scope = []
-    elif include_pattern.match(line):
+    elif (system_include_pattern.match(line) or
+          custom_include_pattern.match(line)):
       current_scope.append((line_num, line))
   scopes.append(current_scope)
 
   for scope in scopes:
     warnings.extend(_CheckIncludeOrderForScope(scope, input_api, f.LocalPath(),
                                                changed_linenums))
   return warnings
 
 
 def _CheckIncludeOrder(input_api, output_api):
   """Checks that the #include order is correct.
 
   1. The corresponding header for source files.
   2. C system files in alphabetical order
   3. C++ system files in alphabetical order
   4. Project's .h files in alphabetical order
 
   Each region between #if and #endif follows these rules separately.
   """
 
   warnings = []
   for f in input_api.AffectedFiles():
     changed_linenums = set([line_num for line_num, _ in f.ChangedContents()])
     if f.LocalPath().endswith('.cc'):
-      warnings = _CheckIncludeOrderInFile(input_api, output_api, f, True,
-                                          changed_linenums)
+      warnings.extend(_CheckIncludeOrderInFile(input_api, f, True,
+                                               changed_linenums))
     elif f.LocalPath().endswith('.h'):
-      warnings = _CheckIncludeOrderInFile(input_api, output_api, f, False,
-                                          changed_linenums)
+      warnings.extend(_CheckIncludeOrderInFile(input_api, f, False,
+                                               changed_linenums))
 
   results = []
   if warnings:
     results.append(output_api.PresubmitPromptWarning(_INCLUDE_ORDER_WARNING,
                                                      warnings))
   return results
 
 
 def _CommonChecks(input_api, output_api):
   """Checks common to both upload and commit."""
   results = []
   results.extend(input_api.canned_checks.PanProjectChecks(
       input_api, output_api, excluded_paths=_EXCLUDED_PATHS))
   results.extend(_CheckAuthorizedAuthor(input_api, output_api))
   results.extend(
     _CheckNoProductionCodeUsingTestOnlyFunctions(input_api, output_api))
   results.extend(_CheckNoIOStreamInHeaders(input_api, output_api))
   results.extend(_CheckNoUNIT_TESTInSourceFiles(input_api, output_api))
   results.extend(_CheckNoNewWStrings(input_api, output_api))
   results.extend(_CheckNoDEPSGIT(input_api, output_api))
   results.extend(_CheckNoBannedFunctions(input_api, output_api))
   results.extend(_CheckNoPragmaOnce(input_api, output_api))
   results.extend(_CheckNoTrinaryTrueFalse(input_api, output_api))
   results.extend(_CheckUnwantedDependencies(input_api, output_api))
   results.extend(_CheckFilePermissions(input_api, output_api))
   results.extend(_CheckNoAuraWindowPropertyHInHeaders(input_api, output_api))
   results.extend(_CheckIncludeOrder(input_api, output_api))
+
+  if any('PRESUBMIT.py' == f.LocalPath() for f in input_api.AffectedFiles()):
+    results.extend(input_api.canned_checks.RunUnitTestsInDirectory(
+        input_api, output_api,
+        input_api.PresubmitLocalPath(),
+        whitelist=[r'.+_test\.py$']))

[Isaac (away), 2013/01/06 10:35:48]

This seems overly broad.  Could we change this to run PRESUBMIT_test.py instead
of any python file that ends with test?

[M-A Ruel, 2013/01/07 15:27:04]

Oh right, it should be r'[^\\/]+_test\.py$'.

   return results
 
 
 def _CheckSubversionConfig(input_api, output_api):
   """Verifies the subversion config file is correctly setup.
 
   Checks that autoprops are enabled, returns an error otherwise.
   """
   join = input_api.os_path.join
   if input_api.platform == 'win32':
@@ skipping 127 matching lines @@
 
   # Match things like path/aura/file.cc and path/file_aura.cc.
   # Same for ash and chromeos.
   if any(re.search('[/_](ash|aura)', f) for f in files):
     trybots += ['linux_chromeos_clang:compile', 'win_aura',
                 'linux_chromeos_asan']
   elif any(re.search('[/_]chromeos', f) for f in files):
     trybots += ['linux_chromeos_clang:compile', 'linux_chromeos_asan']
 
   return trybots