Skip to content

Fix importer call #1837

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

Closed
wants to merge 1 commit into from
Closed

Fix importer call #1837

wants to merge 1 commit into from

Conversation

mbachry
Copy link
Contributor

@mbachry mbachry commented Aug 21, 2016

get_filename expects full module name as a parameter. Currently pytest fails, if I run it with zip importer.

@RonnyPfannschmidt
Copy link
Member

thanks, the fix is correct according to https://www.python.org/dev/peps/pep-0302/

i wonder if there is a simple way to test it

how did you create the zip thats running pytest?

@coveralls
Copy link

Coverage Status

Coverage remained the same at 93.019% when pulling 678750c on mbachry:master into af0ec12 on pytest-dev:master.

@mbachry
Copy link
Contributor Author

mbachry commented Aug 21, 2016

I run tests from zips in my project (https://github.com/mbachry/exxo/), but here's a relatively simple way to reproduce, if you have access to Linux with Python 3.5:

$ mkdir app
$ cat > app/foo.py
import pytest

def main():
    pytest.main(['--pyarg', 'foo'])
<Ctrl-D>
$ python3.5 -m zipapp -o /tmp/foo.zip -m foo:main app
$ python3.5 /tmp/foo.zip 
============================================================================ test session starts =============================================================================
platform linux -- Python 3.5.2, pytest-3.0.1.dev, py-1.4.31, pluggy-0.3.1
rootdir: /home/marcin/pytest, inifile: tox.ini
INTERNALERROR> Traceback (most recent call last):
INTERNALERROR>   File "/home/marcin/pytest/_pytest/main.py", line 96, in wrap_session
INTERNALERROR>     session.exitstatus = doit(config, session) or 0
INTERNALERROR>   File "/home/marcin/pytest/_pytest/main.py", line 130, in _main
INTERNALERROR>     config.hook.pytest_collection(session=session)
INTERNALERROR>   File "/home/marcin/pytest/_pytest/vendored_packages/pluggy.py", line 724, in __call__
INTERNALERROR>     return self._hookexec(self, self._nonwrappers + self._wrappers, kwargs)
INTERNALERROR>   File "/home/marcin/pytest/_pytest/vendored_packages/pluggy.py", line 338, in _hookexec
INTERNALERROR>     return self._inner_hookexec(hook, methods, kwargs)
INTERNALERROR>   File "/home/marcin/pytest/_pytest/vendored_packages/pluggy.py", line 333, in <lambda>
INTERNALERROR>     _MultiCall(methods, kwargs, hook.spec_opts).execute()
INTERNALERROR>   File "/home/marcin/pytest/_pytest/vendored_packages/pluggy.py", line 596, in execute
INTERNALERROR>     res = hook_impl.function(*args)
INTERNALERROR>   File "/home/marcin/pytest/_pytest/main.py", line 139, in pytest_collection
INTERNALERROR>     return session.perform_collect()
INTERNALERROR>   File "/home/marcin/pytest/_pytest/main.py", line 590, in perform_collect
INTERNALERROR>     items = self._perform_collect(args, genitems)
INTERNALERROR>   File "/home/marcin/pytest/_pytest/main.py", line 608, in _perform_collect
INTERNALERROR>     parts = self._parsearg(arg)
INTERNALERROR>   File "/home/marcin/pytest/_pytest/main.py", line 702, in _parsearg
INTERNALERROR>     parts[0] = self._tryconvertpyarg(parts[0])
INTERNALERROR>   File "/home/marcin/pytest/_pytest/main.py", line 690, in _tryconvertpyarg
INTERNALERROR>     path = loader.get_filename()
INTERNALERROR> TypeError: zipimporter.get_filename() takes exactly 1 argument (0 given)

Not sure if it's easy to turn it into a proper unit test...

@nicoddemus
Copy link
Member

I think we should add a test that does exactly what @mbachry posted. I will try to get to it tonight, unless @mbachry wants to tackle it.

@mbachry
Copy link
Contributor Author

mbachry commented Aug 22, 2016

@nicoddemus Thanks. I'll leave it to you then.

nicoddemus added a commit to nicoddemus/pytest that referenced this pull request Aug 22, 2016
nicoddemus added a commit to nicoddemus/pytest that referenced this pull request Aug 22, 2016
@nicoddemus
Copy link
Member

Superseded by #1844.

Thanks again @mbachry!

@nicoddemus nicoddemus closed this Aug 22, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants