Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"--import-mode=importlib" not working? #233

Open
andioz opened this issue Oct 14, 2021 · 9 comments
Open

"--import-mode=importlib" not working? #233

andioz opened this issue Oct 14, 2021 · 9 comments

Comments

@andioz
Copy link

andioz commented Oct 14, 2021

Hi,

I want to make my first steps with pytest-cases, but I'm stumbling immediately. I added a simple test using @parametrize_with_cases, but this fails with ModuleNotFoundError:

======================================================= test session starts ========================================================
platform linux -- Python 3.8.10, pytest-6.2.5, py-1.10.0, pluggy-1.0.0
rootdir: /tmp/pytest_cases_sample/tests, configfile: pytest.ini
plugins: pythonpath-0.7.3, cases-3.6.4, mock-3.6.1
collected 0 items / 1 error                                                                                                        

============================================================== ERRORS ==============================================================
______________________________________________ ERROR collecting mypack/test_dummy.py _______________________________________________
ImportError while importing test module '/tmp/pytest_cases_sample/tests/mypack/test_dummy.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
tests/mypack/test_dummy.py:14: in <module>
    def test_greet_with_cases_passes(name):
venv/lib/python3.8/site-packages/pytest_cases/common_pytest.py:893: in apply
    container = get_function_host(test_or_fixture_func)
venv/lib/python3.8/site-packages/pytest_cases/common_others.py:251: in get_function_host
    host = get_host_module(func)
venv/lib/python3.8/site-packages/pytest_cases/common_others.py:226: in get_host_module
    return import_module(a.__module__)
/usr/lib/python3.8/importlib/__init__.py:127: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
E   ModuleNotFoundError: No module named 'test_dummy'
===================================================== short test summary info ======================================================
ERROR tests/mypack/test_dummy.py
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! Interrupted: 1 error during collection !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
========================================================= 1 error in 0.12s =========================================================

The example project was extracted from my real project, including the pytest.ini file. After some time I recognized, the addopts = --import-mode=importlib line in the config causes the problem.

Well, OK, one way would be to change this setting, but because I have an existing middle-sized project, I don't want to change settings without a detailed knowledge about all side-effects.

I wonder, is this import-mode not supported by pytest-cases or am I missing some other required settings? I attach my sample project here.

Many thanks!

pytest_cases_sample.zip

PS: quote from https://docs.pytest.org/en/6.2.x/pythonpath.html:

We intend to make importlib the default in future releases.

@smarie
Copy link
Owner

smarie commented Oct 14, 2021

Thanks a lot @andioz for the feedback ! From the pytest documentation link that you point out:

--import-mode=importlib [...] makes test modules non-importable by each other. [...] Users which require this should turn their tests into proper packages instead.

