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

scripts/slave/recipe_test_api.py

+import collections
+import copy
+import functools
+
+from .recipe_util import ModuleInjectionSite
+
+def combineify(name, dest, a, b):
+  """
+  Combines dictionary members in two objects into a third one using addition.
+
+  Args:
+    name - the name of the member
+    dest - the destination object
+    a - the first source object
+    b - the second source object
+  """
+  dest_dict = getattr(dest, name)
+  dest_dict.update(getattr(a, name))
+  for k, v in getattr(b, name).iteritems():
+    if k in dest_dict:
+      dest_dict[k] += v
+    else:
+      dest_dict[k] = v
+
+
+class BaseTestData(object):
+  def __init__(self, enabled=True):
+    super(BaseTestData, self).__init__()
+    self._enabled = enabled
+
+  @property
+  def enabled(self):
+    return self._enabled
+
+
+class PlaceholderTestData(BaseTestData):
+  def __init__(self, data=None):
+    super(PlaceholderTestData, self).__init__()
+    self.data = data
+
+  def __repr__(self):
+    return "PlaceholderTestData(%s)" % self.data
+
+  def __copy__(self):
+    ret = PlaceholderTestData()
+    ret.data = copy.deepcopy(self.data)
+    return ret
+
+
+class StepTestData(BaseTestData):
+  """
+  Mutable container for per-step test data.
+
+  This data is consumed while running the recipe (during
+  annotated_run.run_steps).
+  """
+  def __init__(self):
+    super(StepTestData, self).__init__()
+    # { (module, placeholder) -> [data] }
+    self.placeholder_data = collections.defaultdict(list)
+    self._retcode = None
+
+  def __add__(self, other):
+    assert isinstance(other, StepTestData)
+    ret = StepTestData()
+
+    # We don't use combineify to merge placeholder_data here because
+    # simply concatenating placeholder_data's list value is not meaningful to
+    # consumers of this object.
+    # Producers of this object should use the append() method.
+    ret.placeholder_data.update(copy.deepcopy(self.placeholder_data))
+    for k, v in other.placeholder_data.iteritems():
+      assert k not in ret.placeholder_data
+      ret.placeholder_data[k] = copy.deepcopy(v)
+
+    # pylint: disable=W0212
+    ret._retcode = self._retcode
+    if other._retcode is not None:
+      assert ret._retcode is None
+      ret._retcode = other._retcode
+    return ret
+
+  def append(self, other):
+    self._retcode = self._retcode or other._retcode  # pylint: disable=W0212
+    combineify('placeholder_data', self, self, other)
+    return self
+
+  def pop_placeholder(self, name_pieces):
+    l = self.placeholder_data[name_pieces]
+    if l:
+      return l.pop(0)
+    else:
+      return PlaceholderTestData()
+
+  @property
+  def retcode(self):  # pylint: disable=E0202
+    return self._retcode or 0
+
+  @retcode.setter
+  def retcode(self, value):  # pylint: disable=E0202
+    self._retcode = value
+
+  def __repr__(self):
+    return "StepTestData(%s)" % str({
+      'placeholder_data': dict(self.placeholder_data.iteritems()),
+      'retcode': self._retcode,
+    })
+
+
+class ModuleTestData(BaseTestData, dict):
+  """
+  Mutable container for test data for a specific module.
+
+  This test data is consumed at module load time (i.e. when CreateRecipeApi
+  runs).
+  """
+  def __add__(self, other):
+    assert isinstance(other, ModuleTestData)
+    ret = ModuleTestData()
+    ret.update(self)
+    ret.update(other)
+    return ret
+
+  def __repr__(self):
+    return "ModuleTestData(%s)" % super(ModuleTestData, self).__repr__()
+
+
+class TestData(BaseTestData):
+  def __init__(self, name=None):
+    super(TestData, self).__init__()
+    self.name = name
+    self.properties = {}  # key -> val
+    self.mod_data = collections.defaultdict(ModuleTestData)
+    self.step_data = collections.defaultdict(StepTestData)
+
+  def __add__(self, other):
+    assert isinstance(other, TestData)
+    ret = TestData(self.name or other.name)
+
+    ret.properties.update(self.properties)
+    ret.properties.update(other.properties)
+
+    combineify('mod_data', ret, self, other)
+    combineify('step_data', ret, self, other)
+
+    return ret
+
+  def empty(self):
+    return not self.step_data
+
+  def __repr__(self):
+    return "TestData(%s)" % str({
+      'name': self.name,
+      'properties': self.properties,
+      'mod_data': dict(self.mod_data.iteritems()),
+      'step_data': dict(self.step_data.iteritems()),
+    })
+
+
+class DisabledTestData(BaseTestData):
+  def __init__(self):
+    super(DisabledTestData, self).__init__(False)
+
+  def __getattr__(self, name):
+    return self
+
+  def pop_placeholder(self, _name_pieces):
+    return self
+
+
+def static_wraps(func):
+  wrapped_fn = func
+  if isinstance(func, staticmethod):
+    wrapped_fn = func.__func__
+  return functools.wraps(wrapped_fn)
+
+
+def static_call(obj, func, *args, **kwargs):
+  if isinstance(func, staticmethod):
+    return func.__get__(obj)(*args, **kwargs)
+  else:
+    return func(obj, *args, **kwargs)
+
+
+def mod_test_data(func):
+  @static_wraps(func)
+  def inner(self, *args, **kwargs):
+    assert isinstance(self, RecipeTestApi)
+    mod_name = self._module.NAME  # pylint: disable=W0212
+    ret = TestData(None)
+    data = static_call(self, func, *args, **kwargs)
+    ret.mod_data[mod_name][inner.__name__] = data
+    return ret
+  return inner
+
+
+def placeholder_step_data(func):
+  @static_wraps(func)
+  def inner(self, *args, **kwargs):
+    assert isinstance(self, RecipeTestApi)
+    mod_name = self._module.NAME  # pylint: disable=W0212
+    ret = StepTestData()
+    placeholder_data, retcode = static_call(self, func, *args, **kwargs)
+    ret.placeholder_data[(mod_name, inner.__name__)].append(
+      PlaceholderTestData(placeholder_data))
+    ret.retcode = retcode
+    return ret
+  return inner
+
+
+class RecipeTestApi(object):

[agable, 2013/09/23 22:57:58]

You're going to have to document this *really well*. In the recipe world,
virtually all members of api (api.properties, api.step, etc) are explicitly
declared in DEPS (and those that aren't, e.g. apply_config, I think are
confusing already and would like to move somewhere else, though I don't know
where.). For testing, there are suddenly these two extra test and step_data
methods that magically appear. Document these (and the methods on RecipeApi
itself) really really really really well.

[iannucci, 2013/09/24 02:18:51]

Well... with a data set of 1, all things are unexpected.

I think the basic flow is:
  * GenSteps gets a RecipeApi (which has some methods). Into that object are
also injected all DEPS api's
  * GenTests gets a RecipeTestApi (which has some methods). Into that object are
also injected all test_api's.

They do need docs though.

+  def __init__(self, module=None, test_data=DisabledTestData()):
+    """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()
+    self._module = module
+
+    assert isinstance(test_data, (ModuleTestData, DisabledTestData))
+    self._test_data = test_data
+
+  @staticmethod
+  def test(name):
+    return TestData(name)
+
+  @staticmethod
+  def step_data(name, *data):
+    assert all(isinstance(d, StepTestData) for d in data)
+    ret = TestData(None)
+    ret.step_data[name] = reduce(sum, data)
+    return ret