Add initial postprocess unit test thingy.

recipe_engine/simulation_test.py

 # Copyright 2014 The LUCI Authors. All rights reserved.
 # Use of this source code is governed under the Apache License, Version 2.0
 # that can be found in the LICENSE file.
 
 """Provides simulator test coverage for individual recipes."""
 
 import StringIO
+import ast
 import contextlib
+import copy
 import json
 import logging
 import os
 import re
 import sys
+import textwrap
+import traceback
+
+from collections import OrderedDict
 
 from . import env
 from . import stream
+import astunparse
 import expect_tests
 
 # This variable must be set in the dynamic scope of the functions in this file.
 # We do this instead of passing because the threading system of expect tests
 # doesn't know how to serialize it.
 _UNIVERSE = None
 
 
-def RenderExpectation(test_data, raw_expectations):
-  """Applies the step filters (e.g. whitelists, etc.) to the raw_expectations,
-  if the TestData actually contains any filters.
+
+linecache = {}
+def getLines(filename):

[dnj, 2016/10/04 16:42:35]

nit: Add underscores to these global functions and variables so it's clear
they're not intended to be used outside of this file.

[iannucci, 2016/10/06 20:28:09]

Done.

+  filename = os.path.abspath(filename)
+  if filename in linecache:
+    return linecache[filename]
+
+  with open(filename, 'r') as f:
+    ret = f.readlines()
+    linecache[filename] = ret
+    return ret
+
+
+def getReferencedNamesAndNode(filename, lineno):
+  lines = getLines(filename)
+  i = lineno-1
+  for i in range(lineno-1, 0, -1):

[dnj, 2016/10/04 16:42:35]

nit: xrange

[dnj, 2016/10/06 21:20:19]

(This and other comments in this file).

[iannucci, 2016/10/06 22:47:11]

oops

+    try:
+      to_parse = ''.join(lines[i:lineno]).strip()
+      node = ast.parse(to_parse)
+      break
+    except SyntaxError:
+      continue

[martiniss, 2016/10/03 19:14:19]

Why is this here? comment? :)

[iannucci, 2016/10/06 22:47:11]

Done.

+  names = set(n.id for n in ast.walk(node) if isinstance(n, ast.Name))
+  return (names, node, i)
+
+
+class Checker(object):
+  def __init__(self):
+    self._failed_checks = []
+    self._ignore_set = [self]

[martiniss, 2016/10/03 19:14:19]

Don't really understand ignore set

[iannucci, 2016/10/06 22:47:11]

Done.

+
+  def _ignore(self, obj):
+    self._ignore_set.extend(obj)
+
+  def _inner(self, hint, exp):

[martiniss, 2016/10/03 19:14:19]

Some docs on the magic in this function would be nice :)

[dnj, 2016/10/04 16:42:34]

Since "_inner" isn't recursive or anything, let's name it something more
intuitive. Suggest: "_call_impl" or "_resolve_and_append_check"?

[dnj, 2016/10/04 16:42:35]

+176 on docs here. This is probably the most magic code in recipe engine :)

[iannucci, 2016/10/06 22:47:11]

Done.

+    if exp:
+      # this check passed
+      return
+
+    stk = None

[dnj, 2016/10/04 16:42:35]

nit: this shouldn't be needed, since all paths cause it to be defined.

[iannucci, 2016/10/06 22:47:11]

Done.

+    try:
+      raise ZeroDivisionError
+    except ZeroDivisionError:
+      stk = sys.exc_info()[2].tb_frame.f_back.f_back
+
+    refNames, node, actual_lineno = getReferencedNamesAndNode(
+      stk.f_code.co_filename, stk.f_lineno)
+    local_vars = OrderedDict()
+    for k, v in stk.f_locals.iteritems():
+      if k not in refNames:
+        continue
+      if any(v is itm for itm in self._ignore_set):
+        continue
+      local_vars[k] = v
+    self._failed_checks.append(expect_tests.Check(
+      hint,
+      astunparse.unparse(node),
+      local_vars,
+      stk.f_code.co_filename,
+      actual_lineno,
+      False
+    ))
+
+  def __call__(self, arg1, arg2=None):
+    if arg2 is not None:
+      hint = arg1
+      exp = arg2
+    else:
+      hint = None
+      exp = arg1
+    self._inner(hint, exp)
+
+
+def checkIsSubset(a, b):

[martiniss, 2016/10/03 19:14:19]

tests?

[dnj, 2016/10/04 16:42:34]

docs?

[iannucci, 2016/10/06 22:47:11]

Done.

+  # both a and b should be ordered step lists.
+  if a is b:

[dnj, 2016/10/04 16:42:35]

Could use "==" here to catch duplicates that aren't the exact same object.

[iannucci, 2016/10/06 22:47:11]

I'm not certain how that would ever happen in practice? This is essentially
implemented in the loop below anyway.

+    return
+
+  for step_name, step in a.iteritems():
+    if step_name not in b:
+      raise ValueError(

[dnj, 2016/10/04 16:42:34]

Maybe define a PostProcessError type?

[iannucci, 2016/10/06 22:47:10]

Done.

+        'post_process introduced new step %r' % step_name)
+
+    if 'name' not in step:
+      raise ValueError(
+        'post_process removed "name" from step %r' % step_name)

[dnj, 2016/10/04 16:42:35]

Should document that this is not allowed in "post_process".

[iannucci, 2016/10/06 22:47:10]

Done.

+
+    orig = b[step_name]
+    if step is orig:

[dnj, 2016/10/04 16:42:34]

== ?

+      continue
+
+    for k, v in step.iteritems():
+      if k not in orig:
+        raise ValueError(
+          'post_process introduced new key %r in step %r' % (
+            k, step_name))
+
+      orig_v = orig[k]
+      if v is orig_v:

[dnj, 2016/10/04 16:42:34]

== ?

[iannucci, 2016/10/06 22:47:11]

Again, not sure how this would happen in practice, and this is implemented
below. The purpose of using 'is' is to avoid looping over the whole data
structure as an optimization. Using == will end up looping over every data
member twice if they're not equal.

+        continue
+
+      if type(v) != type(orig_v):
+        raise ValueError(
+          'post_process changed type of key %r in step %r from %s to %s' % (
+            k, step_name, type(orig_v), type(v)))

[dnj, 2016/10/04 16:42:35]

For rendering type names, "type(x).__name__" is a more user-friendly.

[iannucci, 2016/10/06 22:47:11]

Done.

+
+      if isinstance(v, list):

[dnj, 2016/10/04 16:42:35]

Unless we specifically care about "list" and "dict", consider using
collections.Sequence and collections.Mapping respectively.

[iannucci, 2016/10/06 22:47:10]

We do, expectations will never have something other than primitive
json-encodable types.

+        if len(v) > len(orig_v):
+          raise ValueError(
+            'post_process %r in step %r is longer than the original: %d v %d' %
+            (k, step_name, len(v), len(orig_v)))
+        iov = iv = 0

[martiniss, 2016/10/03 19:14:19]

not immediately obvious what these variables are for.

[iannucci, 2016/10/06 22:47:11]

Done.

+        while iov < len(orig_v) - 1 and iv < len(v) - 1:
+          if v[iv] == orig_v[iov]:
+            iv += 1
+          iov += 1
+        if iv != len(v) - 1:
+          raise ValueError(
+            'post_process added elements to %r in step %r: %r' % (
+              k, step_name, v[iv:]))
+      elif isinstance(v, dict):
+        for subk, subv in v.iteritems():
+          if subk not in orig_v:

[dnj, 2016/10/04 16:42:35]

Can get by with only one lookup:

orig_item = orig_v.get(subk) # Use a sentinel object instead of None if None is
acceptable.
if subk != orig_item:
  if orig_item is None:
    raise "added element"
  else:
    raise "changed element"

[iannucci, 2016/10/06 22:47:10]

Done.

+            raise ValueError(
+              'post_process added element %s[%r] in step %r' % (
+                k, subk, step_name))
+          if subv != orig_v[subk]:
+            raise ValueError(
+              'post_process changed element %s[%r] in step %r: %r to %r' % (
+                k, subk, step_name, subv, orig_v[subk]))
+      elif v != orig_v:
+        raise ValueError(
+          'post_process changed value of key %r in step %r from %s to %s' % (
+            k, step_name, orig_v, v))
+
+
+def RenderExpectation(test_data, step_odict):
+  """Applies the step post_process actions to the step_odict, if the
+  TestData actually contains any.
 
   Returns the final expect_tests.Result."""
-  if test_data.whitelist_data:
-    whitelist_data  = dict(test_data.whitelist_data)  # copy so we can mutate it
-    def filter_expectation(step):
-      whitelist = whitelist_data.pop(step['name'], None)
-      if whitelist is None:
-        return
+  checker = Checker()
 
-      whitelist = set(whitelist)  # copy so we can mutate it
-      if len(whitelist) > 0:
-        whitelist.add('name')
-        step = {k: v for k, v in step.iteritems() if k in whitelist}
-        whitelist.difference_update(step.keys())
-        if whitelist:
-          raise ValueError(
-            "The whitelist includes fields %r in step %r, but those fields"
-            " don't exist."
-            % (whitelist, step['name']))
-      return step
-    raw_expectations = filter(filter_expectation, raw_expectations)
+  for hook, args, kwargs in test_data.post_process_hooks:
+    input_odict = copy.deepcopy(step_odict)
+    checker._ignore(input_odict)

[martiniss, 2016/10/03 19:14:19]

why do we always ignore the input dict?

[iannucci, 2016/10/06 22:47:10]

Done.

+    rslt = hook(checker, input_odict, *args, **kwargs)
+    if rslt is not None:
+      checkIsSubset(rslt, step_odict)
+      step_odict = rslt
 
-    if whitelist_data:
-      raise ValueError(
-        "The step names %r were included in the whitelist, but were never run."
-        % [s['name'] for s in whitelist_data])
-
-  return expect_tests.Result(raw_expectations)
+  return expect_tests.Result(step_odict.values(), checker._failed_checks)
 
 
 class SimulationAnnotatorStreamEngine(stream.AnnotatorStreamEngine):
 
   def __init__(self):
     self._step_buffer_map = {}
     super(SimulationAnnotatorStreamEngine, self).__init__(
         self.step_buffer(None))
 
   def step_buffer(self, step_name):
     return self._step_buffer_map.setdefault(step_name, StringIO.StringIO())
 
   def new_step_stream(self, step_config):
     return self._create_step_stream(step_config,
                                     self.step_buffer(step_config.name))
 
 
-def RunRecipe(test_data):
+# This maps from (recipe_name,test_name) -> yielded test_data. It's outside of

[dnj, 2016/10/04 16:42:35]

Can you make this a namedtuple?

[iannucci, 2016/10/06 22:47:11]

I tried, it makes it more verbose for very little benefit. 2 element tuples used
exclusively as dictionary keys are perfectly fine imo.

+# RunRecipe so that it can persist between RunRecipe calls in the same process.
+_GEN_TEST_CACHE = {}
+
+def RunRecipe(recipe_name, test_name):
   """Actually runs the recipe given the GenTests-supplied test_data."""
   from . import config_types
   from . import loader
   from . import run
   from . import step_runner
-  from . import stream
+
+  if recipe_name not in _GEN_TEST_CACHE:
+    recipe_script = _UNIVERSE.load_recipe(recipe_name)
+    test_api = loader.create_test_api(recipe_script.LOADED_DEPS, _UNIVERSE)
+    for test_data in recipe_script.GenTests(test_api):
+      _GEN_TEST_CACHE[(recipe_name, test_data.name)] = test_data
+
+  test_data = _GEN_TEST_CACHE[(recipe_name, test_name)]
 
   config_types.ResetTostringFns()
 
   annotator = SimulationAnnotatorStreamEngine()
   stream_engine = stream.ProductStreamEngine(
       stream.StreamEngineInvariants(),
       annotator)
   with stream_engine:
     step_runner = step_runner.SimulationStepRunner(stream_engine, test_data,
                                                    annotator)
 
-    engine = run.RecipeEngine(step_runner, test_data.properties, _UNIVERSE)
-    recipe_script = _UNIVERSE.load_recipe(test_data.properties['recipe'])
+    props = test_data.properties.copy()

[dnj, 2016/10/04 16:42:34]

deepcopy? I think properties can be nested.

[iannucci, 2016/10/06 22:47:11]

they can, but it doesn't matter.

+    props['recipe'] = recipe_name
+    engine = run.RecipeEngine(step_runner, props, _UNIVERSE)
+    recipe_script = _UNIVERSE.load_recipe(recipe_name)
     api = loader.create_recipe_api(recipe_script.LOADED_DEPS, engine, test_data)
     result = engine.run(recipe_script, api)
 
     # Don't include tracebacks in expectations because they are too sensitive to
     # change.
     result.result.pop('traceback', None)
-    raw_expectations = step_runner.steps_ran + [result.result]
+    raw_expectations = step_runner.steps_ran.copy()
+    raw_expectations[result.result['name']] = result.result
 
     try:
       return RenderExpectation(test_data, raw_expectations)
     except:
       print
       print "The expectations would have been:"
       json.dump(raw_expectations, sys.stdout, indent=2)
       raise
 
 
@@ skipping 13 matching lines @@
 def cover_omit():
   omit = [ ]
 
   for mod_dir_base in _UNIVERSE.module_dirs:
     if os.path.isdir(mod_dir_base):
         omit.append(os.path.join(mod_dir_base, '*', 'resources', '*'))
 
   return omit
 
 
-class InsufficientTestCoverage(Exception): pass
+class InsufficientTestCoverage(Exception):
+  pass
 
 
 @expect_tests.covers(test_gen_coverage)
 def GenerateTests():
   from . import loader
 
   cover_mods = [ ]
   for mod_dir_base in _UNIVERSE.module_dirs:
     if os.path.isdir(mod_dir_base):
       cover_mods.append(os.path.join(mod_dir_base, '*.py'))
 
   for recipe_path, recipe_name in _UNIVERSE.loop_over_recipes():
     try:
       recipe = _UNIVERSE.load_recipe(recipe_name)
       test_api = loader.create_test_api(recipe.LOADED_DEPS, _UNIVERSE)
 
       covers = cover_mods + [recipe_path]
 
-      full_expectation_count = 0
       for test_data in recipe.GenTests(test_api):
-        if not test_data.whitelist_data:
-          full_expectation_count += 1
         root, name = os.path.split(recipe_path)
         name = os.path.splitext(name)[0]
         expect_path = os.path.join(root, '%s.expected' % name)
-
-        test_data.properties['recipe'] = recipe_name.replace('\\', '/')
         yield expect_tests.Test(
             '%s.%s' % (recipe_name, test_data.name),
-            expect_tests.FuncCall(RunRecipe, test_data),
+            expect_tests.FuncCall(RunRecipe, recipe_name, test_data.name),
             expect_dir=expect_path,
             expect_base=test_data.name,
             covers=covers,
             break_funcs=(recipe.RunSteps,)
         )
-
-      if full_expectation_count < 1:
-        raise InsufficientTestCoverage(
-          'Must have at least 1 test without a whitelist!')
     except:
       info = sys.exc_info()
       new_exec = Exception('While generating results for %r: %s: %s' % (
         recipe_name, info[0].__name__, str(info[1])))
       raise new_exec.__class__, new_exec, info[2]
 
 
 def main(universe, args=None):
   """Runs simulation tests on a given repo of recipes.
 
@@ skipping 10 matching lines @@
                   'TESTING_SLAVENAME']:
     if env_var in os.environ:
       logging.warn("Ignoring %s environment variable." % env_var)
       os.environ.pop(env_var)
 
   global _UNIVERSE
   _UNIVERSE = universe
 
   expect_tests.main('recipe_simulation_test', GenerateTests,
                     cover_omit=cover_omit(), args=args)