Added support for properly building and running isolates in recipes.

scripts/slave/recipe_modules/isolate/api.py

Index: scripts/slave/recipe_modules/isolate/api.py
diff --git a/scripts/slave/recipe_modules/isolate/api.py b/scripts/slave/recipe_modules/isolate/api.py
index 8696c7d8ad8c0279307411d4039b8b0f2b6517e1..d8187636c8f40cab0cfc3d0348bb4806926f6e16 100644
--- a/scripts/slave/recipe_modules/isolate/api.py
+++ b/scripts/slave/recipe_modules/isolate/api.py
@@ -4,17 +4,32 @@
 
 from slave import recipe_api
 
+ISOLATE_SERVER = 'https://isolateserver.appspot.com'

[iannucci, 2014/01/10 23:50:25]

theoretically, this (and the test_isolation_mode->hashtable) should go in a
config for this module.
practically, this seems like a perfect thing to refactor later when we have more
users and other values for these.

[Ken Russell (switch to Gerrit), 2014/01/11 00:35:38]

Yes, let's keep this simple for now and refactor it later.

+
 class IsolateApi(recipe_api.RecipeApi):
   """APIs for interacting with isolates."""
 
+  def __init__(self, **kwargs):
+    super(IsolateApi, self).__init__(**kwargs)
+    self._manifest_hashes = {}
+
+  def set_isolate_environment(self, config):
+    """Modifies the passed Config (which should generally be api.chromium.c)
+    to set up the appropriate GYP_DEFINES to upload isolates to the isolate
+    server during the build. This must be called early in your recipe;
+    definitely before the checkout and runhooks steps."""
+    config.gyp_env.GYP_DEFINES['test_isolation_mode'] = 'hashtable'
+    config.gyp_env.GYP_DEFINES['test_isolation_outdir'] = ISOLATE_SERVER
+
   @recipe_api.inject_test_data
   def manifest_to_hash(self, targets):
     """Returns a step which runs manifest_to_hash.py against the given array
     of targets. Assigns the result to the swarm_hashes factory property.
     (This implies this step can currently only be run once per recipe.)"""
     def followup_fn(step_result):
+      self._manifest_hashes = step_result.json.output
       step_result.presentation.properties['swarm_hashes'] = (
-        step_result.json.output)
+        self._manifest_hashes)
     return self.m.python(
       'manifest_to_hash',
       self.m.path.build('scripts', 'slave', 'swarming', 'manifest_to_hash.py'),
@@ -22,3 +37,59 @@ class IsolateApi(recipe_api.RecipeApi):
        '--output-json', self.m.json.output(),
       ] + targets,
       followup_fn=followup_fn)
+
+  @property
+  def manifest_hashes(self):
+    """Returns the dictionary of hashes that have been produced during this
+    run. These come either from the incoming swarm_hashes factory property,
+    or from calling manifest_to_hash, above, at some point during the
+    run."""
+    return self.m.properties.get('swarm_hashes', self._manifest_hashes)

[iannucci, 2014/01/10 23:50:25]

Nice :)

+
+  def run_isolate_test(self, test, args=None, xvfb=False, name=None,
+                       annotate=None, results_url=None, perf_dashboard_id=None,
+                       test_type=None, generate_json_file=False,
+                       results_directory=None, build_number=None,
+                       builder_name=None, python_mode=False, spawn_dbus=True,
+                       parallel=False, **kwargs):

[iannucci, 2014/01/10 23:50:25]

You probably should let all of these be taken care of by **kwargs.

def ...(self, test, args=(), **runtest_kwargs):
  ... runtests(..., **runtest_kwargs)

Otherwise you have a potentially conflicting set of default values, and a
duplication of arg names. Also verbose :).

[Ken Russell (switch to Gerrit), 2014/01/11 00:35:38]

Thanks. Fixed this.

+    """Runs a test which has previously been uploaded to the isolate server.
+    Expects to find the test 'test' as a key in the manifest_hashes
+    dictionary."""
+    isolate_hash = self.manifest_hashes[test]
+
+    if not name:
+      name = test

[iannucci, 2014/01/10 23:50:25]

idiomatically, I tend to prefer:
  name = name or test

[Ken Russell (switch to Gerrit), 2014/01/11 00:35:38]

Done.

+
+    # TODO(kbr): will need this later. Comment out for now for code coverage.
+    # The step name must end in 'test' or 'tests' in order for the results to
+    # automatically show up on the flakiness dashboard.
+    # if not (name.endswith('test') or name.endswith('tests')):
+    #   name = '%s_tests' % name
+
+    if not args:
+      args = []

[iannucci, 2014/01/10 23:50:25]

If you make args default to an empty tuple `()`, you don't need this.

If you tried to set the default to [] and got pylint yelling at you, the reason
is that default arguments are evaluated at function definition time, which means
the args default value would be a mutable global across all invocations of this
method (icky).

Unfortunately there are no frozen mapping types in python, so if args were
supposed to be a dict, you'd want to use this idiom:

def ...(..., args=None, ...):
  args = args or {}

[Ken Russell (switch to Gerrit), 2014/01/11 00:35:38]

Unfortunately, [] + () does not work. Leaving this as is for now.

+
+    full_args = [
+      '-H',
+      isolate_hash,
+      '-I',
+      ISOLATE_SERVER
+    ] + args
+
+    return self.m.chromium.runtests(
+      self.m.path.checkout('tools', 'swarming_client', 'run_isolated.py'),
+      args=full_args,
+      name=name,
+      xvfb=xvfb,
+      annotate=annotate,
+      results_url=results_url,
+      perf_dashboard_id=perf_dashboard_id,
+      test_type=test_type,
+      generate_json_file=generate_json_file,
+      results_directory=results_directory,
+      build_number=build_number,
+      builder_name=builder_name,
+      python_mode=python_mode,
+      spawn_dbus=spawn_dbus,
+      parallel=parallel,
+      **kwargs)