Fix getting module with --import-mode=importlib

[The-Compiler]

With the pytest 6 RC, a new --import-mode=importlib was added, which uses importlib rather than sys.path hacks to import test modules. I'd really like to use it, and in future pytest releases that'll become the default.

Unfortunately, it fails with pytest-bdd:

tests/end2end/features/test_backforward_bdd.py:21: in <module>
    bdd.scenarios('backforward.feature')
.tox/py38-pyqt515/lib/python3.8/site-packages/pytest_bdd/scenario.py:296: in scenarios
    features_base_dir = get_features_base_dir(module)
.tox/py38-pyqt515/lib/python3.8/site-packages/pytest_bdd/scenario.py:241: in get_features_base_dir
    default_base_dir = os.path.dirname(caller_module.__file__)
E   AttributeError: 'NoneType' object has no attribute '__file__'

because module here is None (relevant code):

def scenarios(*feature_paths, **kwargs):
    frame = inspect.stack()[1]
    module = inspect.getmodule(frame[0])    # <- this is None

    features_base_dir = kwargs.get("features_base_dir")
    if features_base_dir is None:
        features_base_dir = get_features_base_dir(module)

This fix seems to work, and is inspired from how pytest imports modules in this case. I'm not sure if it's the right solution though, there might be some easier way towards the same goal?

cc @nicoddemus

[nicoddemus]

I think the approach is sound, and indeed it was the best way I could find to do the same thing in pytest. 👍

The failures are related to py27: you will need to update the custom find_module to only go down with the importlib approach on py35+ I think.

[The-Compiler]

Oh, I didn't realize pytest-bdd still supported Python 2... I ended up just moving the imports, as this scenario only happens with --import-mode=importlib which is in only avaialble a Python 3 only release of pytest. Thus, I don't think we should ever get over the if module is not None: on Python 2.

[nicoddemus]

Thus, I don't think we should ever get over the if module is not None: on Python 2.

You are right, good point!

Seems now only linters have failed. 👍

[The-Compiler]

Fixed!

All done! ✨ 🍰 ✨
1 file reformatted.

[codecov]

Codecov Report

Merging #381 into master will decrease coverage by 0.61%.
The diff coverage is 31.25%.

@@            Coverage Diff             @@
##           master     #381      +/-   ##
==========================================
- Coverage   95.51%   94.89%   -0.62%     
==========================================
  Files          48       48              
  Lines        1671     1686      +15     
  Branches      167      170       +3     
==========================================
+ Hits         1596     1600       +4     
- Misses         43       53      +10     
- Partials       32       33       +1     

Impacted Files	Coverage Δ
pytest_bdd/scenario.py	90.00% <31.25%> (-5.76%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8008c0f...94e0e28. Read the comment docs.

[youtux]

Thanks for the patch, @The-Compiler. Sorry for the late response.
Could you also add a test that would cover the invocation of pytest with --import-mode=importlib?

[The-Compiler]

I'll take a look - I wasn't sure how much work that'd be, but I guess with the migration to pytester/testdir tests and runpytest_subprocess that should work.

[nicoddemus]

Ref: pytest-dev/pytest#7245

[The-Compiler]

I tried to add a test to tests/feature/test_scenario.py:

def test_importlib(testdir):
    """Test scenario function with importlib import mode."""
    testdir.makefile(
        ".feature",
        simple="""
        Feature: Simple feature
            Scenario: Simple scenario
                Given I have a bar
        """,
    )
    testdir.makepyfile(
        """
        from pytest_bdd import scenario, given

        @scenario("simple.feature", "Simple scenario")
        def test_simple():
            pass

        @given("I have a bar")
        def bar():
            return "bar"
        """
    )

    result = testdir.runpytest_subprocess('--import-mode=importlib')
    result.assert_outcomes(passed=1)

but it fails with this inside the subprocess:

______________________ ERROR collecting test_importlib.py ______________________
test_importlib.py:3: in <module>
    @scenario("simple.feature", "Simple scenario")
/home/florian/proj/pytest-bdd/pytest_bdd/scenario.py:219: in scenario
    feature = Feature.get_feature(features_base_dir, feature_name, encoding=encoding)
/home/florian/proj/pytest-bdd/pytest_bdd/feature.py:389: in get_feature
    feature = Feature(base_path, filename, encoding=encoding)
/home/florian/proj/pytest-bdd/pytest_bdd/feature.py:279: in __init__
    with codecs.open(filename, encoding=encoding) as f:
/usr/lib/python3.8/codecs.py:905: in open
    file = builtins.open(filename, mode, buffering)
E   FileNotFoundError: [Errno 2] No such file or directory: '/home/florian/proj/pytest-bdd/pytest_bdd/simple.feature'

Note the wrong path to simple.feature - with or without my fix, I get that path (instead of some temporary directory) as soon as I add --import-mode=importlib to pytester...

@nicoddemus do you have an idea what's going on there?

[nicoddemus]

@nicoddemus do you have an idea what's going on there?

Sorry, no. 😞

One difference is that importlib doesn't add the imported test module to sys.modules nor changes sys.path, don't know if pytest-bdd might be doing something (like creating that file) as a side effect of that.

[youtux]

I'm trying to get this to work (starting from the test you posted) on another branch too: https://github.com/pytest-dev/pytest-bdd/tree/work-with-importlib.

I fixed the get_caller_module to also call the find_module function. This fixes the issue you posted (where simple.feature would be searched in the wrong directory.

Unfortunately, now there is another issue where the fixture pytestbdd_given_I have a bar cannot be found in the test module:

=================================== FAILURES ===================================
_________________________________ test_simple __________________________________

self = <FixtureRequest for <Function test_simple>>
argname = 'pytestbdd_given_I have a bar'

    def _get_active_fixturedef(
        self, argname: str
    ) -> Union["FixtureDef", PseudoFixtureDef]:
        try:
>           return self._fixture_defs[argname]
E           KeyError: 'pytestbdd_given_I have a bar'

/Users/youtux/Developer/pytest-bdd/.env/lib/python3.8/site-packages/_pytest/fixtures.py:589: KeyError

During handling of the above exception, another exception occurred:

request = <FixtureRequest for <Function test_simple>>
step = <pytest_bdd.feature.Step object at 0x10d006040>
scenario = <pytest_bdd.feature.Scenario object at 0x10d0063d0>
encoding = 'utf-8'

    def _find_step_function(request, step, scenario, encoding):
        """Match the step defined by the regular expression pattern.
    
        :param request: PyTest request object.
        :param step: Step.
        :param scenario: Scenario.
    
        :return: Function of the step.
        :rtype: function
        """
        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))

