Skip to content

Conversation

msabramo
Copy link
Contributor

@msabramo msabramo commented Nov 1, 2014

This is a similar change to #292, but based on the branch in #293 (brings over tox.ini changes from @sigmavirus24 on the 1.0-alpha branch).

$ tox -e py27
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-0.9.2.zip
py27 runtests: PYTHONHASHSEED='2339110401'
py27 runtests: commands[0] | py.test -q tests/
..............................................x...............................................................................................................................................................................................................................................................................................................................................................................................................................................................
493 passed, 1 xfailed in 3.79 seconds
___________________________________________________________________________________ summary ____________________________________________________________________________________
  py27: commands succeeded
  congratulations :)

$ tox -e py27 -- --tb=short --pdb --pyargs tests.integration.test_repos_release -k test_download
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-0.9.2.zip
py27 runtests: PYTHONHASHSEED='3359558432'
py27 runtests: commands[0] | py.test --tb=short --pdb --pyargs tests.integration.test_repos_release -k test_download
============================================================================= test session starts ==============================================================================
platform darwin -- Python 2.7.6 -- py-1.4.26 -- pytest-2.6.4
collected 6 items

tests/integration/test_repos_release.py .

=================================================================== 5 tests deselected by '-ktest_download' ====================================================================
==================================================================== 1 passed, 5 deselected in 0.09 seconds ====================================================================
___________________________________________________________________________________ summary ____________________________________________________________________________________
  py27: commands succeeded
  congratulations :)

@msabramo msabramo force-pushed the tox.ini_use_py.test_directly_instead_of_python_setup.py_test_2 branch from cd41587 to ec3eda6 Compare November 1, 2014 22:52
@msabramo
Copy link
Contributor Author

msabramo commented Nov 1, 2014

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}}
Copy link
Owner

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?

Copy link
Contributor Author

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!

Copy link
Contributor Author

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.

Copy link
Contributor Author

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.

Copy link
Owner

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.

Copy link
Contributor Author

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!

Copy link
Contributor Author

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.

Copy link
Contributor Author

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.

Copy link
Contributor Author

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.

Copy link
Contributor Author

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.

@msabramo
Copy link
Contributor Author

msabramo commented Nov 2, 2014

@sigmavirus24: Does 38e0b2f make this more to your liking?

@msabramo msabramo force-pushed the tox.ini_use_py.test_directly_instead_of_python_setup.py_test_2 branch from 38e0b2f to 3713e17 Compare November 2, 2014 04:21
@msabramo
Copy link
Contributor Author

msabramo commented Nov 2, 2014

Rebased on to sigmavirus24/develop (4ea647e).

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.
@msabramo msabramo force-pushed the tox.ini_use_py.test_directly_instead_of_python_setup.py_test_2 branch from d31ff43 to 95e92f4 Compare November 2, 2014 04:29
@msabramo
Copy link
Contributor Author

msabramo commented Nov 2, 2014

This PR is kind of long and unnecessarily complicated because a lot of what it did was already done on sigmavirus24/develop. The change that I am still proposing is quite short and simple, so I'm going to close this PR and refer you to #296 instead.

@msabramo msabramo closed this Nov 2, 2014
@msabramo msabramo deleted the tox.ini_use_py.test_directly_instead_of_python_setup.py_test_2 branch November 2, 2014 04:39
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.

2 participants