I tried to modify the tests structure in your example to add __init__.py files everywhere but it deos not work.. :(

There is a long thread of discussion here, maybe we'll find an answer here ? pytest-dev/pytest#7245

An important point is here: pytest-dev/pytest#7245 (comment)

One way to do that is to add addopts = --importmode=prepend to your pytest.ini file. We don't intend to remove the prepend mode in the future at all.

I'll try that

@andioz
Copy link
Author

andioz commented Oct 14, 2021

I tried to modify the tests structure in your example to add __init__.py files everywhere but it deos not work.. :(

Yeah sure, I don't know where I've read about __init__.py in test directories, but I'm sure this is to avoid anyway.

There is a long thread of discussion here, maybe we'll find an answer here ? pytest-dev/pytest#7245

This is a good resource I think.

Of course prepend or the more defensive append will work for the example and I guess for my project too, I have to check. But if importlib will become the default in future, many questions will arise by pytest-cases-newbies like me!

@smarie
Copy link
Owner

smarie commented Oct 14, 2021

Indeed, fair point. I confirm that import-mode=prepend works since this is the default. The pytest team says that this will not disappear, it will just not be the default anymore. So we have a workaround.

Now can we do better ? Currently there are three places where using import-mode=importlib breaks pytest-cases:

  • @parametrize_with_cases needs to be able to find the module object where the decorated function lives, in order to create fixtures in there, when some cases need to be transformed into fixtures (because they are either parametrized or require fixtures themselves).

  • @parametrize needs to be able to find the module object where the decorated function lives, in order to create a "union" fixture and possible "parameter' fixtures in there, when some parameters are fixture references.

  • finally, when users define fixtures in their cases-containing files, an existing workaround is to import them from that other file. See Feature Request: Ability to define fixtures in case file #228

All those behaviours could be removed (I guess) by any of the following :

  • finding a reliable way to get the test module object from a test function, that works in all modes. Currently I use importlib.import_module(f.__module__) to find the module object from the test fonction f, but this is failing when import-mode=importlib

  • registering fixtures directly with the fixture manager, rather than adding them in the module. That way no need to find the module :)

I would be happy to hear about any solution to either of these, if anyone wishes to take a stab. Meanwhile, I leave this open as a "has_workaround" but do not add the "bug" label until this is the pytest default.

Thanks again !

@andioz
Copy link
Author

andioz commented Oct 14, 2021

I'm not sure, but it looks like the pytest-flake8 plugin has no problem with this option, but pytest-pylint has trouble to find the modules too. Maybe worth to have a look.

@smarie
Copy link
Owner

smarie commented Oct 15, 2021

Thanks for the hint !

Do pytest-flake8/coverage/pylint bring any additional value rather than using flake8/coverage/pylint ? I personally always use the latter, and it works fine. Of course other users might think differently, this is why it is definitely important to keep such a comment here.

All those behaviours could be removed (I guess) by any of the following :

* finding a reliable way to get the test module object from a test function, that works in all modes. Currently I use `importlib.import_module(f.__module__)` to find the module object from the test fonction f, but this is failing when `import-mode=importlib`

I poked around and found out that maybe I could use f.__globals__ as a fallback instead of importlib.import_module(f.__module__), in order to get the module __dict__. It seems to be the actual same object behind the scenes, and it is writeable. However it seems like a weird hack to me. I'll push a PR so that users can possibly test this.

@andioz
Copy link
Author

andioz commented Oct 15, 2021

Thank you for spending time on this! Like you, I run flake8/pylint without pytest plugins manually at the moment. Otherwise, I use the coverage plugin on a regular basis, especially with CI runner, this is a nice single command to get all information at once.

@smarie
Copy link
Owner

smarie commented Oct 15, 2021

Neat. I personally use coverage run -m pytest ... in the CI, it works as a charm as well.

I tried to implement the hack discussed above. It works, but uncovers another issue, that is more fundamental:

  • by design, @parametrize_with_cases's cases arg supports that users pass a string representing a (possibly relative) module name. (see doc) This of course requires that all test modules can be imported. Such a feature would become extremely complicated to implement in the new import-mode where test modules can not import each other in a relative way anymore.

I therefore suggest to stop investigating for the moment, and wait until the landscape around the new import mechanisms in pytest gets a little clearer.
Thanks once again @andioz , and have a great week-end!

@ThomasTNO
Copy link

Personally I think it would be nice to roll out the above 'hack' without supporting relative imports when using importlib import-mode. Maybe even raise an exception when one does try to use the unsupported combination, for now.

@smarie
Copy link
Owner

smarie commented Jan 4, 2022

Thanks @ThomasTNO for the feedback ! I do not have the bandwidth to propose such a feature, but would be happy to review a minimalistic PR that would

  • detect the current import-mode, probably in plugin.py
  • when the import-mode is importlib, raise an error when a string is provided in the cases argument of @parametrize_with_cases
  • when the import-mode is importlib, write variables in f.__globals__ instead of writing them in importlib.import_module(f.__module__) (in all situations where variables need to be created in the hosting module of the case function, fixture or test function).
  • provide a dedicated test to validate that this works. This probably would need to be a metatest, that is, using the pytester plugin to launch an independent pytest process using the import-mode.

Let me know !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants