Skip to content

goodpractices - needs __init__.py for easy invocation#4616

Closed
dogweather wants to merge 1 commit into
pytest-dev:masterfrom
dogweather:patch-1
Closed

goodpractices - needs __init__.py for easy invocation#4616
dogweather wants to merge 1 commit into
pytest-dev:masterfrom
dogweather:patch-1

Conversation

@dogweather

@dogweather dogweather commented Jan 8, 2019

Copy link
Copy Markdown

The tests folder must be a package in order to simply run pytest and for it to simply work, as the docs imply. Without this, pytest does not add the current directory to PYTHONPATH, and so seems to mysteriously fail.

Thanks for submitting a PR, your contribution is really appreciated!

Here's a quick checklist that should be present in PRs (you can delete this text from the final description, this is
just a guideline):

  • Create a new changelog file in the changelog folder, with a name like <ISSUE NUMBER>.<TYPE>.rst. See changelog/README.rst for details.
  • Target the master branch for bug fixes, documentation updates and trivial changes.
  • Target the features branch for new features and removals/deprecations.
  • Include documentation when adding new features.
  • Include new tests or update existing tests when applicable.

Unless your change is trivial or a small documentation fix (e.g., a typo or reword of a small section) please:

  • Add yourself to AUTHORS in alphabetical order;

The tests folder must be a package in order to simply run `pytest` and for it to simply work, as the docs imply. Without this, pytest does not add the current directory to PYTHONPATH, and so seems to mysteriously fail.

@RonnyPfannschmidt RonnyPfannschmidt left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this example its intentionally left out

@codecov

codecov Bot commented Jan 8, 2019

Copy link
Copy Markdown

Codecov Report

Merging #4616 into master will decrease coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4616      +/-   ##
==========================================
- Coverage   95.75%   95.74%   -0.02%     
==========================================
  Files         111      111              
  Lines       24678    24678              
  Branches     2446     2446              
==========================================
- Hits        23630    23627       -3     
  Misses        740      740              
- Partials      308      311       +3
Flag Coverage Δ
#docs 29.57% <ø> (+0.07%) ⬆️
#doctesting 29.57% <ø> (+0.07%) ⬆️
#linting 29.57% <ø> (+0.07%) ⬆️
#linux 95.57% <ø> (-0.02%) ⬇️
#nobyte 92.37% <ø> (ø) ⬆️
#numpy 93.19% <ø> (ø) ⬆️
#pexpect 42.13% <ø> (ø) ⬆️
#py27 93.77% <ø> (-0.02%) ⬇️
#py34 91.86% <ø> (+0.06%) ⬆️
#py35 91.88% <ø> (+0.06%) ⬆️
#py36 91.9% <ø> (+0.06%) ⬆️
#py37 93.92% <ø> (+0.02%) ⬆️
#trial 93.19% <ø> (ø) ⬆️
#windows 93.92% <ø> (ø) ⬆️
#xdist 93.77% <ø> (-0.02%) ⬇️
Impacted Files Coverage Δ
src/_pytest/cacheprovider.py 95.75% <0%> (-1.42%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a4c426b...a9314a1. Read the comment docs.

@nicoddemus

Copy link
Copy Markdown
Member

Yep, there's an example which uses proper test packages in that page as well.

But thanks anyway for the PR, we appreciate it!

@nicoddemus nicoddemus closed this Jan 8, 2019
@dogweather

dogweather commented Jan 9, 2019

Copy link
Copy Markdown
Author

@nicoddemus Thanks!

Just so you know, I spent some time choosing the layout that best applied to my case. The text accompanying the version with __init__.py led me away from using that one: "If you need to have test modules with the same name..." I don't, in my case --- so I opted for the simpler choice.

When $ pytest failed with a module not found error, I tried in vain to fix my layout and naming. $ nosetest ran fine, so it was very confusing. After 20 minutes or so I found a half-related SO question that led me to fixing it. It was very frustrating.

The intent isn't to give a broken recipe, is it? Or, if that version of the layout is legit, can I update this PR to add a working pytest invocation for it? (I.e., it'll need to add the current dir to PYTHONPATH, correct?)

@RonnyPfannschmidt

Copy link
Copy Markdown
Member

@dogweather the main reason is that adding a __init__.py will break testing against installed or develop installed versions, that may intentionally not match the workingtree

the general suggestion is to use a installed version, or a editable installed version of the package to test against (

@nicoddemus

Copy link
Copy Markdown
Member

@dogweather thanks for the explanation. You hit a wall that is very common, because it is recommended in general to test against an installed version of your library or an editable install (pip install -e .). Both require a setup.py file though.

For the cases where the user just has a small module and no setup.py file yet, one can use python -m pytest instead of pytest: Python semantics dictate that when python is invoked, the current working directory is always added to the PYTHONPATH. When you invoke pytest, what is being executed is a standalone executable created by setuptools which explicitly doesn't add any directories to PYTHONPATH.

Perhaps we can add the python -m pytest invocation as a note for users without a setup.py? The python -m pytest invocation is not as known as it should.

@nicoddemus

Copy link
Copy Markdown
Member

More information: https://docs.pytest.org/en/latest/pythonpath.html

@nicoddemus

Copy link
Copy Markdown
Member

Hmmm we should probably also add in goodpractices.rst a link to pythonpath.rst... 🤔

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.

3 participants