]> git.ipfire.org Git - thirdparty/sqlalchemy/sqlalchemy.git/commitdiff
Rework combination exclusions
authorMike Bayer <mike_mp@zzzcomputing.com>
Mon, 10 Feb 2020 20:38:39 +0000 (15:38 -0500)
committerMike Bayer <mike_mp@zzzcomputing.com>
Mon, 10 Feb 2020 20:48:31 +0000 (15:48 -0500)
The technique arrived at for doing exclusions inside of combinations
relies upon comparing all the arguments in a particular combination
to some set of combinations that were gathered as having
"exclusions".   This logic is actually broken for the
case where the @testing.combinations has an "id", but if we fix
that, we still have the issue of all the arguments being
compared, which is complicated and also doesn't work for the
case of a py2/py3 incompatibility like a timezone that has
fractional minutes or seconds in it.   It's also not clear
if a @testing.combinations that uses lambdas will work either
(maybe it does though because lambdax == lambdax compares...).

anyway, this patch reworks it so that we hit this on the decorator
side instead, where we add our own decorator and go through
the extra effort to create a decorator that accepts an extra
argument of "exclusions" which we can then check in a way that
is local to the whole pytest @combinations thing in the first place.
The only difficulty is that pytest is very sneaky about looking
at the test function so we need to make sure __wrapped__ isn't
set when doing this.

Change-Id: Ic57aae15b378e0f4ed009e4e82ae7ba73fb6dfc5
(cherry picked from commit 12ec0e06858d84097a051a50a60fe9a1582ee95c)

lib/sqlalchemy/testing/exclusions.py
lib/sqlalchemy/testing/plugin/pytestplugin.py
test/orm/test_defaults.py

index 9ab929e06ddf241da42765cf4392224c1bbbe915..e3e3c06b7331fb7e9d587a1e509b891656178dd9 100644 (file)
@@ -35,20 +35,10 @@ class compound(object):
         self.fails = set()
         self.skips = set()
         self.tags = set()
-        self.combinations = {}
 
     def __add__(self, other):
         return self.add(other)
 
-    def with_combination(self, **kw):
-        copy = compound()
-        copy.fails.update(self.fails)
-        copy.skips.update(self.skips)
-        copy.tags.update(self.tags)
-        copy.combinations.update((f, kw) for f in copy.fails)
-        copy.combinations.update((s, kw) for s in copy.skips)
-        return copy
-
     def add(self, *others):
         copy = compound()
         copy.fails.update(self.fails)
@@ -95,7 +85,6 @@ class compound(object):
         self.skips.update(other.skips)
         self.fails.update(other.fails)
         self.tags.update(other.tags)
-        self.combinations.update(other.combinations)
 
     def __call__(self, fn):
         if hasattr(fn, "_sa_exclusion_extend"):
@@ -118,29 +107,13 @@ class compound(object):
         try:
             yield
         except Exception as ex:
-            all_fails._expect_failure(config._current, ex, None)
+            all_fails._expect_failure(config._current, ex)
         else:
-            all_fails._expect_success(config._current, None)
-
-    def _check_combinations(self, combination, predicate):
-        if predicate in self.combinations:
-            for k, v in combination:
-                if (
-                    k in self.combinations[predicate]
-                    and self.combinations[predicate][k] != v
-                ):
-                    return False
-        return True
+            all_fails._expect_success(config._current)
 
     def _do(self, cfg, fn, *args, **kw):
-        if len(args) > 1:
-            insp = inspect_getfullargspec(fn)
-            combination = list(zip(insp.args[1:], args[1:]))
-        else:
-            combination = None
-
         for skip in self.skips:
