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

Inconsistent behaviour of `python3 -m pytest` and `py.test` #911

Closed
sils opened this issue Aug 4, 2015 · 26 comments
Closed

Inconsistent behaviour of `python3 -m pytest` and `py.test` #911

sils opened this issue Aug 4, 2015 · 26 comments

Comments

@sils
Copy link

@sils sils commented Aug 4, 2015

See http://pastebin.com/52ra33eN

Both commands show

platform linux -- Python 3.4.3 -- py-1.4.30 -- pytest-2.7.2
rootdir: /home/lasse/prog/PyPrint, inifile: setup.cfg
plugins: spec, cov, xdist

but py.test isn't able to get the imports right.

Steps to reproduce:

  • git clone https://github.com/coala-analyzer/PyPrint
  • git checkout pytestrep (Auto sync with origin/pytestrep)
  • py.test/python3 -m pytest in a python3 environment
@sils
Copy link
Author

@sils sils commented Aug 4, 2015

The error is that I missed to put __init__.py files in the package. Still I find this inconsistent behaviour alarming, there's probably a bug behind that. Also note that I got similarly weird results when running python -m pytest on CI: it worked only for python 3.3 and 3.4 but not for 3.2 and pypy 3.2 which raised the import error agian.

@RonnyPfannschmidt
Copy link
Member

@RonnyPfannschmidt RonnyPfannschmidt commented Sep 13, 2015

@sils1297 this is a artifact of the new style namespaces

@Zearin
Copy link
Contributor

@Zearin Zearin commented Feb 2, 2016

Perhaps you should raise a Warning to let the user know this may occur. (Bonus points for adding a notice to the documentation, too...but the immediate relevancy of a Warning in the terminal is a better first step, IMO.)

@ssangervasi
Copy link

@ssangervasi ssangervasi commented Apr 5, 2016

I am also seeing different behavior between py.test and python -m pytest using Python 2.7. The latter always works, while the former can only find tests if I have __init__.py in my test directories. Technically init files solve the issue, but this is explicitly advised against in the pytest documentation.

@RonnyPfannschmidt is there any further reading on "new style namespaces"?

@RonnyPfannschmidt
Copy link
Member

@RonnyPfannschmidt RonnyPfannschmidt commented Apr 5, 2016

@ssangervasi new style namespaces where introduced in python3 - there no longer is a __init__.py with a namespace declaration needed

can you perhaps show full output of the invocation

another potential issue in your case is pythonpath vs imports in tests vs rootdirs which is unrelated to this issue

@ssangervasi
Copy link

@ssangervasi ssangervasi commented Apr 6, 2016

@RonnyPfannschmidt thanks for the reply. You were correct and I now understand that my issue was unrelated. I was overlooking the fact that python -m ... adds the current directory to PYTHONPATH.

@RonnyPfannschmidt
Copy link
Member

@RonnyPfannschmidt RonnyPfannschmidt commented Apr 7, 2016

Py.test never does that unless there is a toplevel conftest, please give US more information

@santagada
Copy link

@santagada santagada commented Jul 15, 2016

Seeing the same problem, python3 -m pytest works and py.test doesn't for xonsh. it is a PYTHONPATH issue and the docs are very misleading in http://docs.pytest.org/en/latest/usage.html#cmdline:

python -m pytest [...]
This is equivalent to invoking the command line script py.test [...] directly.

no they are not, exactly because of pythonpath. Can we just do that on py.test?

@DuncanBetts
Copy link
Contributor

@DuncanBetts DuncanBetts commented Nov 26, 2016

Perhaps you should raise a Warning to let the user know this may occur. (Bonus points for adding a notice to the documentation, too...but the immediate relevancy of a Warning in the terminal is a better first step, IMO.)

Perhaps Pull 2085 implementing @enkore's suggestion here adds this?

@nicoddemus
Copy link
Member

@nicoddemus nicoddemus commented Nov 26, 2016

@DuncanBetts I'm not sure I follow, it seems the error is not really related to --pyargs, but the fact that python -m pytest implicitly adds the current directory to PYTHONPATH.

Either way I think updating the documentation would be a good first step as well.

@nicoddemus
Copy link
Member

@nicoddemus nicoddemus commented Sep 28, 2017

@bilderbuchi
Copy link
Contributor

@bilderbuchi bilderbuchi commented Oct 18, 2017

Documentation added in #2850

@nicoddemus
Copy link
Member

@nicoddemus nicoddemus commented Oct 18, 2017

I think we have added enough to the docs alerting to the difference between the two invocations so I'm closing this.

@nicoddemus nicoddemus closed this Oct 18, 2017
@bilderbuchi
Copy link
Contributor

@bilderbuchi bilderbuchi commented Oct 19, 2017

