Refactor the way that TestApi works so that it is actually useful.

scripts/slave/recipe_api.py

Index: scripts/slave/recipe_api.py
diff --git a/scripts/slave/recipe_api.py b/scripts/slave/recipe_api.py
index 91fcc12ab205db4d3a126b0381a9f6d5686e99e4..6ba1ce3713e6f1f938982d621f465a866105b6fa 100644
--- a/scripts/slave/recipe_api.py
+++ b/scripts/slave/recipe_api.py
@@ -2,6 +2,7 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
+import copy
 import functools
 import imp
 import inspect
@@ -48,6 +49,12 @@ class InputDataPlaceholder(Placeholder):
 class ModuleInjectionSite(object):
   pass
 
+class RecipeTestApi(object):
+  def __init__(self, module=None):
+    """Note: Injected dependencies are NOT available in __init__()."""
+    # If we're the 'root' api, inject directly into 'self'.
+    # Otherwise inject into 'self.m'
+    self.m = self if module is None else ModuleInjectionSite()
 
 class RecipeApi(object):
   """
@@ -69,6 +76,10 @@ class RecipeApi(object):
     # Otherwise inject into 'self.m'
     self.m = self if module is None else ModuleInjectionSite()
 
+    # If our module has a test api, it gets injected here.
+    self.test_api = None
+
+
   def _sanitize_config_vars(self, config_name, CONFIG_VARS):
     params = self.get_config_defaults(config_name)
     params.update(CONFIG_VARS)
@@ -121,6 +132,7 @@ def load_recipe_modules(mod_dirs):
   def patchup_module(submod):
     submod.CONFIG_CTX = getattr(submod, 'CONFIG_CTX', None)
     submod.API = getattr(submod, 'API', None)
+    submod.TEST_API = getattr(submod, 'TEST_API', None)
     submod.DEPS = frozenset(getattr(submod, 'DEPS', ()))
 
     if hasattr(submod, 'config'):
@@ -139,6 +151,17 @@ def load_recipe_modules(mod_dirs):
 
     assert submod.API, 'Submodule has no api? %s' % (submod)
 
+    if hasattr(submod, 'test_api'):
+      for v in submod.test_api.__dict__.itervalues():
+        if inspect.isclass(v) and issubclass(v, RecipeTestApi):
+          assert not submod.TEST_API, (
+            'More than one TestApi subclass: %s' % submod.api)
+          submod.TEST_API = v
+      assert submod.API, (
+        'Submodule has test_api.py but no TestApi subclass? %s'
+        % (submod)
+      )
+
   RM = 'RECIPE_MODULES'
   def find_and_load(fullname, modname, path):
     if fullname not in sys.modules or fullname == RM:
@@ -207,7 +230,8 @@ def load_recipe_modules(mod_dirs):
     imp.release_lock()
 
 
-def CreateRecipeApi(names, mod_dirs, mocks=None, **kwargs):
+def CreateApi(mod_dirs, names, mocks=None, required=None,

[agable, 2013/09/16 20:29:25]

If 'required' and 'optional' are not antitheses of each other, their names
shouldn't be.

+              optional=None, kwargs=None):
   """
   Given a list of module names, return an instance of RecipeApi which contains

[agable, 2013/09/16 20:29:25]

Update docstring to reflect new argument ordering.

   those modules as direct members.
@@ -221,16 +245,21 @@ def CreateRecipeApi(names, mod_dirs, mocks=None, **kwargs):
     mod_dirs (list): A list of paths to directories which contain modules.
     mocks (dict): An optional dict of {<modname>: <mock data>}. Each module
         expects its own mock data.
-    **kwargs: Data passed to each module api. Usually this will contain:
+    kwargs: Data passed to each module api. Usually this will contain:
         properties (dict): the properties dictionary (used by the properties
             module)
         step_history (OrderedDict): the step history object (used by the
             step_history module!)
   """
-
+  kwargs = kwargs or {}
   recipe_modules = load_recipe_modules(mod_dirs)
 
-  inst_map = {None: RecipeApi()}
+  inst_maps = {}
+  if required:

[agable, 2013/09/16 20:29:25]

Definitely not a fan of the 'required' and 'optional' names. Better docstring
will help, but something else needs to be fixed here too. Still thinking about
it.

+    inst_maps[required[0]] = { None: required[1]() }
+  if optional:
+    inst_maps[optional[0]] = { None: optional[1]() }
+
   dep_map = {None: set(names)}
   def create_maps(name):
     if name not in dep_map:
@@ -240,11 +269,29 @@ def CreateRecipeApi(names, mod_dirs, mocks=None, **kwargs):
       map(create_maps, dep_map[name])
 
       mock = None if mocks is None else mocks.get(name, {})
-      inst_map[name] = module.API(module=module, mock=mock, **kwargs)
+      if required:
+        api = getattr(module, required[0])
+        inst_maps[required[0]][name] = api(module=module, mock=mock, **kwargs)
+      if optional:
+        api = getattr(module, optional[0], None) or optional[1]
+        inst_maps[optional[0]][name] = api(module=module)

[agable, 2013/09/16 20:29:25]

As long as you're treating these as "required" and "optional" rather than "real"
and "test", they should both have mocks. I think we should just be explicit
about the test-helper nature of "optional".

+
   map(create_maps, names)
 
+  if required:
+    MapDependencies(dep_map, inst_maps[required[0]])
+  if optional:
+    MapDependencies(dep_map, inst_maps[optional[0]])
+    if required:
+      for name, module in inst_maps[required[0]].iteritems():
+        module.test_api = inst_maps[optional[0]][name]
+
+  return inst_maps[(required or optional)[0]][None]
+
+
+def MapDependencies(dep_map, inst_map):

[agable, 2013/09/16 20:29:25]

While we're here, please rename "to_remove" and "to_pop" to better names: e.g.
"done_deps" and "satisfied_modules" or something like that.

   # NOTE: this is 'inefficient', but correct and compact.
-  did_something = True
+  dep_map = copy.deepcopy(dep_map)
   while dep_map:
     did_something = False
     to_pop = []
@@ -264,7 +311,15 @@ def CreateRecipeApi(names, mod_dirs, mocks=None, **kwargs):
     map(dep_map.pop, to_pop)
     assert did_something, 'Did nothing on this loop. %s' % dep_map
 
-  return inst_map[None]
+
+def CreateRecipeApi(mod_dirs, names, mocks=None, **kwargs):
+  return CreateApi(mod_dirs, names, mocks=mocks, kwargs=kwargs,
+                   required=('API', RecipeApi),
+                   optional=('TEST_API', RecipeTestApi))
+
+
+def CreateTestApi(mod_dirs, names):
+  return CreateApi(mod_dirs, names, optional=('TEST_API', RecipeTestApi))
 
 
 def wrap_followup(kwargs, pre=False):