Move android buildbot test logic into python

build/android/buildbot/bb_run_tests.py

Index: build/android/buildbot/bb_run_tests.py
diff --git a/build/android/buildbot/bb_run_tests.py b/build/android/buildbot/bb_run_tests.py
new file mode 100755
index 0000000000000000000000000000000000000000..55554416d96374e7d658cf59ddbcf5073a9059f7
--- /dev/null
+++ b/build/android/buildbot/bb_run_tests.py
@@ -0,0 +1,242 @@
+#!/usr/bin/env python
+# 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.
+
+import collections
+import glob
+import json
+import optparse
+import os
+import pipes
+import shutil
+import subprocess
+import sys
+
+sys.path.append(os.path.join(os.path.dirname(__file__), '..'))
+from pylib import buildbot_report
+from pylib import constants
+
+TESTING = 'BUILDBOT_TESTING' in os.environ

[xusydoc (do not use), 2013/01/04 17:09:21]

2 spaces between top-level defines

+CHROME_SRC = constants.CHROME_DIR
+
+
+# Describes an instrumation test suite:
+#   test: Name of test we're running.
+#   apk: apk to be installed.
+#   apk_package: package for the apk to be installed.
+#   test_apk: apk to run tests on.
+#   test_data: data folder in format destination:source.
+I_TEST = collections.namedtuple('InstrumentationTest', [
+    'name', 'apk', 'apk_package', 'test_apk', 'test_data'])
+
+INSTRUMENTATION_TESTS = dict((suite.name, suite) for suite in [

[xusydoc (do not use), 2013/01/04 17:09:21]

same

+    I_TEST('ContentShell',
+           'ContentShell.apk',
+           'org.chromium.content_shell',
+           'ContentShellTest',

[frankf, 2012/12/27 19:57:47]

There's a lot of redundancy here. Why can't you derive everything from the apk
itself based on the convention we use.

[Isaac (away), 2012/12/27 22:23:03]

I am porting bash code to python.  If we want to simplify logic we can do that
in a follow up CL.

+           'content:content/test/data/android/device_files'),
+    I_TEST('ChromiumTestShell',
+           'ChromiumTestShell.apk',
+           'org.chromium.chrome.testshell',
+           'ChromiumTestShellTest',
+           'chrome:chrome/test/data/android/device_files'),
+    I_TEST('AndroidWebView',
+           'AndroidWebView.apk',
+           'org.chromium.android_webview',
+           'AndroidWebViewTest',
+           'webview:android_webview/test/data/device_files'),
+    ])
+
+VALID_TESTS = set(['ui', 'unit', 'webkit', 'webkit_layout'])

[xusydoc (do not use), 2013/01/04 17:09:21]

same

+
+
+def RunCmd(command, flunk_on_failure=True):
+  """Run a command relative to the chrome source root."""
+
+  # Add adb binary to path. In the future, might use build_internal copy.
+  env = dict(os.environ)
+  env['PATH'] = os.pathsep.join([
+      env['PATH'], os.path.join(constants.ANDROID_SDK_ROOT, 'platform-tools')])
+
+  command_str = ' '.join(map(pipes.quote, command))
+  print '>', command_str
+  if not TESTING:
+    code = subprocess.Popen(command, cwd=CHROME_SRC, env=env).wait()
+  else:
+    code = 0
+  print '<', command_str
+  if code != 0:
+    print 'ERROR: non-zero status %d from %s' % (code, command)
+    if flunk_on_failure:
+      buildbot_report.PrintError()
+    else:
+      buildbot_report.PrintWarning()
+  return code
+
+
+def RunTestSuites(options, suite):
+  """Manages an invocation of run_tests.py.
+
+  Args:
+    options: options object.
+    suite: The suite to pass to run_tests or None to run default suites.
+  """
+  args = ['--verbose']
+  if suite:
+    args.extend(['-s', suite])
+  if options.target == 'Release':
+    args.append('--release')
+  if options.asan:
+    args.append('--tool=asan')
+  RunCmd(['build/android/run_tests.py'] + args)

[frankf, 2012/12/27 19:57:47]

