Remove code that rewrites code (#309)

* Stop testing for old pytest and python versions

* Switch from pytest-pep8 to pycodestyle.

Pytest-pep8 received the last update in 2014, and it is now not working with pytest >= 4.5

* Explicitly state used markers

* Remove supposedly useless test

* Fix pytest missing markers definitions

* Fix pytest missing markers definitions

* Fix wrong command line usage

* Remove compatibility with ancient pytest

* Be more lenient when checking for failed test string

* Remove dead code

Author: youtux

pytest_bdd/plugin.py

@@ -13,12 +13,7 @@
 def pytest_addhooks(pluginmanager):
     """Register plugin hooks."""
     from pytest_bdd import hooks
-    try:
-        # pytest >= 2.8
-        pluginmanager.add_hookspecs(hooks)
-    except AttributeError:
-        # pytest < 2.8
-        pluginmanager.addhooks(hooks)
+    pluginmanager.add_hookspecs(hooks)
 
 
 @given('trace')
@@ -92,3 +87,20 @@ def pytest_cmdline_main(config):
 def pytest_bdd_apply_tag(tag, function):
     mark = getattr(pytest.mark, tag)
     return mark(function)
+
+
+@pytest.mark.tryfirst
+def pytest_collection_modifyitems(session, config, items):
+    """Re-order items using the creation counter as fallback.
+
+    Pytest has troubles to correctly order the test items for python < 3.6.
+    For this reason, we have to apply some better ordering for pytest_bdd scenario-decorated test functions.
+
+    This is not needed for python 3.6+, but this logic is safe to apply in that case as well.
+    """
+    # TODO: Try to only re-sort the items that have __pytest_bdd_counter__, and not the others,
+    #  since there may be other hooks that are executed before this and that want to reorder item as well
+    def item_key(item):
+        pytest_bdd_counter = getattr(item.function, '__pytest_bdd_counter__', 0)
+        return (item.reportinfo()[:2], pytest_bdd_counter)
+    items.sort(key=item_key)

pytest_bdd/scenario.py

@@ -20,39 +20,31 @@
     from _pytest import fixtures as pytest_fixtures
 except ImportError:
     from _pytest import python as pytest_fixtures
-import six
 
 from . import exceptions
 from .feature import (
     Feature,
-    force_encode,
     force_unicode,
     get_features,
 )
 from .steps import (
-    execute,
-    get_caller_function,
     get_caller_module,
     get_step_fixture_name,
     inject_fixture,
-    recreate_function,
 )
 from .types import GIVEN
 from .utils import CONFIG_STACK, get_args
 
-if six.PY3:  # pragma: no cover
-    import runpy
-
-    def execfile(filename, init_globals):
-        """Execute given file as a python script in given globals environment."""
-        result = runpy.run_path(filename, init_globals=init_globals)
-        init_globals.update(result)
-
 
 PYTHON_REPLACE_REGEX = re.compile(r"\W")
 ALPHA_REGEX = re.compile(r"^\d+_*")
 
 
+# We have to keep track of the invocation of @scenario() so that we can reorder test item accordingly.
+# In python 3.6+ this is no longer necessary, as the order is automatically retained.
+_py2_scenario_creation_counter = 0
+
+
 def find_argumented_step_fixture_name(name, type_, fixturemanager, request=None):
     """Find argumented step fixture name."""
     # happens to be that _arg2fixturedefs is changed during the iteration so we use a copy
@@ -88,9 +80,11 @@ def _find_step_function(request, step, scenario, encoding):
     """
     name = step.name
     try:
+        # Simple case where no parser is used for the step
         return request.getfixturevalue(get_step_fixture_name(name, step.type, encoding))
     except pytest_fixtures.FixtureLookupError:
         try:
+            # Could not find a fixture with the same name, let's see if there is a parser involved
             name = find_argumented_step_fixture_name(name, step.type, request._fixturemanager, request)
             if name:
                 return request.getfixturevalue(name)
@@ -204,63 +198,53 @@ def _execute_scenario(feature, scenario, request, encoding):
 FakeRequest = collections.namedtuple("FakeRequest", ["module"])
 
 
-def _get_scenario_decorator(feature, feature_name, scenario, scenario_name, caller_module, caller_function, encoding):
-    """Get scenario decorator."""
-    g = locals()
-    g["_execute_scenario"] = _execute_scenario
+def _get_scenario_decorator(feature, feature_name, scenario, scenario_name, encoding):
+    global _py2_scenario_creation_counter
 
-    scenario_name = force_encode(scenario_name, encoding)
+    counter = _py2_scenario_creation_counter
+    _py2_scenario_creation_counter += 1
 
-    def decorator(_pytestbdd_function):
-        if isinstance(_pytestbdd_function, pytest_fixtures.FixtureRequest):
+    # HACK: Ideally we would use `def decorator(fn)`, but we want to return a custom exception
+    # when the decorator is misused.
+    # Pytest inspect the signature to determine the required fixtures, and in that case it would look
+    # for a fixture called "fn" that doesn't exist (if it exists then it's even worse).
+    # It will error with a "fixture 'fn' not found" message instead.
+    # We can avoid this hack by using a pytest hook and check for misuse instead.
+    def decorator(*args):
+        if not args:
             raise exceptions.ScenarioIsDecoratorOnly(
                 "scenario function can only be used as a decorator. Refer to the documentation.",
             )
-
-        g.update(locals())
-
-        args = get_args(_pytestbdd_function)
+        [fn] = args
+        args = get_args(fn)
         function_args = list(args)
         for arg in scenario.get_example_params():
             if arg not in function_args:
                 function_args.append(arg)
-        if "request" not in function_args:
-            function_args.append("request")
 
-        code = """def {name}({function_args}):
+        @pytest.mark.usefixtures(*function_args)
+        def scenario_wrapper(request):
             _execute_scenario(feature, scenario, request, encoding)
-            _pytestbdd_function({args})""".format(
-            name=_pytestbdd_function.__name__,
-            function_args=", ".join(function_args),
-            args=", ".join(args))
-
-        execute(code, g)
-
-        _scenario = recreate_function(
-            g[_pytestbdd_function.__name__],
-            module=caller_module,
-            firstlineno=caller_function.f_lineno,
-        )
+            return fn(*[request.getfixturevalue(arg) for arg in args])
 
         for param_set in scenario.get_params():
             if param_set:
-                _scenario = pytest.mark.parametrize(*param_set)(_scenario)
-
+                scenario_wrapper = pytest.mark.parametrize(*param_set)(scenario_wrapper)
         for tag in scenario.tags.union(feature.tags):
             config = CONFIG_STACK[-1]
-            config.hook.pytest_bdd_apply_tag(tag=tag, function=_scenario)
+            config.hook.pytest_bdd_apply_tag(tag=tag, function=scenario_wrapper)
 
-        _scenario.__doc__ = "{feature_name}: {scenario_name}".format(
+        scenario_wrapper.__doc__ = u"{feature_name}: {scenario_name}".format(
             feature_name=feature_name, scenario_name=scenario_name)
-        _scenario.__scenario__ = scenario
-        scenario.test_function = _scenario
-        return _scenario
-
-    return recreate_function(decorator, module=caller_module, firstlineno=caller_function.f_lineno)
+        scenario_wrapper.__scenario__ = scenario
+        scenario_wrapper.__pytest_bdd_counter__ = counter
+        scenario.test_function = scenario_wrapper
+        return scenario_wrapper
+    return decorator
 
 
 def scenario(feature_name, scenario_name, encoding="utf-8", example_converters=None,
-             caller_module=None, caller_function=None, features_base_dir=None, strict_gherkin=None):
+             caller_module=None, features_base_dir=None, strict_gherkin=None):
     """Scenario decorator.
 
     :param str feature_name: Feature file name. Absolute or relative to the configured feature base path.
@@ -269,9 +253,9 @@ def scenario(feature_name, scenario_name, encoding="utf-8", example_converters=N
     :param dict example_converters: optional `dict` of example converter function, where key is the name of the
         example parameter, and value is the converter function.
     """
+
     scenario_name = force_unicode(scenario_name, encoding)
     caller_module = caller_module or get_caller_module()
-    caller_function = caller_function or get_caller_function()
 
     # Get the feature
     if features_base_dir is None:
@@ -280,7 +264,7 @@ def scenario(feature_name, scenario_name, encoding="utf-8", example_converters=N
         strict_gherkin = get_strict_gherkin()
     feature = Feature.get_feature(features_base_dir, feature_name, encoding=encoding, strict_gherkin=strict_gherkin)
 
-    # Get the sc_enario
+    # Get the scenario
     try:
         scenario = feature.scenarios[scenario_name]
     except KeyError:
@@ -298,13 +282,11 @@ def scenario(feature_name, scenario_name, encoding="utf-8", example_converters=N
     scenario.validate()
 
     return _get_scenario_decorator(
-        feature,
-        feature_name,
-        scenario,
-        scenario_name,
-        caller_module,
-        caller_function,
-        encoding,
+        feature=feature,
+        feature_name=feature_name,
+        scenario=scenario,
+        scenario_name=scenario_name,
+        encoding=encoding,
     )
 
 
@@ -375,7 +357,6 @@ def scenarios(*feature_paths, **kwargs):
         (attr.__scenario__.feature.filename, attr.__scenario__.name)
         for name, attr in module.__dict__.items() if hasattr(attr, '__scenario__'))
 
-    index = 10
     for feature in get_features(abs_feature_paths, strict_gherkin=strict_gherkin):
         for scenario_name, scenario_object in feature.scenarios.items():
             # skip already bound scenarios
@@ -386,9 +367,6 @@ def _scenario():
                 for test_name in get_python_name_generator(scenario_name):
                     if test_name not in module.__dict__:
                         # found an unique test name
-                        # recreate function to set line number
-                        _scenario = recreate_function(_scenario, module=module, firstlineno=index * 4)
-                        index += 1
                         module.__dict__[test_name] = _scenario
                         break
             found = True

pytest_bdd/steps.py

@@ -32,7 +32,6 @@ def article(author):
 """
 
 from __future__ import absolute_import
-from types import CodeType
 import inspect
 import sys
 
@@ -41,7 +40,6 @@ def article(author):
     from _pytest import fixtures as pytest_fixtures
 except ImportError:
     from _pytest import python as pytest_fixtures
-import six
 
 from .feature import parse_line, force_encode
 from .types import GIVEN, WHEN, THEN
@@ -90,7 +88,7 @@ def step_func(request):
         func = pytest.fixture(scope=scope)(lambda: step_func)
         func.__doc__ = 'Alias for the "{0}" fixture.'.format(fixture)
         _, name = parse_line(name)
-        contribute_to_module(module, get_step_fixture_name(name, GIVEN), func)
+        setattr(module, get_step_fixture_name(name, GIVEN), func)
         return _not_a_fixture_decorator
 
     return _step_decorator(GIVEN, name, converters=converters, scope=scope, target_fixture=target_fixture)
@@ -185,86 +183,12 @@ def lazy_step_func():
             step_func.converters = lazy_step_func.converters = converters
 
         lazy_step_func = pytest.fixture(scope=scope)(lazy_step_func)
-        contribute_to_module(
-            module=get_caller_module(),
-            name=get_step_fixture_name(parsed_step_name, step_type),
-            func=lazy_step_func,
-        )
-
+        setattr(get_caller_module(), get_step_fixture_name(parsed_step_name, step_type), lazy_step_func)
         return func
 
     return decorator
 
 
-def recreate_function(func, module=None, name=None, add_args=[], firstlineno=None):
-    """Recreate a function, replacing some info.
-
-    :param func: Function object.
-    :param module: Module to contribute to.
-    :param add_args: Additional arguments to add to function.
-
-    :return: Function copy.
-    """
-    def get_code(func):
-        return func.__code__ if six.PY3 else func.func_code
-
-    def set_code(func, code):
-        if six.PY3:
-            func.__code__ = code
-        else:
-            func.func_code = code
-
-    argnames = [
-        "co_argcount", "co_nlocals", "co_stacksize", "co_flags", "co_code", "co_consts", "co_names",
-        "co_varnames", "co_filename", "co_name", "co_firstlineno", "co_lnotab", "co_freevars", "co_cellvars",
-    ]
-    if six.PY3:
-        argnames.insert(1, "co_kwonlyargcount")
-        if sys.version_info.minor >= 8:
-            argnames.insert(1, "co_posonlyargcount")
-
-    for arg in get_args(func):
-        if arg in add_args:
-            add_args.remove(arg)
-
-    args = []
-    code = get_code(func)
-    for arg in argnames:
-        if module is not None and arg == "co_filename":
-            args.append(module.__file__)
-        elif name is not None and arg == "co_name":
-            args.append(name)
-        elif arg == "co_argcount":
-            args.append(getattr(code, arg) + len(add_args))
-        elif arg == "co_varnames":
-            co_varnames = getattr(code, arg)
-            args.append(co_varnames[:code.co_argcount] + tuple(add_args) + co_varnames[code.co_argcount:])
-        elif arg == "co_firstlineno":
-            args.append(firstlineno if firstlineno else 1)
-        else:
-            args.append(getattr(code, arg))
-
-    set_code(func, CodeType(*args))
-    if name is not None:
-        func.__name__ = name
-    return func
-
-
-def contribute_to_module(module, name, func):
-    """Contribute a function to a module.
-
-    :param module: Module to contribute to.
-    :param name: Attribute name.
-    :param func: Function object.
-
-    :return: New function copy contributed to the module
-    """
-    name = force_encode(name)
-    func = recreate_function(func, module=module)
-    setattr(module, name, func)
-    return func
-
-
 def get_caller_module(depth=2):
     """Return the module of the caller."""
     frame = sys._getframe(depth)
@@ -274,16 +198,6 @@ def get_caller_module(depth=2):
     return module
 
 
-def get_caller_function(depth=2):
-    """Return caller function."""
-    return sys._getframe(depth)
-
-
-def execute(code, g):
-    """Execute given code in given globals environment."""
-    exec(code, g)
-
-
 def inject_fixture(request, arg, value):
     """Inject fixture into pytest fixture request.
 

tests/feature/test_scenario.py

@@ -57,11 +57,19 @@ def test2():
     test2(request)
 
 
-def test_scenario_not_decorator(request):
+def test_scenario_not_decorator(testdir):
     """Test scenario function is used not as decorator."""
-    func = scenario(
-        'comments.feature',
-        'Strings that are not comments')
+    testdir.makefile('.feature', foo="""
+        Scenario: Foo
+            Given I have a bar
+        """)
+    testdir.makepyfile("""
+        from pytest_bdd import scenario
+
+        test_foo = scenario('foo.feature', 'Foo')
+        """)
+
+    result = testdir.runpytest()
 
-    with pytest.raises(exceptions.ScenarioIsDecoratorOnly):
-        func(request)
+    result.assert_outcomes(failed=1)
+    result.stdout.fnmatch_lines("*ScenarioIsDecoratorOnly: scenario function can only be used as a decorator*")