Skip to content

Command-line arguments that are files which exist cause conftest.py not to be loaded from the usual location #436

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

Closed
pytestbot opened this issue Jan 31, 2014 · 10 comments
Labels
type: bug problem that needs to be addressed

Comments

@pytestbot
Copy link
Contributor

Originally reported by: Marc Abramowitz (BitBucket: msabramo, GitHub: msabramo)


Maybe or maybe not related to https://bitbucket.org/hpk42/pytest/issue/416/option-added-in-conftestpy-not-available, which uses a very similar example ("runslow) in fact.

$ pip freeze | grep pytest
pytest==2.5.2

$ tree
.
└── tests
    ├── conftest.py
    └── test_stuff.py

1 directory, 2 files

$ cat tests/conftest.py
import pytest


def pytest_addoption(parser):
    parser.addoption("--runslow", action="store_true",
                     help="run slow tests")
    print("*** Added --runslow option")


def pytest_runtest_setup(item):
    print("*** value of --runslow option: %r" % item.config.getoption("--runslow"))

$ cat tests/test_stuff.py
def test_stuff():
    assert 1 + 1 == 2

Now...

The first time that I run py.test with --junitxml junit.xml and -k test_stuff and there is no junit.xml file yet, it works fine:

$ py.test --junitxml junit.xml --runslow -s -k test_stuff
*** Added --runslow option
============================================================================= test session starts ==============================================================================
platform darwin -- Python 2.7.6 -- py-1.4.20 -- pytest-2.5.2
collected 1 items

tests/test_stuff.py *** value of --runslow option: True
.

------------------------------------------------------------ generated xml file: /Users/marca/pytest-bug/junit.xml -------------------------------------------------------------
=========================================================================== 1 passed in 0.01 seconds ===========================================================================

Now that junit.xml exists:

$ ls -l junit.xml
-rw-r--r--  1 marca  staff   215B Jan 30 16:06 junit.xml

A second invocation fails:

$ py.test --junitxml junit.xml --runslow -s -k test_stuff
usage: py.test [options] [file_or_dir] [file_or_dir] [...]
py.test: error: unrecognized arguments: --runslow

If I remove --junitxml junit.xml, it doesn't exhibit the problem:

$ py.test --runslow -s -k test_stuff
*** Added --runslow option
============================================================================= test session starts ==============================================================================
platform darwin -- Python 2.7.6 -- py-1.4.20 -- pytest-2.5.2
collected 1 items

tests/test_stuff.py *** value of --runslow option: True
.

=========================================================================== 1 passed in 0.01 seconds ===========================================================================

It also goes away if I specify the tests/ directory:

$ py.test --junitxml junit.xml --runslow -s -k test_stuff tests/
*** Added --runslow option
============================================================================= test session starts ==============================================================================
platform darwin -- Python 2.7.6 -- py-1.4.20 -- pytest-2.5.2
collected 1 items

tests/test_stuff.py *** value of --runslow option: True
.

------------------------------------------------------------ generated xml file: /Users/marca/pytest-bug/junit.xml -------------------------------------------------------------
=========================================================================== 1 passed in 0.01 seconds ===========================================================================

I can't figure out what is going on here...


@pytestbot
Copy link
Contributor Author

Original comment by Marc Abramowitz (BitBucket: msabramo, GitHub: msabramo):


Ah, I just discovered that it's not unique to --junitxml. It also happens with --resultlog:

$ py.test --resultlog result.log --runslow -s -k test_stuff
*** Added --runslow option
============================================================================= test session starts ==============================================================================
platform darwin -- Python 2.7.6 -- pytest-2.4.3.dev2
collected 1 items

tests/test_stuff.py *** value of --runslow option: True
.

=========================================================================== 1 passed in 0.01 seconds ===========================================================================

$ py.test --resultlog result.log --runslow -s -k test_stuff
usage: py.test [options] [file_or_dir] [file_or_dir] [...]
py.test: error: unrecognized arguments: --runslow

So my theory is that when py.test sees a file on the command-line and it exists, it treats it something like a test and then expects the conftest.py to be in the same directory as it.

This is bolstered by the fact that if the resultlog file is in the same directory as conftest.py, then it doesn't happen:

