Make output placeholders like json.output index-able by name.

recipe_engine/recipe_test_api.py

 # Copyright 2013-2015 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 contextlib
 
 from .util import ModuleInjectionSite, static_call, static_wraps
 from .types import freeze
 
-def combineify(name, dest, a, b):
+def combineify(name, dest, a, b, overwrite=False):
   """
   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
+    overwrite - if True, for the same key, overwrite value from a with the one
+        from b; otherwise, use addition to merge them.
   """
   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
+      if not overwrite:
+        dest_dict[k] += v
+      else:
+        dest_dict[k] = v

[iannucci, 2016/03/22 22:54:24]

I don't think this needs distinction any more: all output placeholders should
overwrite, right?

[stgao, 2016/03/22 23:40:25]

We still need this distinction, as we discussed offline for the following cases:

combineify('mod_data', ret, self, other)
combineify('step_data', ret, self, other)
combineify('depend_on_data', ret, self, other)

     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):
+class InputPlaceholderTestData(BaseTestData):
   def __init__(self, data=None):
-    super(PlaceholderTestData, self).__init__()
+    super(InputPlaceholderTestData, self).__init__()
     self.data = data
 
   def __repr__(self):
-    return "PlaceholderTestData(%r)" % (self.data,)
+    return "InputPlaceholderTestData(%r)" % (self.data,)

[iannucci, 2016/03/22 22:54:24]

we don't need this class at all

[stgao, 2016/03/22 23:40:25]

We need this one for the stdin below at line #139. It's strange to return an
OutputPlaceholderTestData there.

Or should we return None at #139?

+
+
+class OutputPlaceholderTestData(BaseTestData):
+  def __init__(self, data=None, name=None):
+    super(OutputPlaceholderTestData, self).__init__()
+    self.data = data
+    self.name = name
+
+  def __repr__(self):
+    if self.name is None:
+      return "OutputPlaceholderTestData(%r)" % (self.data,)

[iannucci, 2016/03/22 22:54:24]

maybe `OutputPlaceholderTestData(DEFAULT, %r)` ?

[stgao, 2016/03/22 23:40:25]

Done.

+    else:
+      return "OutputPlaceholderTestData(%r, %r)" % (self.name, self.data,)
 
 
 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] }. These are for output placeholders.
-    self.placeholder_data = collections.defaultdict(list)
+    # { (module, placeholder, name) -> data }. Data are for output placeholders.
+    self.placeholder_data = collections.defaultdict(dict)
     self.override = False
     self._stdout = None
     self._stderr = None
     self._retcode = None
 
   def __add__(self, other):
     assert isinstance(other, StepTestData)
 
     if other.override:
       return other
 
     ret = StepTestData()
 
-    combineify('placeholder_data', ret, self, other)
+    combineify('placeholder_data', ret, self, other, overwrite=True)
 
     # pylint: disable=W0212
     ret._stdout = other._stdout or self._stdout
     ret._stderr = other._stderr or self._stderr
     ret._retcode = self._retcode
     if other._retcode is not None:
       assert ret._retcode is None
       ret._retcode = other._retcode
 
     return ret
 
   def unwrap_placeholder(self):
-    # {(module, placeholder): [data]} => data.
+    # {(module, placeholder, name): data} => data.
     assert len(self.placeholder_data) == 1
-    data_list = self.placeholder_data.items()[0][1]
-    assert len(data_list) == 1
-    return data_list[0]
+    return self.placeholder_data.values()[0]
 
-  def pop_placeholder(self, name_pieces):
-    l = self.placeholder_data[name_pieces]
-    if l:
-      return l.pop(0)
-    else:
-      return PlaceholderTestData()
+  def pop_placeholder(self, module_name, placeholder_name, name):
+    return self.placeholder_data.pop(
+        (module_name, placeholder_name, name), OutputPlaceholderTestData())
 
   @property
   def retcode(self):  # pylint: disable=E0202
     return self._retcode or 0
 
   @retcode.setter
   def retcode(self, value):  # pylint: disable=E0202
     self._retcode = value
 
   @property
   def stdout(self):
-    return self._stdout or PlaceholderTestData(None)
+    return self._stdout or OutputPlaceholderTestData(None)
 
   @stdout.setter
   def stdout(self, value):
-    assert isinstance(value, PlaceholderTestData)
+    assert isinstance(value, OutputPlaceholderTestData)
     self._stdout = value
 
   @property
   def stderr(self):
-    return self._stderr or PlaceholderTestData(None)
+    return self._stderr or OutputPlaceholderTestData(None)
 
   @stderr.setter
   def stderr(self, value):
-    assert isinstance(value, PlaceholderTestData)
+    assert isinstance(value, OutputPlaceholderTestData)
     self._stderr = value
 
   @property
   def stdin(self):  # pylint: disable=R0201
-    return PlaceholderTestData(None)
+    return InputPlaceholderTestData(None)
 
   def __repr__(self):
     return "StepTestData(%r)" % ({
       'placeholder_data': dict(self.placeholder_data.iteritems()),
       'stdout': self._stdout,
       'stderr': self._stderr,
       'retcode': self._retcode,
       'override': self.override,
     },)
 
@@ skipping 112 matching lines @@
     },)
 
 
 class DisabledTestData(BaseTestData):
   def __init__(self):
     super(DisabledTestData, self).__init__(False)
 
   def __getattr__(self, name):
     return self
 
-  def pop_placeholder(self, _name_pieces):
+  def pop_placeholder(self, _module_name, _placeholder_name, _name):
     return self
 
   def pop_step_test_data(self, _step_name, _step_test_data_fn):
     return self
 
   def get_module_test_data(self, _module_name):
     return self
 
   @contextlib.contextmanager
   def should_raise_exception(self, exception): # pylint: disable=unused-argument
@@ skipping 10 matching lines @@
     return ret
   return inner
 
 
 def placeholder_step_data(func):
   """Decorates RecipeTestApi member functions to allow those functions to
   return just the output placeholder data, instead of the normally required
   StepTestData() object.
 
   The wrapped function may return either:
-    * <placeholder data>, <retcode or None>
-    * StepTestData containing exactly one PlaceholderTestData and possible a
-      retcode. This is useful for returning the result of another method which
+    * <placeholder data>, <retcode or None>, <name or None>
+    * StepTestData containing exactly one OutputPlaceholderTestData and possible
+      a retcode. This is useful for returning the result of another method which
       is wrapped with placeholder_step_data.
 
   In either case, the wrapper function will return a StepTestData object with
   the retcode and placeholder datum inserted with a name of:
-    (<Test module name>, <wrapped function name>)
+    (<Test module name>, <wrapped function name>, <name>)
 
   Say you had a 'foo_module' with the following RecipeTestApi:
     class FooTestApi(RecipeTestApi):
       @placeholder_step_data
       @staticmethod
-      def cool_method(data, retcode=None):
-        return ("Test data (%s)" % data), retcode
+      def cool_method(data, retcode=None, name=None):
+        return ("Test data (%s)" % data), retcode, name
 
       @placeholder_step_data
-      def other_method(self, retcode=None):
-        return self.cool_method('hammer time', retcode)
+      def other_method(self, retcode=None, name=None):
+        return self.cool_method('hammer time', retcode=retcode, name=name)
 
-  Code calling cool_method('hello') would get a StepTestData:
+  Code calling cool_method('hello', name='cool1') would get a StepTestData:
     StepTestData(
       placeholder_data = {
-        ('foo_module', 'cool_method'): [
-          PlaceholderTestData('Test data (hello)')
-        ]
+        ('foo_module', 'cool_method', 'cool1') :
+          OutputPlaceholderTestData('Test data (hello)')
       },
       retcode = None
     )
 
-  Code calling other_method(50) would get a StepTestData:
+  Code calling other_method(retcode=50, name='other1') would get a StepTestData:
     StepTestData(
       placeholder_data = {
-        ('foo_module', 'other_method'): [
-          PlaceholderTestData('Test data (hammer time)')
-        ]
+        ('foo_module', 'other_method', 'other1'):
+          OutputPlaceholderTestData('Test data (hammer time)')
       },
       retcode = 50
     )
   """
   @static_wraps(func)
   def inner(self, *args, **kwargs):
     assert isinstance(self, RecipeTestApi)
     mod_name = self._module.NAME  # pylint: disable=W0212
     data = static_call(self, func, *args, **kwargs)
     if isinstance(data, StepTestData):
-      all_data = [i
-                  for l in data.placeholder_data.values()
-                  for i in l]
+      all_data = data.placeholder_data.values()
       assert len(all_data) == 1, (
         'placeholder_step_data is only expecting a single output placeholder '
         'datum. Got: %r' % data
       )
       placeholder_data, retcode = all_data[0], data.retcode
     else:
-      placeholder_data, retcode = data
-      placeholder_data = PlaceholderTestData(placeholder_data)
+      placeholder_data, retcode, name = data
+      placeholder_data = OutputPlaceholderTestData(
+          data=placeholder_data, name=name)
 
     ret = StepTestData()