If we're adding the complexity of another wrapper, we should at least get the
benefit of simplified/consistant test scripts by removing all the buildbot
annotations out of them. So, run_tests.py should not be maintaining a list of
default suites but instead be called  multiple times using -s parameter.

[Isaac (away), 2012/12/27 22:23:03]

This is a pure port - bash -> python.  We are not adding complexity.  Moving the
annotations here (or to bb_run_step) is a good idea but that can be done in a
follow up CL.

[frankf, 2012/12/27 22:33:26]

OK, let's keep this CL simple, but it seems we're on the same page wrt to how
annotations should be done.

On 2012/12/27 22:23:03, Isaac wrote:

+
+
+def InstallApk(apk, apk_package, target):
+  args = ['--apk', apk, '--apk_package', apk_package]
+  if target == 'Release':
+    args.append('--release')
+
+  RunCmd(['build/android/adb_install_apk.py'] + args)
+
+
+def RunInstrumentationSuite(options, test):
+  """Manages an invocation of run_instrumentaiton_tests.py.
+
+  Args:
+    options: options object
+    test: An I_TEST namedtuple
+  """
+  buildbot_report.PrintNamedStep('Instrumentation tests for %s' % test.name)

[frankf, 2012/12/27 19:57:47]

Will crbug.com/161993 be addressed in a follow up CL? We want a clear convention
for step names since they're used in bunch of places including the flakiness
dashboard server.

[Isaac (away), 2012/12/27 22:23:03]

That is a separate bug, not directly related to this.  In both bash and in this
new python code, we create an instrumentation test step and run the
instrumentation tests.  We need to remove the step the
run_intrumentation_tests.py code creates, but I won't be doing that here.

The step name is a placeholder until I came up with a better name.  I'm thinking

'%s tests' % test.name, i.e. "ContentShell tests"

[frankf, 2012/12/27 22:33:26]

Let's avoid spaces. I've proposed a convention in that bug which is more inline
with the naming convention for other test types used in Chromium.

On 2012/12/27 22:23:03, Isaac wrote:

[Isaac (away), 2012/12/27 22:55:20]

OK, I took a look.  I think the word instrumentation is redundant, given that is
the only type of test running on ContentShell, test shell, etc.  How about
'%s_tests' % test.name?

[frankf, 2012/12/27 23:06:10]

It's needed to remove ambiguity in other contexts. I took a quick look at the
flakiness dashboard, there's something called test_shell_tests in webkit for
example. 

On 2012/12/27 22:55:20, Isaac wrote:

[Isaac (away), 2012/12/28 05:31:53]

The instrumentation tests use ChromiumTestShell, which is so named to
distinguish itself from webkit test shell.  Let's see what the other reviewers
think.

+
+  InstallApk(test.apk, test.apk_package, options.target)
+  args = ['--test-apk', test.test_apk, '--test_data', test.test_data, '-vvv',
+          '-I']
+  if options.target == 'Release':
+    args.append('--release')
+  if options.asan:
+    args.append('--tool=asan')
+
+  RunCmd(['build/android/run_instrumentation_tests.py'] + args)

[frankf, 2012/12/27 19:57:47]

Again, run_instrumentation_tests.py won't produce a second step right?

[Isaac (away), 2012/12/27 22:23:03]

See above.  Pure port / wrapper changes.  It is complicated to modify
run_instrumentation_tests as that is called by our internal bots in a different
context.