$ ll tests/result.log
-rw-r--r--  1 marca  staff    34B Jan 30 16:32 tests/result.log

$ py.test --resultlog tests/result.log --runslow -s -k test_stuff
*** Added --runslow option
============================================================================= test session starts ==============================================================================
platform darwin -- Python 2.7.6 -- pytest-2.4.3.dev2
collected 1 items

tests/test_stuff.py *** value of --runslow option: True
.

=========================================================================== 1 passed in 0.01 seconds ===========================================================================

@pytestbot
Copy link
Contributor Author

Original comment by Marc Abramowitz (BitBucket: msabramo, GitHub: msabramo):


OK, I created a PR with a possible way to address this:

https://bitbucket.org/hpk42/pytest/pull-request/116/fix-for-issue-436-add-heuristics-to-ignore/diff

I feel like an even better solution is to discover conftest.py files while doing test discovery. I am guessing that this doesn't happen now because it looks for conftest.py files very early in the process before even command-line arguments are parsed (because it can add options, change discovery, etc. presumably). But maybe it can continue to do this and do another pass looking for conftest.py files in the test discovery phase?

@pytestbot
Copy link
Contributor Author

Original comment by Marc Abramowitz (BitBucket: msabramo, GitHub: msabramo):


I have another approach for solving this at:

https://bitbucket.org/hpk42/pytest/pull-request/117/fix-issue-436-2-by-looking-for-tests-dirs/diff

@pytestbot
Copy link
Contributor Author

Original comment by holger krekel (BitBucket: hpk42, GitHub: hpk42):


A few things:

  • py.test tries to early-discover conftest.py files in order to add project-specific command line options
  • because it doesn't know project-specific command line options on startup it cannot properly parse the command line options but instead uses a heuristic to find out where to look for conftest file (it's searching for an "anchor")
  • if an option is specified as "--OPT ARG" then "ARG" is checked if it's an anchor and pytest checks if there is a conftest.py file (including looking for testpath/test*/conftest.py)
  • it's always possible to circumvent problems by specifying --OPT=ARG because this will not be considered as an anchor because of the leading --.
  • conftest.py files are additionally discovered during collection independently from the startup heuristic, so there is no problem wrt to runtest_setup.

One of the reasons for the heuristics is to allow running pytest like this: py.test ../../some/path/to/tests or py.test src/tests and have it properly discover conftest files and run tests. If we just took CWD and looked from there this would not discover conftest.py specified command line options.

Maybe implementing #430 might help in combination with looking for paths that look like they are pointing somewhere else (../). If the latter, we would first check that other destination for the presence of an ini file and use its conftestscan paramter to discover conftest files. But it remains an heuristic that can go wrong.

We can give up on the ability of specifying ../* and project-specific options and always use CWD as an anchor to look for ini and conftest files and implement #430 to allow for custom options using a more exact pattern for finding conftest files relative to the ini file. This scheme has the advantage that it is a predictable (independent from option args) way of checking things.

Note that for this issue, implementing #430 and specifying conftestscan = ... pointing to the conftest file would have prevented the surprise as long as the junitxml file is created below the directory containing the ini file.

I tend to think it's better to introduce fully predictable behaviour rather than trying to enhance the heuristics. Might break some people's test invocation scripts or tox.ini's so maybe better a change for 2.6?

@pytestbot
Copy link
Contributor Author

Original comment by Marc Abramowitz (BitBucket: msabramo, GitHub: msabramo):


@hpk42: Thanks for the detailed explanation!

it's always possible to circumvent problems by specifying --OPT=ARG because this
will not be considered as an anchor because of the leading --.

Ah, I didn't know this, but now that I do, that made me think of another possible way to address the problem...

What if when py.test detects an "anchor" file on the command-line, it outputs a warning along the lines of:

"Found file argument 'junit.xml' in the command-line arguments, preceded by the argument '--junitxml'. Going to look for conftest.py files relative to the directory containing 'junit.xml'. If you don't want this to occur, then you may want to specify '--junitxml=junit.xml' (note equal sign between arguments) instead of '--junitxml junit.xml' (note space between arguments)."

At least that way, folks know what is happening (it's not obvious right now) and you give them a chance to change their habits in a way that prevents problems.

Maybe that warning will be annoying when people specify paths like "../" and what not. But maybe it's worth it, I don't know.

It also occurs to me that in any legitimate case I can think of where one would use "../" or whatever to point py.test to tests in another directory, the argument would seem to always have a slash in it. Well, maybe except for "." and "..". But generally specifying a file in the current directory like "junit.xml" or "result.log" would be a strange thing to do. Would it always be either a directory or a .py file if someone was trying to get py.tests to run tests somewhere else?

Another possible way to address this is to change the interface (so for 2.6) so that the user cannot specify a path by itself to get py.test to run tests there. Instead they would have to supply an explicit option -- e.g.: -w ../ (I picked -w because nosetests has a -w option for "working directory" which might be analogous to this and familiar to some people). Then the code only needs to do the anchor thing if it sees "-w file" and things become quite predictable.

@pytestbot
Copy link
Contributor Author

Original comment by Marc Abramowitz (BitBucket: msabramo, GitHub: msabramo):


A stab at a warning:

$ hg diff
diff --git a/_pytest/config.py b/_pytest/config.py
--- a/_pytest/config.py
+++ b/_pytest/config.py
@@ -484,6 +484,14 @@
                 continue
             anchor = current.join(arg, abs=1)
             if exists(anchor): # we found some file object
+                sys.stderr.write("-" * 78 + "\n")
+                sys.stderr.write(
+                    "Found file in args: %r; Looking for conftest.py\n"
+                    "relative to that file...\n" % arg)
+                sys.stderr.write(
+                    "If this file was a value for a previous argument, you\n"
+                    "might want to separate it with an = rather than a space.\n")
+                sys.stderr.write("-" * 78 + "\n")
                 self._try_load_conftest(anchor)
                 foundanchor = True
         if not foundanchor:

@pytestbot
Copy link
Contributor Author

Original comment by Marc Abramowitz (BitBucket: msabramo, GitHub: msabramo):


And here's another change that I think would be good to make -- make it so that the help shows the arguments with the equals sign instead of the space, since that is less prone to error:

diff --git a/_pytest/config.py b/_pytest/config.py
--- a/_pytest/config.py
+++ b/_pytest/config.py
@@ -442,7 +442,7 @@
             if len(option) == 2 or option[2] == ' ':
                 return_list.append(option)
             if option[2:] == short_long.get(option.replace('-', '')):
-                return_list.append(option)
+                return_list.append(option.replace(' ', '='))
         action._formatted_action_invocation = ', '.join(return_list)
         return action._formatted_action_invocation

Looks like this:

$ py.test -h | egrep '=path'
  --junit-xml=path      create junit-xml style report file at given path.
  --result-log=path     path for machine-readable result log.
  --ignore=path         ignore path during collection (multi-allowed).
  --genscript=path      create standalone pytest script at given target path.

@pytestbot
Copy link
Contributor Author

Original comment by holger krekel (BitBucket: hpk42, GitHub: hpk42):


This got lost a bit (along with the PRs) due to my vacation. On the plus side i took a fresh look and think we could at least cover your original cases by only considering .py or no extension as a possible anchor.

Modifying the help to show "=" is ok i think, helps to reduce the principle problem.

I also agree with adding a warning (could be done by "config.warn" on trunk) but it should only be given if after the actual command line parsing took place and either failed and would provide a different anchor.

As it stands, don't want to otherwise further the heuristics and rather go for #430. Therefore i am bound to decline your Pull Request #116 and 117 but would be very happy for a new one implementing the above suggestions (also only one of them to get started).

@pytestbot
Copy link
Contributor Author

Original comment by Marc Abramowitz (BitBucket: msabramo, GitHub: msabramo):


@hpk42: OK, here's a PR to modify help to show "=": pull request #130

@pytestbot
Copy link
Contributor Author

Original comment by holger krekel (BitBucket: hpk42, GitHub: hpk42):


fix issue436: improved finding of initial conftest files from command
line arguments by using the result of parse_known_args rather than
the previous flaky heuristics. Thanks Marc Abramowitz for tests
and initial fixing approaches in this area.

@pytestbot pytestbot added the type: bug problem that needs to be addressed label Jun 15, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug problem that needs to be addressed
Projects
None yet
Development

No branches or pull requests

1 participant