Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(179)

Unified Diff: recipe_engine/simulation_test.py

Issue 2387763003: Add initial postprocess unit test thingy. (Closed)
Patch Set: more comments Created 4 years, 2 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: recipe_engine/simulation_test.py
diff --git a/recipe_engine/simulation_test.py b/recipe_engine/simulation_test.py
index 17c1d1e511689cdb38340f1af3a4585b640e46df..01b8e55328d1b8d4731797e79f9dd24b8c88d722 100644
--- a/recipe_engine/simulation_test.py
+++ b/recipe_engine/simulation_test.py
@@ -5,15 +5,23 @@
"""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
+import inspect
+
+from collections import OrderedDict, namedtuple
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.
@@ -21,38 +29,350 @@ import expect_tests
# doesn't know how to serialize it.
_UNIVERSE = None
+class _checkTransformer(ast.NodeTransformer):
+ """_checkTransformer is an ast NodeTransformer which extracts the helpful
+ subexpressions from a python expression (specificially, from an invocation of
Michael Achenbach 2016/10/07 16:32:04 nit: s/specificially/specifically
+ the Checker). These subexpressions will be printed along with the check's
+ source code statement to provide context for the failed check.
+
+ It knows the following transformations:
+ * all python identifiers will be resolved to their local variable meaning.
+ * `___ in <instance of dict>` will cause dict.keys() to be printed in lieu
+ of the entire dictionary.
+ * `a[b][c]` will cause `a[b]` and `a[b][c]` to be printed (for an arbitrary
+ level of recursion)
+
+ The transformed ast is NOT a valid python AST... In particular, every reduced
+ subexpression will be an ast.Name() where the id is the code for the
+ subexpression (which may not be a valid name! It could be `foo.bar()`.), and
+ the ctx will be the eval'd value for that element.
+
+ In addition to this, there will be a list of ast.Name nodes in the
+ transformer's `extra` attribute for additional expressions which should be
+ printed for debugging usefulness, but didn't fit into the ast tree anywhere.
+ """
+
+ def __init__(self, lvars, gvars):
+ self.lvars = lvars
+ self.gvars = gvars
+ self.extras = []
+
+ def _eval(self, node):
+ code = astunparse.unparse(node).strip()
+ try:
+ thing = eval(code, self.gvars, self.lvars)
+ return ast.Name(code, thing)
+ except NameError:
+ return node
+
+ def visit_Compare(self, node):
+ # match `___ in instanceof(dict)`
+ node = self.generic_visit(node)
+
+ if len(node.ops) == 1 and isinstance(node.ops[0], ast.In):
+ cmps = node.comparators
+ if len(cmps) == 1 and isinstance(cmps[0], ast.Name):
+ name = cmps[0]
+ if isinstance(name.ctx, dict):
+ node = ast.Compare(
+ node.left,
+ node.ops,
+ [ast.Name(name.id+".keys()", name.ctx.keys())])
+
+ return node
+
+ def visit_Subscript(self, node):
+ # match __[a]
+ node = self.generic_visit(node)
+ if isinstance(node.slice, ast.Index):
+ if isinstance(node.slice.value, ast.Name):
+ self.extras.append(self._eval(node.slice.value))
+ node = self._eval(node)
+ return node
+
+ def visit_Name(self, node):
+ # match foo
+ return self._eval(node)
+
+
+def render_user_value(val):
+ """Takes a subexpression user value, and attempts to render it in the most
+ useful way possible.
+
+ Currently this will use render_re for compiled regular expressions, and will
+ fall back to repr() for everything else.
+
+ It should be the goal of this function to return an `eval`able string that
+ would yield the equivalent value in a python interpreter.
+ """
+ if isinstance(val, re._pattern_type):
+ return render_re(val)
+ return repr(val)
+
+
+def render_re(regex):
+ """Renders a repr()-style value for a compiled regular expression."""
+ actual_flags = []
+ if regex.flags:
+ flags = [
+ (re.IGNORECASE, 'IGNORECASE'),
+ (re.LOCALE, 'LOCALE'),
+ (re.UNICODE, 'UNICODE'),
+ (re.MULTILINE, 'MULTILINE'),
+ (re.DOTALL, 'DOTALL'),
+ (re.VERBOSE, 'VERBOSE'),
+ ]
+ for val, name in flags:
+ if regex.flags & val:
+ actual_flags.append(name)
+ if actual_flags:
+ return 're.compile(%r, %s)' % (regex.pattern, '|'.join(actual_flags))
+ else:
+ return 're.compile(%r)' % regex.pattern
+
+
+class _checker(object):
+ def __init__(self, filename, lineno, funcname, args, kwargs, *ignores):
+ self._failed_checks = []
+
+ # _ignore_set is the set of objects that we should never print as local
+ # variables. We start this set off by including the actual _checker object,
+ # since there's no value to printing that.
+ self._ignore_set = {id(x) for x in ignores+(self,)}
+
+ self._ctx_filename = filename
+ self._ctx_lineno = lineno
+ self._ctx_funcname = funcname
+ self._ctx_args = map(repr, args)
+ self._ctx_kwargs = {k: repr(v) for k, v in kwargs.iteritems()}
+
+ def _process_frame(self, frame, with_vars):
+ """This processes a stack frame into an expect_tests.CheckFrame, which
+ includes file name, line number, function name (of the function containing
+ the frame), the parsed statement at that line, and the relevant local
+ variables/subexpressions (if with_vars is True).
+
+ In addition to transforming the expression with _checkTransformer, this
+ will:
+ * omit subexpressions which resolve to callable()'s
+ * omit the overall step ordered dictionary
+ * transform all subexpression values using render_user_value().
+ """
+ raw_frame, filename, lineno, func_name, _, _ = frame
+
+ filelines, _ = inspect.findsource(raw_frame)
+
+ i = lineno-1
+ # this dumb little loop will try to parse a node out of the ast which ends
+ # at the line that shows up in the frame. To do this, tries parsing that
+ # line, and if it fails, it adds a prefix line. It keeps doing this until
+ # it gets a successful parse.
+ for i in xrange(lineno-1, 0, -1):
+ try:
+ to_parse = ''.join(filelines[i:lineno]).strip()
+ node = ast.parse(to_parse)
+ break
+ except SyntaxError:
+ continue
+ varmap = None
+ if with_vars:
+ xfrmr = _checkTransformer(raw_frame.f_locals, raw_frame.f_globals)
+
+ varmap = {}
+ def add_node(n):
+ if isinstance(n, ast.Name):
+ val = n.ctx
+ if isinstance(val, ast.AST):
+ return
+ if callable(val) or id(val) in self._ignore_set:
+ return
+ if n.id not in varmap:
+ varmap[n.id] = render_user_value(val)
+ map(add_node, ast.walk(xfrmr.visit(copy.deepcopy(node))))
+ # TODO(iannucci): only add extras if verbose is True
+ map(add_node, xfrmr.extras)
+
+ return expect_tests.CheckFrame(
+ filename,
+ lineno,
+ func_name,
+ astunparse.unparse(node).strip(),
+ varmap
+ )
+
+ def _call_impl(self, hint, exp):
+ """This implements the bulk of what happens when you run `check(exp)`. It
+ will crawl back up the stack and extract information about all of the frames
+ which are relevent to the check, including file:lineno and the code
+ statement which occurs at that location for all the frames.
+
+ On the last frame (the one that actually contains the check call), it will
+ also try to obtain relevant local values in the check so they can be printed
+ with the check to aid in debugging and diagnosis. It uses the parsed
+ statement found at that line to find all referenced local variables in that
+ frame.
+ """
+
+ if exp:
+ # TODO(iannucci): collect this in verbose mode.
+ # this check passed
+ return
-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.
+ try:
+ frames = inspect.stack()[2:]
+
+ # grab all frames which have self as a local variable (e.g. frames
+ # associated with this checker), excluding self.__call__.
+ try:
+ i = 0
+ for i, f in enumerate(frames):
+ if self not in f[0].f_locals.itervalues():
+ break
+ keep_frames = [self._process_frame(f, j == 0)
+ for j, f in enumerate(frames[:i-1])]
+ finally:
+ del f
+
+ # order it so that innermost frame is at the bottom
+ keep_frames = keep_frames[::-1]
+
+ self._failed_checks.append(expect_tests.Check(
+ hint,
+ self._ctx_filename,
+ self._ctx_lineno,
+ self._ctx_funcname,
+ self._ctx_args,
+ self._ctx_kwargs,
+ keep_frames,
+ False
+ ))
+ finally:
+ # avoid reference cycle as suggested by inspect docs.
+ del frames
+
+ def __call__(self, arg1, arg2=None):
+ if arg2 is not None:
+ hint = arg1
+ exp = arg2
+ else:
+ hint = None
+ exp = arg1
+ self._call_impl(hint, exp)
+
+
+class PostProcessError(ValueError):
+ def __init__(self, msg):
+ super(PostProcessError, self).__init__("post_process %s" % (msg))
+
+
+class PostProcessStepError(ValueError):
+ def __init__(self, msg, step_name):
+ super(PostProcessStepError, self).__init__(
+ msg + " in step " + repr(step_name))
+
+
+def _checkIsSubset(a, b):
+ """This checks to see that a is a subset of b, where both a and b are
+ OrderedDicts. This will raise a PostProcessError if any data in a is not
+ present in b."""
+
+ # both a and b should be ordered step lists.
+ if a is b:
+ return
+
+ typeOK = (
+ len(b) == 0 and isinstance(b, (OrderedDict, dict)) or
+ len(b) != 0 and isinstance(b, OrderedDict))
+ if not typeOK:
+ raise PostProcessError(
+ 'must always return an OrderedDict() or the empty {}.')
+
+ for step_name, step in a.iteritems():
+ if step_name not in b:
+ raise PostProcessError('introduced new step %r' % step_name)
+
+ if 'name' not in step:
+ raise PostProcessStepError('removed "name"', step_name)
+
+ orig = b[step_name]
+ if step is orig:
+ continue
+
+ for k, v in step.iteritems():
+ if k not in orig:
+ raise PostProcessStepError('introduced new key %r' % k, step_name)
+
+ orig_v = orig[k]
+ if v is orig_v:
+ continue
+
+ if type(v) != type(orig_v):
+ raise PostProcessStepError(
+ 'changed type of key %r from %s to %s' % (
+ k, type(orig_v).__name__, type(v).__name__), step_name)
+
+ if isinstance(v, list):
+ if len(v) > len(orig_v):
+ raise PostProcessStepError(
+ '%r is longer than the original (%d v %d)' %
+ (k, len(v), len(orig_v)), step_name)
+ idx_orig_v = idx_v = 0
+ while idx_orig_v < len(orig_v) - 1 and idx_v < len(v) - 1:
+ if v[idx_v] == orig_v[idx_orig_v]:
+ idx_v += 1
+ idx_orig_v += 1
+ if idx_v != len(v) - 1:
+ raise PostProcessStepError(
+ 'added elements to %r (%r)' % (k, v[idx_v:]), step_name)
+ elif isinstance(v, dict):
+ for subk, subv in v.iteritems():
+ el = orig_v.get(subk)
+ if el is None:
+ raise PostProcessStepError('added element %s[%r]' % (k, subk),
+ step_name)
+ if subv != el:
+ raise PostProcessError(
+ 'changed element %s[%r] in step %r: %r to %r' % (
+ k, subk, step_name, subv, orig_v[subk]))
+ elif v != orig_v:
+ raise PostProcessStepError(
+ 'changed value of key %r from %s to %s' % (k, orig_v, v), step_name)
+
+
+def _nameOfCallable(c):
+ if inspect.isfunction(c):
+ return c.__name__
+ if inspect.ismethod(c):
+ return c.im_class.__name__+'.'+c.__name__
+ if hasattr(c, '__class__') and hasattr(c, '__call__'):
+ return c.__class__.__name__+'.__call__'
+ return repr(c)
+
+
+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
-
- 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)
-
- 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)
+
+ failed_checks = []
+
+ for hook, args, kwargs, filename, lineno in test_data.post_process_hooks:
+ input_odict = copy.deepcopy(step_odict)
+ # we ignore the input_odict so that it never gets printed in full. Usually
+ # the check invocation itself will index the input_odict or will use it only
+ # for a key membership comparison, which provides enough debugging context.
+ checker = _checker(filename, lineno, _nameOfCallable(hook), args, kwargs,
+ input_odict)
+ rslt = hook(checker, input_odict, *args, **kwargs)
+ failed_checks += checker._failed_checks
+ if rslt is not None:
+ _checkIsSubset(rslt, step_odict)
+ step_odict = rslt
+
+ # empty means drop expectation
+ result_data = step_odict.values() if step_odict else None
+ return expect_tests.Result(result_data, failed_checks)
class SimulationAnnotatorStreamEngine(stream.AnnotatorStreamEngine):
@@ -70,13 +390,24 @@ class SimulationAnnotatorStreamEngine(stream.AnnotatorStreamEngine):
self.step_buffer(step_config.name))
-def RunRecipe(test_data):
+# This maps from (recipe_name,test_name) -> yielded test_data. It's outside of
+# 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()
@@ -88,18 +419,21 @@ def RunRecipe(test_data):
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()
+ 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)
+ return _renderExpectation(test_data, raw_expectations)
except:
print
print "The expectations would have been:"
@@ -130,7 +464,8 @@ def cover_omit():
return omit
-class InsufficientTestCoverage(Exception): pass
+class InsufficientTestCoverage(Exception):
+ pass
@expect_tests.covers(test_gen_coverage)
@@ -149,27 +484,18 @@ def GenerateTests():
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' % (

Powered by Google App Engine
This is Rietveld 408576698