Swarming: conditionally run tests on swarming in chromium_trybot recipe.

scripts/slave/recipes/chromium_trybot.py

 # Copyright 2013 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.
 
 DEPS = [
   'bot_update',
   'chromium',
   'gclient',
   'isolate',
   'itertools',
   'json',
   'path',
   'platform',
   'properties',
   'python',
   'raw_io',
   'step',
   'step_history',
+  'swarming',
   'test_utils',
   'tryserver',
 ]
 
 
 BUILDERS = {
   'tryserver.chromium': {
     'builders': {
       'linux_arm_cross_compile': {
         'GYP_DEFINES': {
@@ skipping 304 matching lines @@
         'compile_only': False,
         'testing': {
           'platform': 'win',
         },
       },
     },
   },
 }
 
 
+def add_swarming_builder(original, swarming, server='tryserver.chromium'):
+  """Duplicates builder config on |server|, adding 'enable_swarming: True'."""
+  assert server in BUILDERS
+  assert original in BUILDERS[server]['builders']
+  assert swarming not in BUILDERS[server]['builders']
+  conf = BUILDERS[server]['builders'][original].copy()
+  conf['enable_swarming'] = True
+  BUILDERS[server]['builders'][swarming] = conf
+
+
+add_swarming_builder('linux_chromium_rel', 'linux_chromium_rel_swarming')
+add_swarming_builder('win_chromium_rel', 'win_chromium_rel_swarming')
+add_swarming_builder('mac_chromium_rel', 'mac_chromium_rel_swarming')
+
+
 def GenSteps(api):
   class CheckdepsTest(api.test_utils.Test):  # pylint: disable=W0232
     name = 'checkdeps'
 
     @staticmethod
     def compile_targets():
       return []
 
     @staticmethod
     def run(suffix):
@@ skipping 77 matching lines @@
           step_test_data=lambda: api.json.test_api.canned_gtest_output(True))
 
     def has_valid_results(self, suffix):
       step_name = self._step_name(suffix)
       gtest_results = api.step_history[step_name].json.gtest_results
       if not gtest_results.valid:  # pragma: no cover
         return False
       global_tags = gtest_results.raw.get('global_tags', [])
       return 'UNRELIABLE_RESULTS' not in global_tags
 
-
     def failures(self, suffix):
       step_name = self._step_name(suffix)
       return api.step_history[step_name].json.gtest_results.failures
 
 
+  class SwarmingGTestTest(api.test_utils.Test):
+    # This test is using 'trigger' and 'collect' instead of 'run'.
+    async = True
+
+    def __init__(self, name, args=None):
+      api.test_utils.Test.__init__(self)
+      self._name = name
+      self._args = args or []
+      self._tasks = {}
+      self._results = {}
+
+    @property
+    def name(self):
+      return self._name
+
+    def compile_targets(self):
+      # <X>_run target depends on <X>, and then isolates it invoking isolate.py.
+      # It is a convention, not a hard coded rule.
+      return [self._name + '_run']
+
+    def trigger(self, suffix):
+      assert suffix not in self._tasks, (
+          'Test %s was already triggered' % self._step_name(suffix))
+
+      # *.isolated may be missing if *_run target is misconfigured. It a error

[Paweł Hajdan Jr., 2014/06/10 08:36:56]

nit: "It a error" -> "It's an error"

[Vadim Sh., 2014/06/12 01:00:08]

Done.

