-
Notifications
You must be signed in to change notification settings - Fork 408
tox.ini: use py.test directly instead of python setup.py test #294
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
tox.ini: use py.test directly instead of python setup.py test #294
Conversation
cd41587
to
ec3eda6
Compare
Just rebased this on top of 260cb49 from msabramo/tox.ini_use_py.test_directly_instead_of_python_setup.py_test_2 (i.e.: PR #293). |
deps = | ||
-rdev-requirements.txt | ||
unittest2 | ||
commands = py.test {posargs:{[testenv]pytest_options}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this mean that it will either only use arguments passed at the command line OR what used to be configured for pytest by default?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AFAICT, it will use your defaults if no options are specified on the command-line. In:
{posargs:{[testenv]pytest_options}}
This means to use posargs (args after -- on the tox command-line) if present. Otherwise, it defaults to the expression after the colon, which is the value of the pytest_options
key in the testenv
section, and I set that to the defaults that you used to have in the pytest
section.
My test for this was the first example in the PR which shows the output of tox -e py27
. No options were supplied and it looks to be using your defaults, because it's printing just dots and no filenames, which comes from the -q
option, which is
a default that I carried over.
If I missed some case, let me know!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason that I moved the defaults from the pytest
section to a testenv
section might be subtle and not immediately obvious, so I'll explain just in case.
With a pytest
section, AFAIK, that is always applied by pytest
and I know of no way to override it. Where this is really problematic is when you put something like tests/
in there. If you have that and then use the posargs stuff to tell pytest to run a subset of tests, it is additive instead of replacing. IOW, it will run all of the tests in tests/
and then the ones on the command-line. On the contrary, {posargs:-q tests/}
will default to -q tests/
but REPLACE that if options are supplied on the tox command-line after a double-hyphen.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But if you specify any options after the double-hyphen on the tox command-line, those will replace the defaults. So if you specify a subset of tests to run on the command-line, you will also lose the -q
, unless you specify it on the command-line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've pushed some updates to 1.0-alpha that makes this work as you desire. If you want the verbose output from the tests you can just do tox -e py{{version}} -- -v
, you can also do tox -e py{{version}} -- -k test_download
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I'll try to take a look soon. Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If your 1.0-alpha change does what you said, then I guess this can be closed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your changes on 1.0-alpha
kinda sorta do what I want but not quite. Observe this:
$ tox -e py27 -- -v tests/integration/test_repos_release.py
GLOB sdist-make: /Users/marca/dev/git-repos/github3.py/setup.py
py27 inst-nodeps: /Users/marca/dev/git-repos/github3.py/.tox/dist/github3.py-1.0.0b1.zip
py27 runtests: PYTHONHASHSEED='3544317572'
py27 runtests: commands[0] | py.test -v tests/integration/test_repos_release.py
============================================================================= test session starts ==============================================================================
platform darwin -- Python 2.7.6 -- py-1.4.26 -- pytest-2.6.4
collected 724 items
tests/test_events.py ..............
tests/test_gists.py .....
tests/test_git.py .......
tests/test_github.py ...........................
tests/test_issue_authorize_optional_scope.py ..
...
tests/integration/test_repos_release.py ......
...
tests/integration/test_repos_release.py ......
I only wanted to run the tests in that one module, but it ends up running all of tests and then the one module that I actually wanted to run is run twice, because it adds to rather than replaces the defaults.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just remembered another way to deal with this that might be more palatable to you than what I did here with tox variables, which does feel a bit hacky.
Idea is to remove "tests/" from the "addopts" and instead add a "norecursedirs" with a list of directories to ignore. You probably specify tests/ to prevent py.test from traversing into folders you don't care about like virtualenvs or eggs, etc. Instead of hard-coding where py.test looks, rather tell it what to ignore with norecursedirs
. This doesn't have the side effect above. I'll try to send a PR, but that's the basic idea.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See http://pytest.org/latest/customize.html#builtin-configuration-file-options
What was the rationale for including tests/
? Probably to exclude something else I guess. I think we can come up with a way to ignore the stuff that you don't want pytest to run. Ignoring is a more friendly default.
@sigmavirus24: Does 38e0b2f make this more to your liking? |
38e0b2f
to
3713e17
Compare
Rebased on to |
Replace `tests/` in `addopts` with `norecursedirs` Better to filter than to add in a default, so as not to force folks to always run all the tests.
d31ff43
to
95e92f4
Compare
This PR is kind of long and unnecessarily complicated because a lot of what it did was already done on |
This is a similar change to #292, but based on the branch in #293 (brings over
tox.ini
changes from @sigmavirus24 on the1.0-alpha
branch).