/Users/youtux/Developer/pytest-bdd/pytest_bdd/scenario.py:77: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <FixtureRequest for <Function test_simple>>
argname = 'pytestbdd_given_I have a bar'

    def getfixturevalue(self, argname: str) -> Any:
        """ Dynamically run a named fixture function.
    
        Declaring fixtures via function argument is recommended where possible.
        But if you can only decide whether to use another fixture at test
        setup time, you may use this function to retrieve it inside a fixture
        or test function body.
    
        :raise pytest.FixtureLookupError:
            If the given fixture could not be found.
        """
>       fixturedef = self._get_active_fixturedef(argname)

/Users/youtux/Developer/pytest-bdd/.env/lib/python3.8/site-packages/_pytest/fixtures.py:581: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <FixtureRequest for <Function test_simple>>
argname = 'pytestbdd_given_I have a bar'

    def _get_active_fixturedef(
        self, argname: str
    ) -> Union["FixtureDef", PseudoFixtureDef]:
        try:
            return self._fixture_defs[argname]
        except KeyError:
            try:
>               fixturedef = self._getnextfixturedef(argname)

/Users/youtux/Developer/pytest-bdd/.env/lib/python3.8/site-packages/_pytest/fixtures.py:592: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <FixtureRequest for <Function test_simple>>
argname = 'pytestbdd_given_I have a bar'

    def _getnextfixturedef(self, argname: str) -> "FixtureDef":
        fixturedefs = self._arg2fixturedefs.get(argname, None)
        if fixturedefs is None:
            # we arrive here because of a dynamic call to
            # getfixturevalue(argname) usage which was naturally
            # not known at parsing/collection time
            assert self._pyfuncitem.parent is not None
            parentid = self._pyfuncitem.parent.nodeid
            fixturedefs = self._fixturemanager.getfixturedefs(argname, parentid)
            # TODO: Fix this type ignore. Either add assert or adjust types.
            #       Can this be None here?
            self._arg2fixturedefs[argname] = fixturedefs  # type: ignore[assignment]
        # fixturedefs list is immutable so we maintain a decreasing index
        index = self._arg2index.get(argname, 0) - 1
        if fixturedefs is None or (-index > len(fixturedefs)):
>           raise FixtureLookupError(argname, self)
E           _pytest.fixtures.FixtureLookupError: ('pytestbdd_given_I have a bar', <FixtureRequest for <Function test_simple>>)

/Users/youtux/Developer/pytest-bdd/.env/lib/python3.8/site-packages/_pytest/fixtures.py:484: FixtureLookupError

During handling of the above exception, another exception occurred:

request = <FixtureRequest for <Function test_simple>>

    @pytest.mark.usefixtures(*function_args)
    def scenario_wrapper(request):
>       _execute_scenario(feature, scenario, request, encoding)

/Users/youtux/Developer/pytest-bdd/pytest_bdd/scenario.py:180: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
/Users/youtux/Developer/pytest-bdd/pytest_bdd/scenario.py:140: in _execute_scenario
    step_func = _find_step_function(request, step, scenario, encoding=encoding)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