-            if self._check_combinations(combination, skip) and skip(cfg):
+            if skip(cfg):
                 msg = "'%s' : %s" % (
                     config.get_current_test_name(),
                     skip._as_string(cfg),
@@ -150,14 +123,14 @@ class compound(object):
         try:
             return_value = fn(*args, **kw)
         except Exception as ex:
-            self._expect_failure(cfg, ex, combination, name=fn.__name__)
+            self._expect_failure(cfg, ex, name=fn.__name__)
         else:
-            self._expect_success(cfg, combination, name=fn.__name__)
+            self._expect_success(cfg, name=fn.__name__)
             return return_value
 
-    def _expect_failure(self, config, ex, combination, name="block"):
+    def _expect_failure(self, config, ex, name="block"):
         for fail in self.fails:
-            if self._check_combinations(combination, fail) and fail(config):
+            if fail(config):
                 if util.py2k:
                     str_ex = unicode(ex).encode("utf-8", errors="ignore")
                 else:
@@ -172,12 +145,12 @@ class compound(object):
         else:
             util.raise_from_cause(ex)
 
-    def _expect_success(self, config, combination, name="block"):
+    def _expect_success(self, config, name="block"):
         if not self.fails:
             return
 
         for fail in self.fails:
-            if self._check_combinations(combination, fail) and fail(config):
+            if fail(config):
                 raise AssertionError(
                     "Unexpected success for '%s' (%s)"
                     % (
index 015fee22ba79254493f18e1b208bb6060f0bbf73..6e958ab5e199b3c7b1a802638dacc2c1a24e5467 100644 (file)
@@ -7,6 +7,7 @@ except ImportError:
 
 import argparse
 import collections
+from functools import update_wrapper
 import inspect
 import itertools
 import operator
@@ -16,6 +17,13 @@ import sys
 
 import pytest
 
+try:
+    import typing
+except ImportError:
+    pass
+else:
+    if typing.TYPE_CHECKING:
+        from typing import Sequence
 
 try:
     import xdist  # noqa
@@ -297,6 +305,49 @@ def getargspec(fn):
         return inspect.getargspec(fn)
 
 
+def _pytest_fn_decorator(target):
+    """Port of langhelpers.decorator with pytest-specific tricks."""
+
+    from sqlalchemy.util.langhelpers import format_argspec_plus
+    from sqlalchemy.util.compat import inspect_getfullargspec
+
+    def _exec_code_in_env(code, env, fn_name):
+        exec(code, env)
+        return env[fn_name]
+
+    def decorate(fn, add_positional_parameters=()):
+
+        spec = inspect_getfullargspec(fn)
+        if add_positional_parameters:
+            spec.args.extend(add_positional_parameters)
+
+        metadata = dict(target="target", fn="fn", name=fn.__name__)
+        metadata.update(format_argspec_plus(spec, grouped=False))
+        code = (
+            """\
+def %(name)s(%(args)s):
+    return %(target)s(%(fn)s, %(apply_kw)s)
+"""
+            % metadata
+        )
+        decorated = _exec_code_in_env(
+            code, {"target": target, "fn": fn}, fn.__name__
+        )
+        if not add_positional_parameters:
+            decorated.__defaults__ = getattr(fn, "im_func", fn).__defaults__
+            decorated.__wrapped__ = fn
+            return update_wrapper(decorated, fn)
+        else:
+            # this is the pytest hacky part.  don't do a full update wrapper
+            # because pytest is really being sneaky about finding the args
+            # for the wrapped function
+            decorated.__module__ = fn.__module__
+            decorated.__name__ = fn.__name__
+            return decorated
+
+    return decorate
+
+
 class PytestFixtureFunctions(plugin_base.FixtureFunctions):
     def skip_test_exception(self, *arg, **kw):
         return pytest.skip.Exception(*arg, **kw)
@@ -328,8 +379,6 @@ class PytestFixtureFunctions(plugin_base.FixtureFunctions):
 
         argnames = kw.pop("argnames", None)
 
-        exclusion_combinations = []
-
         def _filter_exclusions(args):
             result = []
             gathered_exclusions = []
@@ -339,13 +388,12 @@ class PytestFixtureFunctions(plugin_base.FixtureFunctions):
                 else:
                     result.append(a)
 
-            exclusion_combinations.extend(
-                [(exclusion, result) for exclusion in gathered_exclusions]
-            )
-            return result
+            return result, gathered_exclusions
 
         id_ = kw.pop("id_", None)
 
+        tobuild_pytest_params = []
+        has_exclusions = False
         if id_:
             _combination_id_fns = self._combination_id_fns
 
@@ -366,53 +414,87 @@ class PytestFixtureFunctions(plugin_base.FixtureFunctions):
                 if char in _combination_id_fns
             ]
 
-            arg_sets = [
-                pytest.param(
-                    *_arg_getter(_filter_exclusions(arg))[1:],
-                    id="-".join(
-                        comb_fn(getter(arg)) for getter, comb_fn in fns
+            for arg in arg_sets:
+                if not isinstance(arg, tuple):
+                    arg = (arg,)
+
+                fn_params, param_exclusions = _filter_exclusions(arg)
+
+                parameters = _arg_getter(fn_params)[1:]
+
+                if param_exclusions:
+                    has_exclusions = True
+
+                tobuild_pytest_params.append(
+                    (
+                        parameters,
+                        param_exclusions,
+                        "-".join(
+                            comb_fn(getter(arg)) for getter, comb_fn in fns
+                        ),
                     )
                 )
-                for arg in [
-                    (arg,) if not isinstance(arg, tuple) else arg
-                    for arg in arg_sets
-                ]
-            ]
+
         else:
-            # ensure using pytest.param so that even a 1-arg paramset
-            # still needs to be a tuple.  otherwise paramtrize tries to
-            # interpret a single arg differently than tuple arg
-            arg_sets = [
-                pytest.param(*_filter_exclusions(arg))
-                for arg in [
-                    (arg,) if not isinstance(arg, tuple) else arg
-                    for arg in arg_sets
-                ]
-            ]
+
+            for arg in arg_sets:
+                if not isinstance(arg, tuple):
+                    arg = (arg,)
+
+                fn_params, param_exclusions = _filter_exclusions(arg)
+
+                if param_exclusions:
+                    has_exclusions = True
+
+                tobuild_pytest_params.append(
+                    (fn_params, param_exclusions, None)
+                )
+
+        pytest_params = []
+        for parameters, param_exclusions, id_ in tobuild_pytest_params:
+            if has_exclusions:
+                parameters += (param_exclusions,)
+
+            param = pytest.param(*parameters, id=id_)
+            pytest_params.append(param)
 
         def decorate(fn):
             if inspect.isclass(fn):
+                if has_exclusions:
+                    raise NotImplementedError(
+                        "exclusions not supported for class level combinations"
+                    )
                 if "_sa_parametrize" not in fn.__dict__:
                     fn._sa_parametrize = []
-                fn._sa_parametrize.append((argnames, arg_sets))
+                fn._sa_parametrize.append((argnames, pytest_params))
                 return fn
             else:
                 if argnames is None:
-                    _argnames = getargspec(fn).args[1:]
+                    _argnames = getargspec(fn).args[1:]  # type: Sequence(str)
                 else:
-                    _argnames = argnames
-
-                if exclusion_combinations:
-                    for exclusion, combination in exclusion_combinations:
-                        combination_by_kw = {
-                            argname: val
-                            for argname, val in zip(_argnames, combination)
-                        }
-                        exclusion = exclusion.with_combination(
-                            **combination_by_kw
-                        )
-                        fn = exclusion(fn)
-                return pytest.mark.parametrize(_argnames, arg_sets)(fn)
+                    _argnames = re.split(
+                        r", *", argnames
+                    )  # type: Sequence(str)
+
+                if has_exclusions:
+                    _argnames += ["_exclusions"]
+
+                    @_pytest_fn_decorator
+                    def check_exclusions(fn, *args, **kw):
+                        _exclusions = args[-1]
+                        if _exclusions:
+                            exlu = exclusions.compound().add(*_exclusions)
+                            fn = exlu(fn)
+                        return fn(*args[0:-1], **kw)
+
+                    def process_metadata(spec):
+                        spec.args.append("_exclusions")
+
+                    fn = check_exclusions(
+                        fn, add_positional_parameters=("_exclusions",)
+                    )
+
+                return pytest.mark.parametrize(_argnames, pytest_params)(fn)
 
         return decorate
 
index 07b82b9dab9d1e73f26a1f50a45d35c0e0142b7f..5cadea5ffc7e0075c067e5be5b152a57c99e5adb 100644 (file)
@@ -263,7 +263,13 @@ class ComputedDefaultsOnUpdateTest(fixtures.MappedTest):
             )
 
     @testing.combinations(
-        (True, testing.requires.computed_columns_on_update_returning), (False,)
+        (
+            "eagerload",
+            True,
+            testing.requires.computed_columns_on_update_returning,
+        ),
+        ("noneagerload", False,),
+        id_="ia",
     )
     def test_update_computed(self, eager):
         if eager: