Path in pytest options distracts pytest.ini discovery #1435

Closed
matthiasha opened this Issue Mar 4, 2016 · 17 comments

Comments

Projects
None yet
4 participants
@matthiasha
Contributor

matthiasha commented Mar 4, 2016

Please find the minimal source code example for reproducing in Archive.zip.

Without CLI options, the pytest.ini is found (first assertion in the test passes):

❯❯❯ py.test                             ⏎
================================= test session starts =================================
platform linux2 -- Python 2.7.9, pytest-2.9.0, py-1.4.31, pluggy-0.3.1
rootdir: /Users/mzhafn/tmp/pytest-new, inifile: pytest.ini
collected 1 items

test_test.py F

====================================== FAILURES =======================================
______________________________________ test_test ______________________________________

    def test_test():
        assert pytest.config.option.pytest_ini_option == 'set_value'
>       assert pytest.config.option.cli_option == '../../test.log'
E       assert 'unset' == '../../test.log'
E         - unset
E         + ../../test.log

test_test.py:6: AssertionError
============================== 1 failed in 0.04 seconds ===============================

With a path that is part of a custom py.test CLI option, pytest.ini discovery fails:

❯❯❯ py.test --cli-option ../../test.log
================================= test session starts =================================
platform linux2 -- Python 2.7.9, pytest-2.9.0, py-1.4.31, pluggy-0.3.1
rootdir: /Users/mzhafn, inifile:
collected 1 items

test_test.py F

====================================== FAILURES =======================================
______________________________________ test_test ______________________________________

    def test_test():
>       assert pytest.config.option.pytest_ini_option == 'set_value'
E       assert 'unset' == 'set_value'
E         - unset
E         + set_value

test_test.py:5: AssertionError
============================== 1 failed in 0.04 seconds ===============================

platform linux2 -- Python 2.7.9, pytest-2.9.0, py-1.4.31, pluggy-0.3.1
Linux mzhafn-mb-ubuntu 3.19.0-43-generic #49-Ubuntu SMP Sun Dec 27 19:43:07 UTC 2015 x86_64 x86_64 x86_64 GNU/Linux
Distributor ID: Ubuntu
Description: Ubuntu 15.04
Release: 15.04
Codename: vivid

check-manifest (0.31)
devpi-client (2.1.0)
devpi-common (2.0.8)
pip (7.1.2)
pkginfo (1.2.1)
pluggy (0.3.1)
py (1.4.31)
pytest (2.9.0)
requests (2.9.1)
setuptools (20.2.2)
tox (2.3.1)
virtualenv (14.0.6)

@nicoddemus

This comment has been minimized.

Show comment
Hide comment
@nicoddemus

nicoddemus Mar 5, 2016

Member

(Just removed the regression and backward compatibility labels because I see the same failure in 2.8.7)

Member

nicoddemus commented Mar 5, 2016

(Just removed the regression and backward compatibility labels because I see the same failure in 2.8.7)

@matthiasha

This comment has been minimized.

Show comment
Hide comment
@matthiasha

matthiasha Mar 5, 2016

Contributor

We are fine atm with 2.6.4

Bruno Oliveira notifications@github.com schrieb am Sa., 5. März 2016,
20:13:

(Just removed the regression label because I see the same failure in
2.8.7)


Reply to this email directly or view it on GitHub
#1435 (comment).

Contributor

matthiasha commented Mar 5, 2016

We are fine atm with 2.6.4

Bruno Oliveira notifications@github.com schrieb am Sa., 5. März 2016,
20:13:

(Just removed the regression label because I see the same failure in
2.8.7)


Reply to this email directly or view it on GitHub
#1435 (comment).

@nicoddemus

This comment has been minimized.

Show comment
Hide comment
@nicoddemus

nicoddemus Mar 5, 2016

Member

Oh so in 2.6.4 you don't have this problem?

Member

nicoddemus commented Mar 5, 2016

Oh so in 2.6.4 you don't have this problem?

@matthiasha

This comment has been minimized.

Show comment
Hide comment
@matthiasha

matthiasha Mar 5, 2016

Contributor

No, but I haven't investigated on 2.6.4. It's our production system and it
is running fine.

IMO it is the determine_root (or so) function of the pytest config class,
which gets a parameter "args" passed in, which are all CLI arguments
instead of just those which point to the tests to be executed.

Sorry, just brain dumping into my phone.

Bruno Oliveira notifications@github.com schrieb am Sa., 5. März 2016,
21:45:

Oh so in 2.6.4 you don't have this problem?


Reply to this email directly or view it on GitHub
#1435 (comment).

Contributor

matthiasha commented Mar 5, 2016

No, but I haven't investigated on 2.6.4. It's our production system and it
is running fine.

IMO it is the determine_root (or so) function of the pytest config class,
which gets a parameter "args" passed in, which are all CLI arguments
instead of just those which point to the tests to be executed.

Sorry, just brain dumping into my phone.

Bruno Oliveira notifications@github.com schrieb am Sa., 5. März 2016,
21:45:

Oh so in 2.6.4 you don't have this problem?


Reply to this email directly or view it on GitHub
#1435 (comment).

@matthiasha

This comment has been minimized.

Show comment
Hide comment
@matthiasha

matthiasha Mar 7, 2016

Contributor

So it is the function determine_setup. The parameter args should not include any custom pytest options, but rather only the path(s) to the test(s).

Contributor

matthiasha commented Mar 7, 2016

So it is the function determine_setup. The parameter args should not include any custom pytest options, but rather only the path(s) to the test(s).

@diegorusso

This comment has been minimized.

Show comment
Hide comment
@diegorusso

diegorusso Jul 23, 2016

Contributor

I've just tested pull request #1621 and it fixes this issues.

Contributor

diegorusso commented Jul 23, 2016

I've just tested pull request #1621 and it fixes this issues.

@nicoddemus

This comment has been minimized.

Show comment
Hide comment
@nicoddemus

nicoddemus Jul 23, 2016

Member

@diegorusso thanks for testing this! 😁

Member

nicoddemus commented Jul 23, 2016

@diegorusso thanks for testing this! 😁

@nicoddemus

This comment has been minimized.

Show comment
Hide comment
@nicoddemus

nicoddemus Aug 6, 2016

Member

Resolved in #1621

Member

nicoddemus commented Aug 6, 2016

Resolved in #1621

@nicoddemus

This comment has been minimized.

Show comment
Hide comment
@nicoddemus

nicoddemus Aug 9, 2016

Member

Reopening as asked in #1801

Member

nicoddemus commented Aug 9, 2016

Reopening as asked in #1801

@nicoddemus nicoddemus reopened this Aug 9, 2016

@nicoddemus

This comment has been minimized.

Show comment
Hide comment
@nicoddemus

nicoddemus Aug 9, 2016

Member

@matthiasha you couldn't reopen it, given that you were the OP? Just curious. 😁

Member

nicoddemus commented Aug 9, 2016

@matthiasha you couldn't reopen it, given that you were the OP? Just curious. 😁

@matthiasha

This comment has been minimized.

Show comment
Hide comment
@matthiasha

matthiasha Aug 9, 2016

Contributor

According to stackoverflow: "you cannot re-open your own issues if a repo collaborator closed them"

Contributor

matthiasha commented Aug 9, 2016

According to stackoverflow: "you cannot re-open your own issues if a repo collaborator closed them"

@matthiasha

This comment has been minimized.

Show comment
Hide comment
@matthiasha

matthiasha Aug 9, 2016

Contributor

Playing around with the current pytest sources after merging #1621, I figured out that my example
py.test --cli-option ../../test.log

does still not work if you do touch ../../test.log before executing pytest.

Sorry for finding this out only after the merge :(

Contributor

matthiasha commented Aug 9, 2016

Playing around with the current pytest sources after merging #1621, I figured out that my example
py.test --cli-option ../../test.log

does still not work if you do touch ../../test.log before executing pytest.

Sorry for finding this out only after the merge :(

@RonnyPfannschmidt

This comment has been minimized.

Show comment
Hide comment
@RonnyPfannschmidt

RonnyPfannschmidt Aug 9, 2016

Member

lets add xfailing test for that one

Member

RonnyPfannschmidt commented Aug 9, 2016

lets add xfailing test for that one

@matthiasha

This comment has been minimized.

Show comment
Hide comment
@matthiasha

matthiasha Aug 10, 2016

Contributor

Hmm just figured out pytest==2.6.4 also cannot handle this:

touch ../../test.log
py.test --cli-option ../../test.log

But both pytest==2.6.4 and the current pytest sources can handle

touch ../../test.log
py.test --cli-option ../../test.log .

as well as:

touch ../../test.log
py.test --cli-option ../../test.log test_test.py
Contributor

matthiasha commented Aug 10, 2016

Hmm just figured out pytest==2.6.4 also cannot handle this:

touch ../../test.log
py.test --cli-option ../../test.log

But both pytest==2.6.4 and the current pytest sources can handle

touch ../../test.log
py.test --cli-option ../../test.log .

as well as:

touch ../../test.log
py.test --cli-option ../../test.log test_test.py
@matthiasha

This comment has been minimized.

Show comment
Hide comment
@matthiasha

matthiasha Aug 16, 2016

Contributor

So I think it's easiest to handle this as a known issue. As long as a test path (a folder or a file) is given, this issue does not seem to be triggered. So maybe that should go into the documentation?

Contributor

matthiasha commented Aug 16, 2016

So I think it's easiest to handle this as a known issue. As long as a test path (a folder or a file) is given, this issue does not seem to be triggered. So maybe that should go into the documentation?

@nicoddemus

This comment has been minimized.

Show comment
Hide comment
@nicoddemus

nicoddemus Aug 16, 2016

Member

@matthiasha good point. It is unfortunately ambiguous as it is. I guess if #1642 is implemented at least we could document that as a workaround that would allow passing paths to command-line options which wouldn't be considered to determine the rootdir, because the rootdir as being explicitly configured by the user in that scenario.

Either way documenting the current behavior seems to be a good option. Would you care to open a PR? 😁

Member

nicoddemus commented Aug 16, 2016

@matthiasha good point. It is unfortunately ambiguous as it is. I guess if #1642 is implemented at least we could document that as a workaround that would allow passing paths to command-line options which wouldn't be considered to determine the rootdir, because the rootdir as being explicitly configured by the user in that scenario.

Either way documenting the current behavior seems to be a good option. Would you care to open a PR? 😁

matthiasha added a commit to matthiasha/pytest that referenced this issue Aug 19, 2016

matthiasha added a commit to matthiasha/pytest that referenced this issue Aug 19, 2016

@matthiasha

This comment has been minimized.

Show comment
Hide comment
@matthiasha

matthiasha Aug 19, 2016

Contributor

Done, #1819 . That's my first PR ever BTW, hope it's quite OK :)

Contributor

matthiasha commented Aug 19, 2016

Done, #1819 . That's my first PR ever BTW, hope it's quite OK :)

@pyup-bot pyup-bot referenced this issue in gasparka/pyha_demo_project Dec 18, 2017

Closed

Pin pytest to latest version 3.3.1 #6

@Emantor Emantor referenced this issue in labgrid-project/labgrid Feb 15, 2018

Closed

--lg-log affects pytest's rootdir #182

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment