Add a new argument to skip shebang checks on python3

A bug in the logic enabling tests to be run on python3 resulted in
tests never being run in python3 unless they also have a shebang line
pointing to python3. Since this bug was introduced, effectively, most
tests that are supposed to be run on python3 do not run at all. Since
then a lot of python3 tests have regressed in Chromium.

In order to fix the issue moving forward, this introduces a new
argument that skips the shebang check to run a test in python3. Tests
will be re-enabled one by one until every single instance in Chromium
has been updated.

Bug: 1223478
Change-Id: I91a0688c6f4d9b4fbf18e3d446366cded8c7f2f1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2986400
Reviewed-by: Dirk Pranke <dpranke@google.com>
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
Reviewed-by: Ben Pastene <bpastene@chromium.org>
Commit-Queue: Fabrice de Gans <fdegans@chromium.org>

presubmit_canned_checks.py

@@ -648,6 +648,7 @@ def GetUnitTestsInDirectory(input_api,
                             env=None,
                             run_on_python2=True,
                             run_on_python3=True,
+                            skip_shebang_check=False,
                             allowlist=None,
                             blocklist=None):
   """Lists all files in a directory and runs them. Doesn't recurse.
@@ -682,13 +683,17 @@ def GetUnitTestsInDirectory(input_api,
           'Out of %d files, found none that matched c=%r, s=%r in directory %s'
           % (found, files_to_check, files_to_skip, directory))
     ]
-  return GetUnitTests(
+  return GetUnitTests(input_api, output_api, unit_tests, env, run_on_python2,
-      input_api, output_api, unit_tests, env, run_on_python2, run_on_python3)
+                      run_on_python3, skip_shebang_check)
 
 
-def GetUnitTests(
+def GetUnitTests(input_api,
-    input_api, output_api, unit_tests, env=None, run_on_python2=True,
+                 output_api,
-    run_on_python3=True):
+                 unit_tests,
+                 env=None,
+                 run_on_python2=True,
+                 run_on_python3=True,
+                 skip_shebang_check=False):
   """Runs all unit tests in a directory.
 
   On Windows, sys.executable is used for unit tests ending with ".py".
@@ -721,19 +726,32 @@ def GetUnitTests(
           kwargs=kwargs,
           message=message_type))
     else:
-      if has_py3_shebang(unit_test) and run_on_python3:
+      test_run = False
+      # TODO(crbug.com/1223478): The intent for this line was to run the test
+      # on python3 if the file has a shebang OR if it was explicitly requested
+      # to run on python3. Since tests have been broken since this landed, we
+      # introduced the |skip_shebang_check| argument to work around the issue
+      # until every caller in Chromium has been fixed.
+      if (skip_shebang_check or has_py3_shebang(unit_test)) and run_on_python3:
         results.append(input_api.Command(
             name=unit_test,
             cmd=cmd,
             kwargs=kwargs,
             message=message_type,
             python3=True))
+        test_run = True
       if run_on_python2:
         results.append(input_api.Command(
             name=unit_test,
             cmd=cmd,
             kwargs=kwargs,
             message=message_type))
+        test_run = True
+      if not test_run:
+        output_api.PresubmitPromptWarning(
+            "Some python tests were not run. You may need to add\n"
+            "skip_shebang_check=True for python3 tests.",
+            items=unit_test)
   return results
 
 
@@ -743,7 +761,8 @@ def GetUnitTestsRecursively(input_api,
                             files_to_check,
                             files_to_skip,
                             run_on_python2=True,
-                            run_on_python3=True):
+                            run_on_python3=True,
+                            skip_shebang_check=False):
   """Gets all files in the directory tree (git repo) that match files_to_check.
 
   Restricts itself to only find files within the Change's source repo, not
@@ -769,9 +788,12 @@ def GetUnitTestsRecursively(input_api,
           % (found, files_to_check, files_to_skip, directory))
     ]
 
-  return GetUnitTests(input_api, output_api, tests,
+  return GetUnitTests(input_api,
+                      output_api,
+                      tests,
                       run_on_python2=run_on_python2,
-                      run_on_python3=run_on_python3)
+                      run_on_python3=run_on_python3,
+                      skip_shebang_check=skip_shebang_check)
 
 
 def GetPythonUnitTests(input_api, output_api, unit_tests):