Split Placeholder into InputPlaceholder and OutputPlaceholder.

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 413 matching lines @@
             self._test_data.step_data.keys(),
             self._test_data.expected_exception))
 
   @property
   def steps_ran(self):
     return self._step_history.values()
 
 
 # Result of 'render_step'.
 Placeholders = collections.namedtuple(
-    'Placeholders', ['cmd', 'stdout', 'stderr', 'stdin'])
+    'Placeholders', ['inputs_cmd', 'outputs_cmd', 'stdout', 'stderr', 'stdin'])
 
 
 def render_step(step, step_test):
   """Renders a step so that it can be fed to annotator.py.
 
   Args:
     step: The step to render.
     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.
-  placeholders = collections.defaultdict(lambda: collections.defaultdict(list))
+  input_phs = collections.defaultdict(lambda: collections.defaultdict(list))
+  output_phs = collections.defaultdict(lambda: collections.defaultdict(list))
   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)
+      if isinstance(item, util.InputPlaceholder):
+        tdata = step_test.pop_input_placeholder(item.name_pieces)

[iannucci, 2016/03/12 03:36:12]

not needed: inputs are never mocked

[stgao, 2016/03/22 05:56:43]

Reverted to the old approach.

We still need a default mock for inputs, otherwise exceptions will be thrown
when the underlying code check test_data.enabled (e.g. in
raw_io.InputDataPlaceholder.render).

+        input_phs[module_name][placeholder_name].append((item, tdata))
+      else:
+        assert isinstance(item, util.OutputPlaceholder), (
+            'Not an OutputPlaceholder: %r' % item)
+        tdata = step_test.pop_output_placeholder(item.name_pieces)
+        output_phs[module_name][placeholder_name].append((item, tdata))
       new_cmd.extend(item.render(tdata))
-      placeholders[module_name][placeholder_name].append((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:
-      assert isinstance(placeholder, util.Placeholder), key
+      if key == 'stdin':
+        assert isinstance(placeholder, util.InputPlaceholder), key
+      else:
+        assert isinstance(placeholder, util.OutputPlaceholder), key
       tdata = getattr(step_test, key)
       placeholder.render(tdata)
       assert placeholder.backing_file
       rendered_step[key] = placeholder.backing_file
     stdio_placeholders[key] = (placeholder, tdata)
 
-  return rendered_step, Placeholders(cmd=placeholders, **stdio_placeholders)
+  return rendered_step, Placeholders(
+      inputs_cmd=input_phs, outputs_cmd=output_phs, **stdio_placeholders)
 
 
 def construct_step_result(step, retcode, placeholders):
   """Constructs a StepData step result from step return data.
 
-  The main purpose of this function is to add placeholder results into the
-  step result where placeholders appeared in the input step.
+  The main purpose of this function is to add output placeholder results into
+  the step result where output placeholders appeared in the input step.
+  Also give input placeholders the chance to do the clean-up if needed.
   """
 
   step_result = types.StepData(step, retcode)
 
   class BlankObject(object):
     pass
 
-  # Placeholders inside step |cmd|.
-  for module_name, pholders in placeholders.cmd.iteritems():
+  # Input placeholders inside step |cmd|.
+  for _, pholders in placeholders.inputs_cmd.iteritems():
+    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])
 
   # 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':

[iannucci, 2016/03/12 03:36:12]

this looks wrong to me...? Not sure what this is supposed to be for?

[stgao, 2016/03/22 05:56:43]

Reverted.

However, I thought we don't want to include all inputs in the step result, as
recipes should not refer to the inputs. I might have misunderstood what you said
earlier.

For the ``input_phs`` above, do we also want to include them in the step result?

+      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