+
+
+def RunWebkitLint(target):
+  """Lint WebKit's TestExpectation files."""
+  buildbot_report.PrintNamedStep('webkit_lint')
+  RunCmd(['webkit/tools/layout_tests/run_webkit_tests.py',
+          '--lint-test-files',
+          '--chromium',
+          '--target', target])
+
+
+def RunWebkitLayoutTests(options):
+  """Run layout tests on an actual device."""
+  buildbot_report.PrintNamedStep('webkit_tests')
+  RunCmd(['webkit/tools/layout_tests/run_webkit_tests.py',
+          '--no-show-results',
+          '--no-new-test-results',
+          '--full-results-html',
+          '--clobber-old-results',
+          '--exit-after-n-failures', '5000',
+          '--exit-after-n-crashes-or-timeouts', '100',
+          '--debug-rwt-logging',
+          '--results-directory', '..layout-test-results',
+          '--target', options.target,
+          '--builder-name', options.build_properties.get('buildername', ''),
+          '--build-number', options.build_properties.get('buildnumber', ''),
+          '--master-name', options.build_properties.get('mastername', ''),
+          '--build-name', options.build_properties.get('buildername', ''),
+          '--platform=chromium-android',
+          '--test-results-server',
+          options.factory_properties.get('test_results_server', '')])
+
+
+def MainTestWrapper(options):
+  if options.install:
+    test_obj = INSTRUMENTATION_TESTS[options.install]
+    InstallApk(test_obj.apk, test_obj.apk_package, options.target)
+
+  if not options.test_filter:
+    return
+
+  # Device check and alert emails
+  RunCmd(['build/android/device_status_check.py'], flunk_on_failure=False)
+
+  # Spawn logcat monitor
+  logcat_dir = os.path.join(CHROME_SRC, 'out/logcat')
+  shutil.rmtree(logcat_dir, ignore_errors=True)
+  if not TESTING:
+    subprocess.Popen(
+        ['build/android/adb_logcat_monitor.py', logcat_dir], cwd=CHROME_SRC)
+
+  if 'unit' in options.test_filter:
+    RunTestSuites(options, None)
+  if 'ui' in options.test_filter:
+    for test in INSTRUMENTATION_TESTS.itervalues():
+      RunInstrumentationSuite(options, test)
+  if 'webkit' in options.test_filter:
+    RunTestSuites(options, 'webkit_unit_tests')
+    RunTestSuites(options, 'TestWebKitAPI')
+    RunWebkitLint(options.target)
+  if 'webkit_layout' in options.test_filter:
+    RunWebkitLayoutTests(options)
+
+  if options.experimental:
+    pass
+
+  # Print logcat, kill logcat monitor
+  buildbot_report.PrintNamedStep('Logcat dump')
+  RunCmd(['build/android/adb_logcat_printer.py', logcat_dir])
+
+  buildbot_report.PrintNamedStep('Test report')
+  for report in glob.glob(
+      os.path.join(CHROME_SRC, 'out', options.target, 'test_logs', '*.log')):
+    subprocess.Popen(['cat', report]).wait()
+    os.remove(report)
+
+
+def main(argv):
+  parser = optparse.OptionParser()
+
+  def convert_json(option, _, value, parser):
+    setattr(parser.values, option.dest, json.loads(value))
+
+  parser.add_option('--build-properties', action='callback',
+                    callback=convert_json, type='string', default={},
+                    help='build properties in JSON format')
+  parser.add_option('--factory-properties', action='callback',
+                    callback=convert_json, type='string', default={},
+                    help='factory properties in JSON format')
+  parser.add_option('--slave-properties', action='callback',
+                    callback=convert_json, type='string', default={},
+                    help='Properties set by slave script in JSON format')
+  parser.add_option('--experimental', action='store_true',
+                    help='Run experiemental tests')
+  parser.add_option('-f', '--test-filter', metavar='<filter>', default=[],
+                    action='append',
+                    help=('Run a test suite. Test suites: "%s"' %
+                          '", "'.join(VALID_TESTS)))
+  parser.add_option('--asan', action='store_true', help='Run tests with asan.')
+  parser.add_option('--install', metavar='<apk name>',
+                    help='Install an apk by name')
+  options, args = parser.parse_args(argv[1:])
+
+  def ParserError(msg):
+    """We avoid parser.error because it calls sys.exit."""
+    parser.print_help()
+    print >>sys.stderr, '\nERROR:', msg
+    return 1
+
+  if args:
+    return ParserError('Unused args %s' % args)
+
+  unknown_tests = set(options.test_filter) - VALID_TESTS
+  if unknown_tests:
+    return ParserError('Unknown tests %s' % list(unknown_tests))
+
+  setattr(options, 'target', options.factory_properties.get('target', 'Debug'))
+
+  MainTestWrapper(options)
+
+if __name__ == '__main__':

[xusydoc (do not use), 2013/01/04 17:09:21]

same

+  sys.exit(main(sys.argv))