request = <FixtureRequest for <Function test_simple>>
step = <pytest_bdd.feature.Step object at 0x10d006040>
scenario = <pytest_bdd.feature.Scenario object at 0x10d0063d0>
encoding = 'utf-8'

    def _find_step_function(request, step, scenario, encoding):
        """Match the step defined by the regular expression pattern.
    
        :param request: PyTest request object.
        :param step: Step.
        :param scenario: Scenario.
    
        :return: Function of the step.
        :rtype: function
        """
        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)
                raise
            except pytest_fixtures.FixtureLookupError:
>               raise exceptions.StepDefinitionNotFoundError(
                    u"""Step definition is not found: {step}."""
                    """ Line {step.line_number} in scenario "{scenario.name}" in the feature "{feature.filename}""".format(
                        step=step, scenario=scenario, feature=scenario.feature
                    )
                )
E               pytest_bdd.exceptions.StepDefinitionNotFoundError: Step definition is not found: Given "I have a bar". Line 3 in scenario "Simple scenario" in the feature "/private/var/folders/dy/_0849zr93fj1td2_07t4ybpr0000gn/T/pytest-of-youtux/pytest-9/test_importlib0/simple.feature

/Users/youtux/Developer/pytest-bdd/pytest_bdd/scenario.py:86: StepDefinitionNotFoundError
=========================== short test summary info ============================
FAILED test_importlib.py::test_simple - pytest_bdd.exceptions.StepDefinitionN...
============================== 1 failed in 0.13s ===============================
=========================== short test summary info ============================
FAILED tests/feature/test_scenario.py::test_importlib - AssertionError: asser...
======================= 1 failed, 98 deselected in 0.67s =======================

This happens only when running the test with --import-mode=importlib.
I still don't understand why that happens, I will continue investigating.

[youtux]

It seems that the module object where we set the fixture (done by _step_decorator) is not the same module object used by pytest when discovering fixtures (FixtureManager.parsefactories).
That's why the fixture is not found. Not sure if this is easy to fix.

I added some print statements so that I can confirm this.
This is the execution without --import-mode=importlib:

============================= test session starts ==============================
platform darwin -- Python 3.8.5, pytest-6.0.1, py-1.9.0, pluggy-0.13.1 -- /Users/youtux/Developer/pytest-bdd/.env/bin/python
cachedir: .pytest_cache
rootdir: /Users/youtux/Developer/pytest-bdd, configfile: pytest.ini
plugins: bdd-3.4.0
collecting ... 
----- setting "pytestbdd_given_I have a bar" on <module 'test_importlib' from '/Users/youtux/Developer/pytest-bdd/test_basd/test_importlib.py'>, 4450122224
FixtureManager.parsefactories. holderobj=<module 'test_importlib' from '/Users/youtux/Developer/pytest-bdd/test_basd/test_importlib.py'>, id: 4450122224)
collected 1 item

test_importlib.py::test_simple <- pytest_bdd/scenario.py 

============================== 1 passed in 5.71s ===============================

Process finished with exit code 0
PASSED

The pytestbdd_given_I have a bar is set on the same module object that is later read by FixtureManager.parsefactories.

This is instead the execution with --import-mode=importlib:

============================= test session starts ==============================
platform darwin -- Python 3.8.5, pytest-6.0.1, py-1.9.0, pluggy-0.13.1 -- /Users/youtux/Developer/pytest-bdd/.env/bin/python
cachedir: .pytest_cache
rootdir: /Users/youtux/Developer/pytest-bdd, configfile: pytest.ini
plugins: bdd-3.4.0
collecting ... 
----- setting "pytestbdd_given_I have a bar" on <module 'test_importlib' from '/Users/youtux/Developer/pytest-bdd/test_basd/test_importlib.py'>, 4488468576
FixtureManager.parsefactories. holderobj=<module 'test_importlib' from '/Users/youtux/Developer/pytest-bdd/test_basd/test_importlib.py'>, id: 4484315872)
collected 1 item

test_importlib.py::test_simple <- pytest_bdd/scenario.py FAILED
pytest_bdd/scenario.py:177 (test_simple)
self = <FixtureRequest for <Function test_simple>>

The fixture is set on a different module object, and the fixture lookup will fail.

[youtux]

So it seems that indeed find_module() is not returning the same module object that is executing; it uses importlib.module_from_spec(), which "Creates a module based on the provided spec.".
I can't find a way to fix it.

[youtux]

I may have found a way, replacing (in _step_decorator)

setattr(get_caller_module(), get_step_fixture_name(parsed_step_name, step_type), lazy_step_func)

with

prev_frame = inspect.stack()[1]
prev_frame.frame.f_locals[fixture_step_name] = lazy_step_func

[youtux]

Fixed in #384. Thanks for your effort and for bringing this up.