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

recipe_engine/recipe_test_api.py

Index: recipe_engine/recipe_test_api.py
diff --git a/recipe_engine/recipe_test_api.py b/recipe_engine/recipe_test_api.py
index 117bfe33ae8d39eb769454d3f5a0bf14309fbdce..f5fc2ca327d42f3b0c9e67cd3538feef81e8757c 100644
--- a/recipe_engine/recipe_test_api.py
+++ b/recipe_engine/recipe_test_api.py
@@ -8,7 +8,7 @@ import contextlib
 from . import util
 from .types import freeze
 
-def combineify(name, dest, a, b):
+def combineify(name, dest, a, b, allow_merge=True):
   """
   Combines dictionary members in two objects into a third one using addition.
 
@@ -17,11 +17,13 @@ def combineify(name, dest, a, b):
     dest - the destination object
     a - the first source object
     b - the second source object
+    allow_merge - whether to allow merge values from a and b with the same key.
   """
   dest_dict = getattr(dest, name)
   dest_dict.update(getattr(a, name))
   for k, v in getattr(b, name).iteritems():
     if k in dest_dict:
+      assert allow_merge, '2+ test data has the same key %r' % k

[iannucci, 2016/03/12 03:43:55]

I'm not sure if this error will be very clear?

[stgao, 2016/03/22 05:58:04]

Removed, as this assert will make GenTests fail to override the default mocked
output in modules.

       dest_dict[k] += v
     else:
       dest_dict[k] = v
@@ -37,13 +39,26 @@ class BaseTestData(object):
     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,)
+
+
+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,)
+    else:
+      return "OutputPlaceholderTestData(%r, %r)" % (self.name, self.data,)
 
 
 class StepTestData(BaseTestData):
@@ -57,7 +72,8 @@ class StepTestData(BaseTestData):
     super(StepTestData, self).__init__()
     # { (module, placeholder) -> [data] }
     self.input_placeholder_data = collections.defaultdict(list)
-    self.output_placeholder_data = collections.defaultdict(list)
+    # { (module, placeholder, name) -> data }
+    self.output_placeholder_data = collections.defaultdict(dict)
     self.override = False
     self._stdout = None
     self._stderr = None
@@ -72,7 +88,7 @@ class StepTestData(BaseTestData):
     ret = StepTestData()
 
     combineify('input_placeholder_data', ret, self, other)
-    combineify('output_placeholder_data', ret, self, other)
+    combineify('output_placeholder_data', ret, self, other, allow_merge=False)

[iannucci, 2016/03/12 03:43:55]

We should make sure that the following case still works:

  # in module, provides default mocked output with name 'main_output'
  api.step("step", ..., step_test_data=lambda: self.m.raw_io.test_api.output(
     'something', name="main_output"))

  # in GenTests, can override that mocked output
  yield (api.test("name") + 
         api.step_data("step",
           api.raw_io.output("other", name="main_output"))

I'm assuming this assertion here just prevents you from doing

  + api.step_data("step", api.raw_io.output("foo"), api.raw_io.output("bar"))

OR

  + api.step_data("step", api.raw_io.output("foo"))
  + api.step_data("step", api.raw_io.output("bar"))

?

[stgao, 2016/03/22 05:58:04]

Added a testcase in recipe_modules/raw_io/example.py
And the assertion above was fired, thus I removed the assertion.
Yes, they are the cases the assert was intended for. But it seems we can't do it
now for the first case you mentioned.

 
     # pylint: disable=W0212
     ret._stdout = other._stdout or self._stdout
@@ -85,24 +101,20 @@ class StepTestData(BaseTestData):
     return ret
 
   def unwrap_output_placeholder(self):
-    # {(module, placeholder): [data]} => data.
+    # {(module, placeholder, name): data} => data.
     assert len(self.output_placeholder_data) == 1
-    data_list = self.output_placeholder_data.items()[0][1]
-    assert len(data_list) == 1
-    return data_list[0]
+    return self.output_placeholder_data.values()[0]
 
-  def _pop_placeholder(self, name_pieces, placeholder_data):
-    l = placeholder_data[name_pieces]
+  def pop_input_placeholder(self, module_name, placeholder_name):
+    l = self.input_placeholder_data[(module_name, placeholder_name)]
     if l:
       return l.pop(0)
     else:
-      return PlaceholderTestData()
-
-  def pop_input_placeholder(self, name_pieces):
-    return self._pop_placeholder(name_pieces, self.input_placeholder_data)
+      return InputPlaceholderTestData()
 
-  def pop_output_placeholder(self, name_pieces):
-    return self._pop_placeholder(name_pieces, self.output_placeholder_data)
+  def pop_output_placeholder(self, module_name, placeholder_name, name):
+    return self.output_placeholder_data.pop(
+        (module_name, placeholder_name, name), OutputPlaceholderTestData())
 
   @property
   def retcode(self):  # pylint: disable=E0202
@@ -114,29 +126,30 @@ class StepTestData(BaseTestData):
 
   @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()),
+      'output_placeholder_data': dict(self.output_placeholder_data.iteritems()),
+      'input_placeholder_data': dict(self.input_placeholder_data.iteritems()),
       'stdout': self._stdout,
       'stderr': self._stderr,
       'retcode': self._retcode,
@@ -265,10 +278,10 @@ class DisabledTestData(BaseTestData):
   def __getattr__(self, name):
     return self
 
-  def pop_input_placeholder(self, _name_pieces):
+  def pop_input_placeholder(self, _module_name, _placeholder_name):
     return self
 
-  def pop_output_placeholder(self, _name_pieces):
+  def pop_output_placeholder(self, _module_name, _placeholder_name, _name):
     return self
 
   def pop_step_test_data(self, _step_name, _step_test_data_fn):
@@ -299,42 +312,40 @@ def placeholder_step_data(func):
   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(
       output_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(
       output_placeholder_data = {
-        ('foo_module', 'other_method'): [
-          PlaceholderTestData('Test data (hammer time)')
-        ]
+        ('foo_module', 'other_method', 'other1'):
+          OutputPlaceholderTestData('Test data (hammer time)')
       },
       retcode = 50
     )
@@ -352,12 +363,13 @@ def placeholder_step_data(func):
       )
       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.output_placeholder_data[(mod_name, inner.__name__)].append(
-        placeholder_data)
+    key = (mod_name, inner.__name__, placeholder_data.name)
+    ret.output_placeholder_data[key] = placeholder_data
     ret.retcode = retcode
     return ret
   return inner
@@ -384,10 +396,10 @@ class RecipeTestApi(object):
                  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.
+        output_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:
@@ -399,17 +411,18 @@ class RecipeTestApi(object):
         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.
@@ -449,9 +462,9 @@ class RecipeTestApi(object):
 
     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