ronaldoussoren / modulegraph2

Modulegraph2 is a library for creating and introspecting the dependency graph between Python modules.
MIT License
11 stars 8 forks source link

PyTest's AssertionRewritingHook not recognised #8

Open marty-se opened 4 years ago

marty-se commented 4 years ago

I tried modulegraph2 on a project that uses pytest, but ended up with:

RuntimeError: Don't known how to handle <_pytest.assertion.rewrite.AssertionRewritingHook object at 0x117370c50> for <module>

Obviously, there is no case for AssertionRewritingHook (nor importlib.abc.MetaPathFinder which it implements: https://github.com/pytest-dev/pytest/blob/master/src/_pytest/assertion/rewrite.py#L56) in node_for_spec(). I'm however not sure what the appropriate solution would be...?

Thanks.

ronaldoussoren commented 4 years ago

I need to look into this. I thought I had added support for all of importlib, but I clearly missed the MetaPathFinder case :-(

ronaldoussoren commented 4 years ago

How can I reproduce the issue?

I've tried the following script from the pytest documentation:

import pytest

def checkconfig(x):
    __tracebackhide__ = True
    if not hasattr(x, "config"):
        pytest.fail("not configured: {}".format(x))

def test_something():
    checkconfig(42)

That works fine though, even if I add other imports below the import of pytest.

marty-se commented 4 years ago

Hmm, right, I realise now that my issue is probably related to the fact that I use modulegraph2 from a pytest plugin.

I have a script that looks like this:

import re

import modulegraph2
import pytest

class MyPlugin:

    def __init__(self):
        self.collected = []
        self.modulegraph = modulegraph2.ModuleGraph()

    def pytest_collection_modifyitems(self, items):
        for item in items:
            nodeid = item.nodeid
            module_spec = self._module_spec_from_nodeid(nodeid)

            self.modulegraph.add_module(module_spec)

    def _module_spec_from_nodeid(self, nodeid: str) -> str:
        matches = re.match(r'(.*)\.py::.*', nodeid)
        assert matches
        filepath_part = matches[0]
        return filepath_part.replace('/', '.')

def main():
    my_plugin = MyPlugin()
    pytest.main(['--collect-only', 'tests/'], plugins=[my_plugin])

if __name__ == '__main__':
    main()

... and I also have a tests/test_foo.py file with the contents from your example. I can then reproduce the error by running my script.

I guess pytest installs the import hook while collecting tests...

ronaldoussoren commented 4 years ago

Thanks. Having a reproducer makes it easier to fix the issue.

ronaldoussoren commented 4 years ago

I can reproduce the issue, but fixing this cleanly is harder. The AssertionRewritingHook in the traceback does not implement the API that modulegraph2 uses to inspect the loading process.

ronaldoussoren commented 4 years ago

I've filed a feature request with PyTest: https://github.com/pytest-dev/pytest/issues/7804

The requests asks to implement the InspectLoader ABC, which would solve the issue without changes in modulegraph2.

marty-se commented 4 years ago

Thank you so much for looking into this. Fixing it on the pytest side sounds reasonable.

ronaldoussoren commented 4 years ago

The pytest project is open for the idea, but requires a pull request. I'm not sure when I'll get around to that.

I'm keeping this open to track the pytest feature request, and because I want to add a modulegraph2 test to ensure this keeps working after it is implemented.

ronaldoussoren commented 4 years ago

PR for the pytest changes: https://github.com/pytest-dev/pytest/pull/7885

That PR is currently incomplete, I've primarily created it to ask if the proposed code changes are acceptable. But with the PR the reproducer for this issue no longer fails (although I still need to create code to validate the structure of the generated graph).