pytest-dev / pytest

The pytest framework makes it easy to write small tests, yet scales to support complex functional testing
https://pytest.org
MIT License
12.11k stars 2.68k forks source link

Making rewrite and import of custom DSL play nice #3477

Open asmodehn opened 6 years ago

asmodehn commented 6 years ago

I am currently writing a custom importer for a Domain Specific Language.

Lets call calc the module written in a simple DSL. Meaning that I do:

with my_custom_importer():  # installs the loader in the path hooks
    from . import calc

The import works fine, the DSL is parsed with the Loader, which override the get_source method, transforming the DSL into python source and importing it.

The problem arises when I use pytest in some test modules:

with my_custom_importer():  # installs the loader in the path hooks
    from . import calc

def test_calc():
    assert calc.works_fine()

if __name__ == '__main__':
    pytest.main(['-s'])

and when pytest attempts to rewrite the asserts in there :

tests/test_lark/calc/tests/test_parse.py:None (tests/test_lark/calc/tests/test_parse.py)
/home/alexv/.virtualenvs/replator/lib/python3.5/site-packages/_pytest/python.py:412: in _importtestmodule
    mod = self.fspath.pyimport(ensuresyspath=importmode)
/home/alexv/.virtualenvs/replator/lib/python3.5/site-packages/py/_path/local.py:668: in pyimport
    __import__(modname)
<frozen importlib._bootstrap>:969: in _find_and_load
    ???
<frozen importlib._bootstrap>:958: in _find_and_load_unlocked
    ???
<frozen importlib._bootstrap>:664: in _load_unlocked
    ???
<frozen importlib._bootstrap>:634: in _load_backward_compatible
    ???
/home/alexv/.virtualenvs/replator/lib/python3.5/site-packages/_pytest/assertion/rewrite.py:213: in load_module
    py.builtin.exec_(co, mod.__dict__)
calc/tests/test_parse.py:5: in <module>
    from .. import calc
E     File "/home/alexv/Projects/palimport/tests/test_lark/calc/calc.lark", line 1
E       //
E        ^
E   SyntaxError: invalid syntax

It breaks on the DSL syntax in that module. It seems it doesn't use the custom loader for that module...

Any idea how to investigate this and get it right ? Any example of pytest running with custom importers (and non-python syntax in module) ? Or is there any way to disable the rewrite, in cases like this where it might get us into trouble ?

my pytest setup:

(replator) alexv@alexv-XPS-Tablet:~/Projects/palimport$ pytest --version
This is pytest version 3.5.1, imported from /home/alexv/.virtualenvs/replator/lib/python3.5/site-packages/pytest.py
setuptools registered plugins:
  pytest-xdist-1.22.2 at /home/alexv/.virtualenvs/replator/lib/python3.5/site-packages/xdist/plugin.py
  pytest-xdist-1.22.2 at /home/alexv/.virtualenvs/replator/lib/python3.5/site-packages/xdist/looponfail.py
  pytest-forked-0.2 at /home/alexv/.virtualenvs/replator/lib/python3.5/site-packages/pytest_forked/__init__.py

I ll try to get a minimal reproducible example, but that might be tricky, given the machinery involved...

pytestbot commented 6 years ago

GitMate.io thinks possibly related issues are https://github.com/pytest-dev/pytest/issues/1871 (No assertion rewriting in files imported from conftest), https://github.com/pytest-dev/pytest/issues/1840 (register_assert_rewrite warns for already imported and rewritten modules), https://github.com/pytest-dev/pytest/issues/2435 (f), https://github.com/pytest-dev/pytest/issues/295 (lazy import of doctest in pdb breaks debugging if the environment makes doctest unimportable), and https://github.com/pytest-dev/pytest/issues/296 (lazy import of doctest in pdb breaks debugging if the environment makes doctest unimportable).

Sup3rGeo commented 6 years ago

@asmodehn , I'm no core developer but probably you want to look at the find_module and load_module methods in the AssertionRewritingHook class and see what it does: https://github.com/pytest-dev/pytest/blob/master/_pytest/assertion/rewrite.py

The import hook is installed in the install_importhook function in the init module: https://github.com/pytest-dev/pytest/blob/master/_pytest/assertion/__init__.py

nicoddemus commented 6 years ago

@Sup3rGeo's tips are on point! 👍

I don't have time to check right now, but it might the order of the hooks because we install the assertion rewriter hook with:

https://github.com/pytest-dev/pytest/blob/93fdad28aae28342940156701b813f6743b6e317/_pytest/assertion/__init__.py#L75-L76

Your my_custom_importer context manager might be installing your hook using sys.meta_path.append, which would explain why pytest's hook runs first and fails to parse the file.

asmodehn commented 6 years ago

Oh this is interesting...

I used to assume the correct practice was to put custom hooks at the end of the list (sys.path_hooks or sys.meta_path), to minimize changes in early import sequence in case other tools did the same...

Then I eventually found out that the python3 FileFinder always expect to be the last one (hook always return something which prevent Finder to look into other hooks further in the list).

So now I append in sys.meta_path, and in sys.path_hooks, I look for filefinder and I put my hooks just before. I dont want to change the usual import mechanism, just add the ability to import something else...

Was I mistaken and hooks should be always prepended to the list ? Is there some general advice about it somewhere that I missed ?

On Fri, May 25, 2018, 01:57 Bruno Oliveira notifications@github.com wrote:

I don't have time to check right now, but it might the order of the hooks, because we install the assertion rewriter hook with:

https://github.com/pytest-dev/pytest/blob/93fdad28aae28342940156701b813f6743b6e317/_pytest/assertion/__init__.py#L75-L76

You might want to do the same within your my_custom_importer context manager, this will ensure your hook runs first.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/pytest-dev/pytest/issues/3477#issuecomment-391901087, or mute the thread https://github.com/notifications/unsubscribe-auth/AANgSBKQE-abSZz2uFDTsThMo7649ksAks5t10kCgaJpZM4T_QWX .