+      # in gyp, not a recipe failure. So carry on with recipe execution.
+      isolated_hash = api.isolate.isolated_tests.get(self._name)
+      if not isolated_hash:
+        return api.python.inline(
+            '[error] %s' % self._step_name(suffix),
+            r"""
+            import sys
+            print '*.isolated file for target %s is missing' % sys.argv[1]
+            sys.exit(1)
+            """,
+            args=[self._name],
+            always_run=True)
+
+      # If rerunning without a patch, run only tests that failed.
+      args = self._args[:]
+      if suffix == 'without patch':
+        failed_tests = sorted(self.failures('with patch'))
+        args.append('--gtest_filter=%s' % ':'.join(failed_tests))
+
+      # Trigger the test on swarming.
+      self._tasks[suffix] = api.swarming.gtest_task(
+          title=self._step_name(suffix),
+          isolated_hash=isolated_hash,
+          test_launcher_summary_output=api.json.gtest_results(
+              add_json_log=False),
+          extra_args=args)
+      return api.swarming.trigger([self._tasks[suffix]], always_run=True)
+
+    def collect(self, suffix):
+      assert suffix not in self._results, (
+          'Results of %s were already collected' % self._step_name(suffix))
+
+      # Emit error if test wasn't triggered. This happens if *.isolated is not
+      # found. (The build is already red by this moment anyway).
+      if suffix not in self._tasks:
+        return api.python.inline(
+            '[collect error] %s' % self._step_name(suffix),
+            r"""
+            import sys
+            print '%s wasn\'t triggered' % sys.argv[1]
+            sys.exit(1)
+            """,
+            args=[self._name],
+            always_run=True)
+
+      # Update step presentation, store step results in self._results.
+      def followup_fn(step_result):
+        r = step_result.json.gtest_results
+        p = step_result.presentation
+        if r.valid:
+          p.step_text += api.test_utils.format_step_text([
+              ['failures:', r.failures]
+          ])
+        self._results[suffix] = r
+
+      # Wait for test on swarming to finish.
+      return api.swarming.collect(
+          [self._tasks[suffix]],
+          always_run=True,
+          can_fail_build=False,
+          followup_fn=followup_fn)
+
+    def has_valid_results(self, suffix):
+      # Test wasn't triggered or wasn't collected.
+      if suffix not in self._tasks or not suffix in self._results:
+        return False
+      # Test ran, but failed to produce valid *.json.
+      gtest_results = self._results[suffix]
+      if not gtest_results.valid:  # pragma: no cover
+        return False
+      global_tags = gtest_results.raw.get('global_tags', [])
+      return 'UNRELIABLE_RESULTS' not in global_tags
+
+    def failures(self, suffix):
+      assert self.has_valid_results(suffix)
+      return self._results[suffix].failures
+
+
   class NaclIntegrationTest(api.test_utils.Test):  # pylint: disable=W0232
     name = 'nacl_integration'
 
     @staticmethod
     def compile_targets():
       return ['chrome']
 
     def run(self, suffix):
       args = [
         '--mode', api.chromium.c.build_config_fs,
         '--json_build_results_output_file', api.json.output(),
       ]
       return api.python(
           self._step_name(suffix),
           api.path['checkout'].join('chrome',
                             'test',
                             'nacl_test_injection',
                             'buildbot_nacl_integration.py'),
           args,
           can_fail_build=False,
           step_test_data=lambda: api.m.json.test_api.output([]))
 
     def has_valid_results(self, suffix):
       return api.step_history[self._step_name(suffix)].json.output is not None
 
     def failures(self, suffix):
       failures = api.step_history[self._step_name(suffix)].json.output
       return [f['raw_name'] for f in failures]
 
+
+  def parse_test_spec(test_spec, enable_swarming, should_use_test):
+    """Returns a list of tests to run and additional targets to compile.
+
+    Uses 'should_use_test' callback to figure out what tests should be skipped.
+
+    Returns triple (compile_targets, gtest_tests, swarming_tests) where
+      gtest_tests is a list of GTestTest
+      swarming_tests is a list of SwarmingGTestTest.
+    """
+    compile_targets = []
+    gtest_tests_spec = []
+    if isinstance(test_spec, dict):
+      compile_targets = test_spec.get('compile_targets', [])
+      gtest_tests_spec = test_spec.get('gtest_tests', [])
+    else:
+      # TODO(nodir): Remove this after
+      # https://codereview.chromium.org/297303012/#ps50001
+      # lands.
+      gtest_tests_spec = test_spec
+
+    gtest_tests = []
+    swarming_tests = []
+    for test in gtest_tests_spec:
+      test_name = None
+      test_dict = None
+
+      # Read test_dict for the test, it defines where test can run.
+      if isinstance(test, unicode):
+        test_name = test.encode('utf-8')
+        test_dict = {}
+      elif isinstance(test, dict):
+        if 'test' not in test:  # pragma: no cover
+          raise ValueError('Invalid entry in test spec: %r' % test)
+        test_name = test['test'].encode('utf-8')
+        test_dict = test
+      else:  # pragma: no cover
+        raise ValueError('Unrecognized entry in test spec: %r' % test)
+
+      # Should skip it completely?
+      if not test_name or not should_use_test(test_dict):
+        continue
+
+      # If test can run on swarming, test_dict has a section that defines when
+      # swarming should be used, in same format as main test dict.
+      use_swarming = False
+      if enable_swarming:
+        swarming_spec = test_dict.get('swarming', False)

[Paweł Hajdan Jr., 2014/06/10 08:36:56]

nit: Could you rename this to "can_use_swarming_if_enabled"?

This is a bit verbose, but hopefully makes it more clear in the test spec, which
is edited by developers which are not infrastructure experts.

I agree it's still not perfectly clear, but just a step I'd like to take.

Feel free to come up with different naming ideas, although I'd really like this
to be clear even if more verbose.

[Vadim Sh., 2014/06/12 01:00:08]

No, because in general it's not a boolean field, but a dict.

I remade it to be a dict with 'can_use_on_swarming_builders' key:

{
  'test': 'browser_tests',
  'swarming': {
    'can_use_on_swarming_builders': true,
    ...
  }
}

Is it clear enough?
I think using JSON for test specs is sub-optimal. JSON doesn't support comments.
I think it should be converted to python dicts loaded with ast.literal_eval.
Then the structure of test spec can be described in the same file as a comment,
so that any developer can understand it with perfect clearness... And since it's
literal_eval, no crazy buildbot-config style shit is possible.

+        if isinstance(swarming_spec, bool):
+          use_swarming = swarming_spec
+          swarming_spec = {}
+        elif isinstance(swarming_spec, dict):

[Paweł Hajdan Jr., 2014/06/10 08:36:56]

We have isinstance for other parts of the spec for backwards compatibility.
Swarming is new, so can we standardize on one format for now? We'll likely have
to extend it anyway, so let's keep the initial version as simple as possible
(less moving parts).

[Vadim Sh., 2014/06/12 01:00:08]

Always dict now.

+          if not swarming_spec:  # pragma: no cover
+            raise ValueError('Empty \'swarming\' section in test spec for %s '
+                             'is ambiguous. Use true or false.' % test_name)
+          use_swarming = should_use_test(swarming_spec)
+        else:  # pragma: no cover
+          raise ValueError(
+              'Unrecognized swarming entry in test spec: %r' % test)
+
+      test_args = test_dict.get('args')
+      if isinstance(test_args, basestring):
+        test_args = [test_args]
+
+      if use_swarming:
+        swarming_tests.append(SwarmingGTestTest(test_name, test_args))
+      else:
+        gtest_tests.append(GTestTest(test_name, test_args))
+
+    return compile_targets, gtest_tests, swarming_tests
+
+
   mastername = api.properties.get('mastername')
   buildername = api.properties.get('buildername')
   master_dict = BUILDERS.get(mastername, {})
   bot_config = master_dict.get('builders', {}).get(buildername)
   assert bot_config, (
       'Unrecognized builder name %r for master %r.' % (
           buildername, mastername))
 
   # Make sure tests and the recipe specify correct and matching platform.
   assert api.platform.name == bot_config.get('testing', {}).get('platform')
 
   api.chromium.set_config(bot_config['chromium_config'],
                           **bot_config.get('chromium_config_kwargs', {}))
   # Settings GYP_DEFINES explicitly because chromium config constructor does
   # not support that.
   api.chromium.c.gyp_env.GYP_DEFINES.update(bot_config.get('GYP_DEFINES', {}))
-  if bot_config.get('use_isolate'):
-    api.isolate.set_isolate_environment(api.chromium.c)
   api.chromium.apply_config('trybot_flavor')
   api.gclient.set_config('chromium')
   api.step.auto_resolve_conflicts = True
 
   yield api.bot_update.ensure_checkout()
   # The first time we run bot update, remember if bot_update mode is on or off.
   bot_update_mode = api.step_history.last_step().json.output['did_run']
   if not bot_update_mode:
     yield api.gclient.checkout(
         revert=True, can_fail_build=False, abort_on_failure=False)
@@ skipping 30 matching lines @@
           'args': ['--test-launcher-print-test-stdio=always'],
         },
         {
           'test': 'browser_tests',
           'exclude_builders': ['tryserver.chromium:win_chromium_x64_rel'],
         },
       ]),
       followup_fn=test_spec_followup_fn,
   )
 
+  def should_use_test(test):
+    """Given a test dict from test spec returns True or False."""
+    if 'platforms' in test:
+      if api.platform.name not in test['platforms']:
+        return False
+    if 'chromium_configs' in test:
+      if bot_config['chromium_config'] not in test['chromium_configs']:
+        return False
+    if 'exclude_builders' in test:
+      if '%s:%s' % (mastername, buildername) in test['exclude_builders']:
+        return False
+    return True
+
+  # Parse test spec file into list of Test instances.
+  compile_targets, gtest_tests, swarming_tests = parse_test_spec(
+      api.step_history['read test spec'].json.output,
+      bot_config.get('enable_swarming'),
+      should_use_test)
+
+  # Swarming uses Isolate to transfer files to swarming bots.
+  # set_isolate_environment modifies GYP_DEFINES to enable test isolation.
+  use_isolate = swarming_tests or bot_config.get('use_isolate')
+  if use_isolate:
+    api.isolate.set_isolate_environment(api.chromium.c)
+
   runhooks_env = bot_config.get('runhooks_env', {})
 
   yield api.chromium.runhooks(env=runhooks_env, abort_on_failure=False,
                               can_fail_build=False)
   if api.step_history.last_step().retcode != 0:
     # Before removing the checkout directory try just using LKCR.
     api.gclient.set_config('chromium_lkcr')
 
     # Since we're likely to switch to an earlier revision, revert the patch,
     # sync with the new config, and apply issue again.
@@ skipping 10 matching lines @@
       if api.step_history.last_step().retcode != 0:
         if api.platform.is_win:
           yield api.chromium.taskkill()
         yield (
           api.path.rmcontents('slave build directory', api.path['slave_build']),
           api.gclient.checkout(revert=False),
           api.tryserver.maybe_apply_issue(),
           api.chromium.runhooks(env=runhooks_env)
         )
 
-  gtest_tests = []
-  compile_targets = []
-  test_spec = api.step_history['read test spec'].json.output
-
-  if isinstance(test_spec, dict):
-    compile_targets = test_spec.get('compile_targets', [])
-    gtest_tests_spec = test_spec.get('gtest_tests', [])
-  else:
-    # TODO (nodir): Remove this after
-    # https://codereview.chromium.org/297303012/#ps50001
-    # lands.
-    gtest_tests_spec = test_spec
-
-  for test in gtest_tests_spec:
-    test_name = None
-    test_args = None
-
-    if isinstance(test, unicode):
-      test_name = test.encode('utf-8')
-    elif isinstance(test, dict):
-      if 'platforms' in test:
-        if api.platform.name not in test['platforms']:
-          continue
-
-      if 'chromium_configs' in test:
-        if bot_config['chromium_config'] not in test['chromium_configs']:
-          continue
-
-      if 'exclude_builders' in test:
-        if '%s:%s' % (mastername, buildername) in test['exclude_builders']:
-          continue
-
-      test_args = test.get('args')
-      if isinstance(test_args, basestring):
-        test_args = [test_args]
-
-      if 'test' not in test:  # pragma: no cover
-        raise ValueError('Invalid entry in test spec: %r' % test)
-
-      test_name = test['test'].encode('utf-8')
-    else:  # pragma: no cover
-      raise ValueError('Unrecognized entry in test spec: %r' % test)
-
-    if test_name:
-      gtest_tests.append(GTestTest(test_name, test_args))
+  # If going to use swarming_client (pinned in src/DEPS), ensure it is
+  # compatible with what recipes expect.
+  if swarming_tests:
+    yield api.swarming.check_client_version()
 
   tests = []
   tests.append(CheckdepsTest())
   tests.append(Deps2GitTest())
