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

Path to directory with tests from pytest.ini file is not overloaded by value from command line #1607

Closed
EdgarOstrowski opened this issue Jun 14, 2016 · 6 comments

Comments

@EdgarOstrowski
Copy link
Contributor

If the pytest.ini file has a test directory path set it will always be taken in to account when collecting test file even if a different value has been specified by the user from command line. This can lead to a situation where the same tests will get collected multiple times.

Lets imagine we have the following file structure:

.
├── pytest.ini
└── regression
    └── test_regression.py

pytest.ini file:

[pytest]
addopts = regression

regression/test_regression.py file:

import pytest

@pytest.mark.slow()
def test_one():
    pass

@pytest.mark.fast()
def test_two():
    pass

If we run py.test without a test directory path like so: py.test --collectonly will result in:

collected 2 items 
<Module 'regression/test_regression.py'>
  <Function 'test_one'>
  <Function 'test_two'>

which is fine, but is you specify a test directory things start to go wrong. py.test regression --collectonly will result in:

collected 4 items 
<Module 'regression/test_regression.py'>
  <Function 'test_one'>
  <Function 'test_two'>
<Module 'regression/test_regression.py'>
  <Function 'test_one'>
  <Function 'test_two'>

My venv:

py==1.4.31
pytest==2.9.2
@RonnyPfannschmidt
Copy link
Member

addopts per design is always taken into account, thats the very point of it
that collection takes files twice into account when given twice as argument is already a known issue waiting for a resolution

@EdgarOstrowski
Copy link
Contributor Author

But for other options the behavior is different. For example if we add -m slow to the addopts section in pytest.ini and run pytest --collectonly the only test_one will be collected. But if we run pytest -m fast -collectonly the marker setting for the ini file gets overloaded by value from the command line, so only test_two gets collected. So I would expect the same behavior for the test directory path.

@nicoddemus
Copy link
Member

There's a difference however: py.test expects multiple paths for collecting tests, but only a single -m option. That means that your addopts option from pytest.ini effectively executes py.test like this:

py.test --collect-only regression regression

So it will collect tests twice.

The -m option does not accept multiple values, so the last option overwrites the first:

py.test --collect-only -m slow -m fast

In this case only -m fast is considered, overriding -m slow.

Collecting tests twice has been reported before (#1187), but since it is a change from current behavior (with some uses, albeit rare) and nobody felt strongly about it, we didn't actually work on it. But it seems to me more and more people are surprised by this, so I guess this is something we should change, dropping duplicates by default.

Finally, I wrote a small plugin, pytest-drop-dup-tests, which drops duplicated tests that you can use for now as a workaround.

@EdgarOstrowski
Copy link
Contributor Author

Thanks for the explanation. I'm now fully aware of the problem. Honestly I reported the issue because I couldn't anything about this in the docs. Maybe it would be worth mentioning this in the documentation? If it was well explained then the current behavior could be left. If you would like I could prepare a PR for this.

@nicoddemus
Copy link
Member

Honestly I reported the issue because I couldn't anything about this in the docs.

Sure, thanks.

Maybe it would be worth mentioning this in the documentation? If it was well explained then the current behavior could be left. If you would like I could prepare a PR for this.

I think we should do this only if we decide to not do #1609 after all. We will probably review this next week during the sprint, so no need to work on this now, but thanks for the offer regardless. 😁

I guess we can close this now, unless you have more questions?

@EdgarOstrowski
Copy link
Contributor Author

I done. Go ahead and close the issue. Good luck with the upcoming sprint 👍

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

No branches or pull requests

4 participants