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

recipe_engine/step_runner.py

 # Copyright (c) 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 cStringIO
 import collections
 import contextlib
 import datetime
 import json
 import os
@@ skipping 431 matching lines @@
     step_test: The test data json dictionary for this step, if any.
                Passed through unaltered to each placeholder.
 
   Returns the rendered step and a Placeholders object representing any
   placeholder instances that were found while rendering.
   """
   rendered_step = dict(step)
 
   # Process 'cmd', rendering placeholders there.
   input_phs = collections.defaultdict(lambda: collections.defaultdict(list))
-  output_phs = collections.defaultdict(lambda: collections.defaultdict(list))
+  output_phs = collections.defaultdict(
+      lambda: collections.defaultdict(collections.OrderedDict))
   new_cmd = []
   for item in step.get('cmd', []):
     if isinstance(item, util.Placeholder):
-      module_name, placeholder_name = item.name_pieces
-      tdata = step_test.pop_placeholder(item.name_pieces)
+      module_name, placeholder_name = item.namespaces
+      tdata = step_test.pop_placeholder(
+          module_name, placeholder_name, item.name)
       new_cmd.extend(item.render(tdata))
       if isinstance(item, util.InputPlaceholder):
         input_phs[module_name][placeholder_name].append((item, tdata))
       else:
         assert isinstance(item, util.OutputPlaceholder), (
             'Not an OutputPlaceholder: %r' % item)
-        output_phs[module_name][placeholder_name].append((item, tdata))
+        # This assert also ensures at most one placeholder has the default name.

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

well, it also asserts (correctly) that you don't have two placeholders with the
name 'a', too, right?

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

Yes. Updated.

+        assert item.name not in output_phs[module_name][placeholder_name], (
+            'Step "%s" has multiple output placeholders of %s.%s. Please '
+            'specify explicit and different names for them.' % (
+              step['name'], module_name, placeholder_name))
+        output_phs[module_name][placeholder_name][item.name] = (item, tdata)
     else:
       new_cmd.append(item)
   rendered_step['cmd'] = new_cmd
 
   # Process 'stdout', 'stderr' and 'stdin' placeholders, if given.
   stdio_placeholders = {}
   for key in ('stdout', 'stderr', 'stdin'):
     placeholder = step.get(key)
     tdata = None
     if placeholder:
@@ skipping 29 matching lines @@
     for _, items in pholders.iteritems():
       for ph, td in items:
         ph.result(step_result.presentation, td)
 
   # Output placeholders inside step |cmd|.
   for module_name, pholders in placeholders.outputs_cmd.iteritems():
     assert not hasattr(step_result, module_name)
     o = BlankObject()
     setattr(step_result, module_name, o)
 
-    for placeholder_name, items in pholders.iteritems():
-      lst = [ph.result(step_result.presentation, td) for ph, td in items]
-      setattr(o, placeholder_name+"_all", lst)
-      setattr(o, placeholder_name, lst[0])
+    for placeholder_name, instances in pholders.iteritems():
+      named_results = {}
+      default_result = None
+      for _, (ph, td) in instances.iteritems():
+        result = ph.result(step_result.presentation, td)
+        if ph.name is None:
+          default_result = result
+        else:
+          named_results[ph.name] = result
+      setattr(o, placeholder_name + "s", named_results)
+
+      if default_result is None and len(named_results) == 1:
+        # Only 1 output placeholder with an explicit name.
+        default_result = named_results.values()[0]
+      setattr(o, placeholder_name, default_result)

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

what if there's two placeholders with names, and no 'None' names.

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

Then the default output is set to None, per our discussion earlier.
Or should we not set it at all in this case?

 
   # Placeholders that are used with IO redirection.
   for key in ('stdout', 'stderr', 'stdin'):
     assert not hasattr(step_result, key)
     ph, td = getattr(placeholders, key)
     result = ph.result(step_result.presentation, td) if ph else None
-    setattr(step_result, key, result)
+    if key != 'stdin':
+      setattr(step_result, key, result)
 
   return step_result
 
 
 def _merge_envs(original, override):
   """Merges two environments.
 
   Returns a new environment dict with entries from |override| overwriting
   corresponding entries in |original|. Keys whose value is None will completely
   remove the environment variable. Values can contain %(KEY)s strings, which
@@ skipping 40 matching lines @@
   supplied command, and only uses the |env| kwarg for modifying the environment
   of the child process.
   """
   saved_path = os.environ['PATH']
   try:
     if path is not None:
       os.environ['PATH'] = path
     yield
   finally:
     os.environ['PATH'] = saved_path