Use logdog butler subcommand to run tests.

build/android/test_wrapper/logdog_wrapper.py

Index: build/android/test_wrapper/logdog_wrapper.py
diff --git a/build/android/test_wrapper/logdog_wrapper.py b/build/android/test_wrapper/logdog_wrapper.py
index 14ed7d138b432d0a2e856ce7f3ae8aa8b6b2e209..dcfc6c5498e72b516af7d970574eef888250346e 100755
--- a/build/android/test_wrapper/logdog_wrapper.py
+++ b/build/android/test_wrapper/logdog_wrapper.py
@@ -8,38 +8,19 @@
 import argparse
 import logging
 import os
+import shutil
 import signal
 import subprocess
 import sys
-import urllib
-
+import tempfile
 
 def CommandParser():
   # Parses the command line arguments being passed in
   parser = argparse.ArgumentParser()
-  parser.add_argument('--logdog-bin-cmd', required=True,
-                      help='Command for running logdog butler binary')
-  parser.add_argument('--project', required=True,
-                      help='Name of logdog project')
-  parser.add_argument('--logdog-server',
-                      default='services-dot-luci-logdog.appspot.com',
-                      help='URL of logdog server, https:// is assumed.')
-  parser.add_argument('--service-account-json', required=True,
-                      help='Location of authentication json')
-  parser.add_argument('--prefix', required=True,
-                      help='Prefix to be used for logdog stream')
-  parser.add_argument('--source', required=True,
-                      help='Location of file for logdog to stream')
-  parser.add_argument('--name', required=True,
-                      help='Name to be used for logdog stream')
+  parser.add_argument('--target', required=True,
+                      help='The test target to be run.')
   return parser
 
-
-def CreateUrl(server, project, prefix, name):
-  stream_name = '%s/%s/+/%s' % (project, prefix, name)
-  return 'https://%s/v/?s=%s' % (server, urllib.quote_plus(stream_name))
-
-
 def CreateSignalForwarder(proc):
   def handler(signum, _frame):
     logging.error('Forwarding signal %s to test process', str(signum))
@@ -47,13 +28,41 @@ def CreateSignalForwarder(proc):
 
   return handler
 
-
 def main():
   parser = CommandParser()
-  args, test_cmd = parser.parse_known_args(sys.argv[1:])
+  args = parser.parse_args(sys.argv[1:])
+
   logging.basicConfig(level=logging.INFO)
-  if not test_cmd:
-    parser.error('Must specify command to run after the logdog flags')
+
+  test_cmd = [
+      os.path.join('bin', 'run_%s' % args.target),
+      '--logcat-output-file', '${ISOLATED_OUTDIR}/logcats',

[mikecase (-- gone --), 2017/02/27 20:25:48]

We do not want to pass both...

'--logcat-output-file', '${ISOLATED_OUTDIR}/logcats',
and 
test_cmd.extend(['--upload-full-logcat', '${ISOLATED_OUTDIR}/logcats'])

otherwise we will be uploading logcats to isolate twice, uploading full logcat
to logdog (and maybe) uploading per test logcats to logdog as well.




If this script is run on the swarming slaves, you can just create a tempdir and
pass that as the arg (as the swarming bot will know to clean that up
automatically). If not, it will be a bit tricky.

[BigBossZhiling, 2017/02/27 21:24:41]

John, do you know that if we create the temp file in mb.py, will it
automatically delete the file at the end?

[jbudorick, 2017/02/27 22:49:15]

Case's response is correct.

[jbudorick, 2017/02/27 22:50:27]

(to clarify: this was w.r.t. mb.py running on a different bot than
logdog_wrapper/test_runner)

[BigBossZhiling, 2017/02/28 07:29:53]

Done.

+      '--target-devices-file', '${SWARMING_BOT_FILE}',
+      '-v']
+
+  logdog_bin_cmd = '../../bin/logdog_butler'
+  if not os.path.exists(logdog_bin_cmd):
+    logging.error(
+        'Logdog binary %s unavailable. Unable to create logdog client',
+        logdog_bin_cmd)
+    temp_directory = None

[jbudorick, 2017/02/27 22:49:15]

I think you could handle this more cleanly with a few context managers:
 - devil.utils.signal_handler.SignalHandler for the SIGTERM handler
 - contextlib_ext.Optional + tempfile_ext.NamedTemporaryDirectory for the temp
directory

[BigBossZhiling, 2017/02/28 07:29:53]

Done.

+  else:
+    test_cmd.extend(['--upload-full-logcat', '${ISOLATED_OUTDIR}/logcats'])
+    project = 'chromium'

[jbudorick, 2017/02/27 22:49:15]

project, output, and coordinator_host can and probably should be module-scope
constants.

[BigBossZhiling, 2017/02/28 07:29:53]

Done.

+    output = 'logdog,service="services"'
+    coordinator_host = 'luci-logdog.appspot.com'
+    temp_directory = tempfile.mkdtemp(prefix='tmp_android_logdog_wrapper')
+    streamserver_uri = 'unix:%s/butler.sock' % temp_directory
+    prefix = 'android/swarming/logcats/${SWARMING_TASK_ID}'
+
+    # Call test_cmdline through logdog butler subcommand.
+    test_cmd = [
+        logdog_bin_cmd, '-project', project,
+        '-output', output,
+        '-prefix', prefix,
+        '-coordinator-host', coordinator_host,
+        'run', '-streamserver-uri', streamserver_uri, '--'] + test_cmd
+
   test_proc = subprocess.Popen(test_cmd)
   original_sigterm_handler = signal.signal(
       signal.SIGTERM, CreateSignalForwarder(test_proc))
@@ -61,31 +70,12 @@ def main():
     result = test_proc.wait()
   finally:
     signal.signal(signal.SIGTERM, original_sigterm_handler)
-  if '${SWARMING_TASK_ID}' in args.prefix:
-    args.prefix = args.prefix.replace('${SWARMING_TASK_ID}',
-                                      os.environ.get('SWARMING_TASK_ID'))
-  url = CreateUrl('luci-logdog.appspot.com', args.project, args.prefix,
-                  args.name)
-  logdog_cmd = [args.logdog_bin_cmd, '-project', args.project,
-                '-output', 'logdog,host=%s' % args.logdog_server,
-                '-prefix', args.prefix,
-                '-service-account-json', args.service_account_json,
-                'stream', '-source', args.source,
-                '-stream', '-name=%s' % args.name]
-
-  if not os.path.exists(args.logdog_bin_cmd):
-    logging.error(
-        'Logdog binary %s unavailable. Unable to upload logcats.',
-        args.logdog_bin_cmd)
-  elif not os.path.exists(args.source):
-    logging.error(
-        'Logcat sources not found at %s. Unable to upload logcats.',
-        args.source)
-  else:
-    subprocess.call(logdog_cmd)
-    logging.info('Logcats are located at: %s', url)
+    if temp_directory:
+      try:
+        shutil.rmtree(temp_directory)
+      except (OSError, IOError):
+        logging.exception('Failed to remove temp directory %s', temp_directory)
   return result
 
-
 if __name__ == '__main__':
   sys.exit(main())