Well, couldn't you make the behaviour of pytest match the behaviour of python -m pytest, as a "real" fix, or is that undesirable?

@RonnyPfannschmidt
Copy link
Member

@RonnyPfannschmidt RonnyPfannschmidt commented Oct 19, 2017

@bilderbuchi that would be a real breaking change for a lit of people - so we cant just go ahead and do that

@bilderbuchi
Copy link
Contributor

@bilderbuchi bilderbuchi commented Oct 19, 2017

I see. Still, is this something you want to do in the future, e.g. at next-major-version? If so, I guess it should be tracked.

Personally, in the way I teach people to set up python projects (scientific scripts etc), I've resorted to just putting a section in the readme that says "try to run pytest, if it fails with an import error, run the (a bit more complex) python -m pytest instead". Which is a workaround, but a bit annoying, and I feel it would be good if the inconsistency would one day disappear.
I guess the situation is a little similar to the existence of both py.test vs. pytest in the past.

@RonnyPfannschmidt
Copy link
Member

@RonnyPfannschmidt RonnyPfannschmidt commented Oct 19, 2017

personally its not something im interested in (i just always use pytest)

but feel free to open up a tracking issue and participate if its something you find helpful

@bilderbuchi
Copy link
Contributor

@bilderbuchi bilderbuchi commented Oct 19, 2017

That is kinda my point, this issue is already the tracking issue for the behaviour in question (and should maybe be reopened), and I am participating. ;-)

@nicoddemus
Copy link
Member

@nicoddemus nicoddemus commented Oct 19, 2017

couldn't you make the behaviour of pytest match the behaviour of python -m pytest, as a "real" fix, or is that undesirable?

@bilderbuchi I sympathize with your problem, but I really think this is not something that we should do. Besides being backward incompatible and probably cause a lot of breakages, it would be undesirable to automatically add the current directory to sys.path: depending on where you are, you will now get different importing semantics (which is the problem with python -m pytest).

@bilderbuchi
Copy link
Contributor

@bilderbuchi bilderbuchi commented Oct 19, 2017

OK, thank you. I understand.

So, in case I want to have a simple project with some, say, data processing script or other one-off stuff that's not installed elsewhere, i.e. a layout like this:

./README.md
./some_project.py
./tests
./tests/conftest.py
./tests/test_some_project.py

what is best practice to make this work with a plain pytest invocation? Is adding a __init__.py file within the tests directory OK/advisable in this case? Pretty often I see recommendations against doing that.
It does seem to work, maybe that's all it takes? The use case is I think similar to https://docs.pytest.org/en/latest/goodpractices.html#tests-as-part-of-application-code

@RonnyPfannschmidt
Copy link
Member

@RonnyPfannschmidt RonnyPfannschmidt commented Oct 19, 2017

@bilderbuchi the suggestion agasint __init__ is because it prevents accidentially testing a source tree when you want to test a installed package

so in your case where you dont install and want to test the local code its perfectly fine, but perhaps worth a comment in the init.py to help understanding should it ever be turned into a installable package

@bilderbuchi
Copy link
Contributor

@bilderbuchi bilderbuchi commented Oct 19, 2017

Great, thanks!

@bilderbuchi
Copy link
Contributor

@bilderbuchi bilderbuchi commented Oct 24, 2017

Also: thank you for this great tool! 👏

@Zearin
Copy link
Contributor

@Zearin Zearin commented Nov 5, 2017

In situations like this—where any change in behavior to fix one problem will probably create other problems, but where both situations are completely valid—I think the best thing to do is to issue a clear notification to the user about the situation, and inform them on how to adjust their invocation of pytest if necessary.

From the pytest docs:

You can invoke testing through the Python interpreter from the command line:

python -m pytest [...]
This is almost equivalent to invoking the command line script pytest [...] directly, except that calling via python will also add the current directory to sys.path.

That is pretty clear in the docs. But ideally, pytest would detect when it is invoked via python -m pytest and print a very noticeable message to the user indicating the potential for sys.path related nonsense.

@nicoddemus
Copy link
Member

@nicoddemus nicoddemus commented Nov 5, 2017

Good point. I'm not sure we can detect it though.

@bskinn
Copy link
Contributor

@bskinn bskinn commented Feb 13, 2019

Late to the party, but I strongly agree with not changing pytest to harmonize the pytest vs python -m pytest behaviors.

The path-modified behavior of python -m is an intrinsic feature of the language, and shimming a tool to paper over it is a bad idea for lots of reasons, not least of which is that it would make pytest inconsistent with other tooling that has not been likewise shimmed.

More-prominent advertising/education of the python -m path-modification behavior is the best approach, IMO, b/c it (1) addresses the root cause, and (2) is equally applicable across the Python ecosystem.

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

Successfully merging a pull request may close this issue.

None yet
9 participants