-    ret.placeholder_data[(mod_name, inner.__name__)].append(placeholder_data)
+    key = (mod_name, inner.__name__, placeholder_data.name)
+    ret.placeholder_data[key] = placeholder_data
     ret.retcode = retcode
     return ret
   return inner
 
 
 class RecipeTestApi(object):
   """Provides testing interface for GenTest method.
 
   There are two primary components to the test api:
     * Test data creation methods (test and step_data)
     * test_api's from all the modules in DEPS.
 
   Every test in GenTests(api) takes the form:
     yield <instance of TestData>
 
   There are 4 basic pieces to TestData:
     name       - The name of the test.
     properties - Simple key-value dictionary which is used as the combined
                  build_properties and factory_properties for this test.
     mod_data   - Module-specific testing data (see the platform module for a
                  good example). This is testing data which is only used once at
                  the start of the execution of the recipe. Modules should
                  provide methods to get their specific test information. See
                  the platform module's test_api for a good example of this.
     step_data  - Step-specific data. There are two major components to this.
         retcode          - The return code of the step
-        placeholder_data - A mapping from placeholder name to the a list of
-                           PlaceholderTestData objects, one for each instance
-                           of that kind of Placeholder in the step.
-        stdout, stderr   - PlaceholderTestData objects for stdout and stderr.
+        placeholder_data - A mapping from placeholder name to the
+                           OutputPlaceholderTestData object in the step.
+        stdout, stderr   - OutputPlaceholderTestData objects for stdout and
+                           stderr.
 
   TestData objects are concatenatable, so it's convenient to phrase test cases
   as a series of added TestData objects. For example:
     DEPS = ['properties', 'platform', 'json']
     def GenTests(api):
       yield (
         api.test('try_win64') +
         api.properties.tryserver(power_level=9001) +
         api.platform('win', 64) +
         api.step_data(
           'some_step',
-          api.json.output("bobface"),
-          api.json.output({'key': 'value'})
+          api.json.output("bobface", name="a"),
+          api.json.output({'key': 'value'}, name="b")
         )
       )
 
   This example would run a single test (named 'try_win64') with the standard
   tryserver properties (plus an extra property 'power_level' whose value was
   over 9000).  The test would run as if it were being run on a 64-bit windows
-  installation, and the step named 'some_step' would have its first json output
-  placeholder be mocked to return '"bobface"', and its second json output
-  placeholder be mocked to return '{"key": "value"}'.
+  installation, and the step named 'some_step' would have the json output of
+  the placeholder with name "a" be mocked to return '"bobface"', and the json
+  output of the placeholder with name "b" be mocked to return
+  '{"key": "value"}'.
 
   The properties.tryserver() call is documented in the 'properties' module's
   test_api.
   The platform() call is documented in the 'platform' module's test_api.
   The json.output() call is documented in the 'json' module's test_api.
   """
   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'
@@ skipping 19 matching lines @@
     return TestData(None)
 
   @staticmethod
   def _step_data(name, *data, **kwargs):
     """Returns a new TestData with the mock data filled in for a single step.
 
     Used by step_data and override_step_data.
 
     Args:
       name - The name of the step we're providing data for
-      data - Zero or more StepTestData objects. These may fill in placeholder
-             data for zero or more modules, as well as possibly setting the
-             retcode for this step.
+      data - Zero or more StepTestData objects. These may fill in output
+             placeholder data for zero or more modules, as well as possibly
+             setting the retcode for this step.
       retcode=(int or None) - Override the retcode for this step, even if it
              was set by |data|. This must be set as a keyword arg.
       stdout - StepTestData object with a single output placeholder datum for a
              step's stdout.
       stderr - StepTestData object with a single output placeholder datum for a
              step's stderr.
       override=(bool) - This step data completely replaces any previously
              generated step data, instead of adding on to it.
 
     Use in GenTests:
@@ skipping 27 matching lines @@
     if data:
       ret.step_data[name] = reduce(lambda x,y: x + y, data)
     if 'retcode' in kwargs:
       ret.step_data[name].retcode = kwargs['retcode']
     if 'override' in kwargs:
       ret.step_data[name].override = kwargs['override']
     for key in ('stdout', 'stderr'):
       if key in kwargs:
         stdio_test_data = kwargs[key]
         assert isinstance(stdio_test_data, StepTestData)
-        setattr(ret.step_data[name], key, stdio_test_data.unwrap_placeholder())
+        setattr(ret.step_data[name], key,
+                stdio_test_data.unwrap_placeholder())
     return ret
 
   def step_data(self, name, *data, **kwargs):
     """See _step_data()"""
     return self._step_data(name, *data, **kwargs)
   step_data.__doc__ = _step_data.__doc__
 
   def override_step_data(self, name, *data, **kwargs):
     """See _step_data()"""
     kwargs['override'] = True
     return self._step_data(name, *data, **kwargs)
   override_step_data.__doc__ = _step_data.__doc__
 
   def expect_exception(self, exc_type): #pylint: disable=R0201
     ret = TestData(None)
     ret.expect_exception(exc_type)
     return ret
 
   def depend_on(self, recipe, properties, result):
     ret = TestData()
     ret.depend_on(recipe, properties, result)
     return ret