Use the sigterm dance for testing/scripts.

https://chromium.googlesource.com/infra/luci/luci-py/+/master/appengine/swarming/doc/Bot.md#graceful-termination_aka-the-sigterm-and-sigkill-dance

test_env.py already has support for this, but it looks like a lot
of the scripts aren't using it. One reason for that could be that
there wasn't a run_command function there yet. I added a simple
run_command function, and made a bunch of scripts use it.

This should fix up all the test suites that use common.py's
functions to run subprocesses including:
 - run_telemetry_as_googletest.py
 - run_chromedriver_test.py
 - telemetry_gpu_unittests.py
 - telemetry_perf_unittests.py
 - telemetry_unittests.py
And many more!

Also delete common's run_integration_test function since no
one is using it.

Bug: 921662, 910584
Change-Id: If670e1b5725e3a7dcfda471b7141d47c73c5ef82
Reviewed-on: https://chromium-review.googlesource.com/c/1461512Reviewed-by: John Budorick <jbudorick@chromium.org>
Reviewed-by: Marc-Antoine Ruel <maruel@chromium.org>
Commit-Queue: John Budorick <jbudorick@chromium.org>
Auto-Submit: Caleb Rouleau <crouleau@chromium.org>
Cr-Commit-Position: refs/heads/master@{#634740}

testing/scripts/common.py

@@ -17,6 +17,7 @@ import traceback
 # Add src/testing/ into sys.path for importing xvfb.
 sys.path.append(os.path.join(os.path.dirname(__file__), '..'))
 import xvfb
+import test_env
 
 # Unfortunately we need to copy these variables from ../test_env.py.
 # Importing it and using its get_sandbox_env breaks test runs on Linux
@@ -79,7 +80,7 @@ def run_script(argv, funcs):
 
 def run_command(argv, env=None, cwd=None):
   print 'Running %r in %r (env: %r)' % (argv, cwd, env)
-  rc = subprocess.call(argv, env=env, cwd=cwd)
+  rc = test_env.run_command(argv, env=env, cwd=cwd)
   print 'Command %r returned exit code %d' % (argv, rc)
   return rc
 
@@ -198,20 +199,6 @@ def extract_filter_list(filter_list):
   return filter_list.split('::')
 
 
-def run_integration_test(script_to_run, extra_args, log_file, output):
-  integration_test_res = subprocess.call(
-      [sys.executable, script_to_run] + extra_args)
-
-  with open(log_file) as f:
-    failures = json.load(f)
-  json.dump({
-      'valid': integration_test_res == 0,
-      'failures': failures,
-  }, output)
-
-  return integration_test_res
-
-
 class BaseIsolatedScriptArgsAdapter(object):
   """The base class for all script adapters that need to translate flags
   set by isolated script test contract into the specific test script's flags.
@@ -350,7 +337,7 @@ class BaseIsolatedScriptArgsAdapter(object):
       if self.options.xvfb:
         exit_code = xvfb.run_executable(cmd, env)
       else:
-        exit_code = subprocess.call(cmd, env=env)
+        exit_code = test_env.run_command(cmd, env=env)
       print 'Command returned exit code %d' % exit_code
       return exit_code
     except Exception:

testing/test_env.py

@@ -174,7 +174,13 @@ def symbolize_snippets_in_json(cmd, env):
 
 
 def run_command_with_output(argv, stdoutfile, env=None, cwd=None):
-  """ Run command and stream its stdout/stderr to the console & |stdoutfile|.
+  """Run command and stream its stdout/stderr to the console & |stdoutfile|.
+
+  Also forward_signals to obey
+  https://chromium.googlesource.com/infra/luci/luci-py/+/master/appengine/swarming/doc/Bot.md#graceful-termination_aka-the-sigterm-and-sigkill-dance
+
+  Returns:
+    integer returncode of the subprocess.
   """
   print('Running %r in %r (env: %r)' % (argv, cwd, env))
   assert stdoutfile
@@ -185,6 +191,8 @@ def run_command_with_output(argv, stdoutfile, env=None, cwd=None):
     forward_signals([process])
     while process.poll() is None:
       sys.stdout.write(reader.read())
+      # This sleep is needed for signal propagation. See the
+      # wait_with_signals() docstring.
       time.sleep(0.1)
     # Read the remaining.
     sys.stdout.write(reader.read())
@@ -192,6 +200,44 @@ def run_command_with_output(argv, stdoutfile, env=None, cwd=None):
     return process.returncode
 
 
+def run_command(argv, env=None, cwd=None, log=True):
+  """Run command and stream its stdout/stderr both to stdout.
+
+  Also forward_signals to obey
+  https://chromium.googlesource.com/infra/luci/luci-py/+/master/appengine/swarming/doc/Bot.md#graceful-termination_aka-the-sigterm-and-sigkill-dance
+
+  Returns:
+    integer returncode of the subprocess.
+  """
+  if log:
+    print('Running %r in %r (env: %r)' % (argv, cwd, env))
+  process = subprocess.Popen(argv, env=env, cwd=cwd, stderr=subprocess.STDOUT)
+  forward_signals([process])
+  return wait_with_signals(process)
+
+
+def wait_with_signals(process):
+  """A version of process.wait() that works cross-platform.
+
+  This version properly surfaces the SIGBREAK signal.
+
+  From reading the subprocess.py source code, it seems we need to explicitly
+  call time.sleep(). The reason is that subprocess.Popen.wait() on Windows
+  directly calls WaitForSingleObject(), but only time.sleep() properly surface
+  the SIGBREAK signal.
+
+  Refs:
+  https://github.com/python/cpython/blob/v2.7.15/Lib/subprocess.py#L692
+  https://github.com/python/cpython/blob/v2.7.15/Modules/timemodule.c#L1084
+
+  Returns:
+    returncode of the process.
+  """
+  while process.poll() is None:
+    time.sleep(0.1)
+  return process.returncode
+
+
 def forward_signals(procs):
   """Forwards unix's SIGTERM or win's CTRL_BREAK_EVENT to the given processes.
 
@@ -286,15 +332,13 @@ def run_executable(cmd, env, stdoutfile=None):
           env=env, stdin=p1.stdout)
       p1.stdout.close()  # Allow p1 to receive a SIGPIPE if p2 exits.
       forward_signals([p1, p2])
-      p1.wait()
-      p2.wait()
+      wait_with_signals(p1)
+      wait_with_signals(p2)
       # Also feed the out-of-band JSON output to the symbolizer script.
       symbolize_snippets_in_json(cmd, env)
       return p1.returncode
     else:
-      p = subprocess.Popen(cmd, env=env)
-      forward_signals([p])
-      return p.wait()
+      return run_command(cmd, env=env, log=False)
   except OSError:
     print >> sys.stderr, 'Failed to start %s' % cmd
     raise

testing/xvfb.py

@@ -89,10 +89,11 @@ def run_executable(cmd, env, stdoutfile=None):
       xvfb_script = __file__
       if xvfb_script.endswith('.pyc'):
         xvfb_script = xvfb_script[:-1]
-      return subprocess.call(['xvfb-run', '-a', "--server-args=-screen 0 "
-                              "1280x800x24 -ac -nolisten tcp -dpi 96 "
-                              "+extension RANDR",
-                              xvfb_script] + cmd, env=env)
+      # TODO(crbug.com/932240): Propagate signals properly.
+      return subprocess.call([
+          'xvfb-run', '-a', "--server-args=-screen 0 "
+          "1280x800x24 -ac -nolisten tcp -dpi 96 "
+          "+extension RANDR", xvfb_script] + cmd, env=env)
   else:
     return test_env.run_executable(cmd, env, stdoutfile)