-  for test in gtest_tests:
-    tests.append(test)
+  tests.extend(gtest_tests)
+  tests.extend(swarming_tests)
   tests.append(NaclIntegrationTest())
 
   compile_targets.extend(bot_config.get('compile_targets', []))
   compile_targets.extend(api.itertools.chain(
       *[t.compile_targets() for t in tests]))
   # TODO(phajdan.jr): Also compile 'all' on win, http://crbug.com/368831 .
   # Disabled for now because it takes too long and/or fails on Windows.
   if not api.platform.is_win and not bot_config.get('exclude_compile_all'):
     compile_targets = ['all'] + compile_targets
   yield api.chromium.compile(compile_targets,
@@ skipping 33 matching lines @@
       yield api.chromium.runhooks()
       yield api.chromium.compile(compile_targets,
                                  name='compile '
                                       '(with patch, lkcr, clobber, nuke)',
                                  force_clobber=True)
 
   # Do not run tests if the build is already in a failed state.
   if api.step_history.failed:
     return
 
-  if bot_config.get('use_isolate'):
+  # Collect *.isolated hashes for all isolated targets, used when triggering
+  # tests on swarming.
+  if use_isolate:
     yield api.isolate.find_isolated_tests(api.chromium.output_dir)
 
   if bot_config['compile_only']:
     return
 
   if bot_config['chromium_config'] not in ['chromium_chromeos',
                                            'chromium_chromeos_clang']:
     # TODO(phajdan.jr): Make it possible to retry telemetry tests (add JSON).
+    # TODO(vadimsh): Trigger swarming tests before telemetry tests.
     yield (
       api.chromium.run_telemetry_unittests(),
       api.chromium.run_telemetry_perf_unittests(),
     )
 
   def deapply_patch_fn(failing_tests):
     if api.platform.is_win:
       yield api.chromium.taskkill()
     if bot_update_mode:
       yield api.bot_update.ensure_checkout(patch=False,
                                            always_run=True,
                                            update_presentation=False)
     else:
       yield api.gclient.checkout(revert=True, always_run=True),
     yield api.chromium.runhooks(always_run=True),
     compile_targets = list(api.itertools.chain(
                                *[t.compile_targets() for t in failing_tests]))
     if compile_targets:
       yield api.chromium.compile(compile_targets,
                                  name='compile (without patch)',
                                  abort_on_failure=False,
                                  can_fail_build=False,
                                  always_run=True)
       if api.step_history['compile (without patch)'].retcode != 0:
         yield api.chromium.compile(compile_targets,
                                    name='compile (without patch, clobber)',
                                    force_clobber=True,
                                    always_run=True)
+      if use_isolate:
+        yield api.isolate.find_isolated_tests(api.chromium.output_dir,
+                                              always_run=True)
 
   yield api.test_utils.determine_new_failures(tests, deapply_patch_fn)
 
 
 def _sanitize_nonalpha(text):
   return ''.join(c if c.isalnum() else '_' for c in text)
 
 
 def GenTests(api):
   canned_checkdeps = {
@@ skipping 184 matching lines @@
             'test': 'browser_tests',
             'args': '--gtest-filter: *NaCl*',
           }, {
             'test': 'base_tests',
             'args': ['--gtest-filter: *NaCl*'],
           },
         ],
       })
     )
   )
+
+  # Successfully compiling, isolating and running two targets on swarming.
+  # Check both 'swarming: True' and 'swarming: {dict}' test spec definitions.
+  yield (
+    api.test('swarming_basic') +
+    props(buildername='linux_chromium_rel_swarming') +
+    api.platform.name('linux') +
+    api.override_step_data('read test spec', api.json.output({
+        'gtest_tests': [
+          {
+            'test': 'base_unittests',
+            'swarming': True,
+          },
+          {
+            'test': 'browser_tests',
+            'swarming': {
+              'platforms': ['linux'],
+            },
+          },
+        ],
+      })
+    ) +
+    api.override_step_data(
+        'find isolated tests',
+        api.isolate.output_json(['base_unittests', 'browser_tests']))
+  )
+
+  # One target (browser_tests) failed to produce *.isolated file.
+  yield (
+    api.test('swarming_missing_isolated') +
+    props(buildername='linux_chromium_rel_swarming') +
+    api.platform.name('linux') +
+    api.override_step_data('read test spec', api.json.output({
+        'gtest_tests': [
+          {
+            'test': 'base_unittests',
+            'swarming': True,
+          },
+          {
+            'test': 'browser_tests',
+            'swarming': True,
+          },
+        ],
+      })
+    ) +
+    api.override_step_data(
+        'find isolated tests',
+        api.isolate.output_json(['base_unittests']))
+  )
+
+  # One test (base_unittest) failed on swarming. It is retried with
+  # deapplied patch.
+  yield (
+    api.test('swarming_deapply_patch') +
+    props(buildername='linux_chromium_rel_swarming') +
+    api.platform.name('linux') +
+    api.override_step_data('read test spec', api.json.output({
+        'gtest_tests': [
+          {
+            'test': 'base_unittests',
+            'swarming': True,
+          },
+          {
+            'test': 'browser_tests',
+            'swarming': True,
+          },
+        ],
+      })
+    ) +
+    api.override_step_data(
+        'find isolated tests',
+        api.isolate.output_json(['base_unittests', 'browser_tests'])) +
+    api.override_step_data('[swarming] base_unittests (with patch)',
+                           canned_test(passing=False)) +
+    api.override_step_data(
+        'find isolated tests (2)',
+        api.isolate.output_json(['base_unittests']))
+  )