pytest-dev / pytest

The pytest framework makes it easy to write small tests, yet scales to support complex functional testing
https://pytest.org
MIT License
11.98k stars 2.66k forks source link

Config-file discovery gets confused by custom command-line arguments #9749

Open nyh opened 2 years ago

nyh commented 2 years ago

pytest looks in the tests' directory, or one of its parents directory, for a configuration file such as pytest.ini or tox.ini and uses the first one it finds. But how does pytest know which directory is the test directory? The relevant logic is in _pytest/config/findpaths.py, starting in determine_setup(). So for example if I run

pytest /some/directory/my/test.py

That logic determines that the test directory is "/some/directory/my". So far so good.

The problem starts when the user adds custom pytest arguments in conftest.py. For example, my project has in conftest.py:

def pytest_addoption(parser):
    parser.addoption('--scylla-path', action='store', default='',
        help='Path to the scylla excutable the tests are running against')

And people start a test with

pytest --scylla-path /some/path /some/directory/my/test.py

The problem now is that the logic in _pytest/config/findpaths.py looks at the command-line arguments, skips (in get_dirs_from_args()) the argument starting with "-" (in this example, --scylla-path) but then does not skip its parameter - /some/path. It then looks for the configuration file in /some/path, which is wrong - and in my case led pytest to find a broken configuration file in that directory and using it.

The ideal fix would be for get_dirs_from_args() to be called after the command line is parsed and the non-positional arguments are removed. I don't know if we can do this, or we have a chicken and egg problem of what gets read first.

Another possible fix is perhaps to first just look for a conftest.py (as we already do in _pytest/config/__init__.py), and if we find one (in my example, it's in in /some/directory/my, not /some/path) also look for pytest.ini in the same directory - NOT look again at all the directories in command line. In other words, it doesn't make too much sense (I think) to pick up conftest.py from one directory, but tox.ini from a different one, and since conftest.py is more important and more pytest-specific, it should be discovered first.

By the way, there is a workaround to solving my problem without fixing pytest at all - instead of running

pytest --scylla-path /some/path /some/directory/my/test.py

The user just needs to run

pytest --scylla-path=/some/path /some/directory/my/test.py

With an equals sign instead of a space. This works well because now the findpaths.py code skips the entire option, not just half of it. But I still think this needs a better fix, because a user might not remember to use an equals sign instead of a space, and if they do use a space, the resulting error message is very unhelpful (I got strange warnings coming from the definitions in a wrong tox.ini file, but without telling me which ini file this is coming from, or why this ini file was chosen).

nyh commented 2 years ago

I just realised that #8846 reported more-or-less the same issue. @nicoddemus closed that issue saying that "Unfortunately we don't have a solution to this atm."

However, above I proposed one solution - I'm not sure it will work in every case but it would in mine: When finding a conftest.py, look for other files like pytest.ini in that directory and don't look at the command line again.

I would like to propose another "solution": forbid the "--arg val" syntax. If "--arg val" didn't work and users were forced to use "--arg=val", we wouldn't have this problem. But this solution can cause backward compatibility problems :-(

In any case, I think this is a real pytest bug, that is very frustrating to debug because the user can get errors from spurious configuration files without even knowing where these files are (I ended up adding printouts to pytest's internals to understand what's going on) - so I don't think that this issue or #8846 should be just closed even if we don't have an immediate solution.

RonnyPfannschmidt commented 2 years ago

it indeed is a real bug, however disallowing the syntax without the equal sign is unfortunately going to be a pretty painful breaking change thats likely not sitting well with a lot of users that dont hit the issue as it would break their automation/scripts

if we ever get around refining config initialization, we can certainly add a certain robustness around conftests, options added and early initialization

pranjalihande commented 2 years ago

Below is my code which was working perfectly fine till I upgraded pytest to version 7.1.0

pytest.main(self.pytest_args + ["-k", tc_cfg.name, "--aq-cfg", tc_cfg.aq_cfg])

and in conftest, I have below code:

def pytest_addoption(parser):
    parser.addoption("--aq-cfg", required=False)

This was working fine for earlier pytest versions. With latest one(pytest 7.1.0) its failing with below error:

ERROR: usage: aqueduct [options] [file_or_dir] [file_or_dir] [...]
aqueduct: error: unrecognized arguments: --aq-cfg all_envs
  inifile: /home/aquser/dev/aqueduct/src/cb/test/tools/aqueduct/pytest.ini
  rootdir: /home/aquser/dev/aqueduct/src/cb/test/tools/aqueduct

This is occurring because latest change added for -k syntax: https://docs.pytest.org/en/stable/changelog.html#pytest-7-1-0-2022-03-13

But if I try to remove --aq-cfg, its failing at parsar and not able to locate file without "--".

Any help on this would be appreciated @nyh @RonnyPfannschmidt

nicoddemus commented 2 years ago

Hi @pranjalihande,

Some things to try:

  1. Join -k with your option: pytest.main(self.pytest_args + ["-k" + tc_cfg.name, "--aq-cfg", tc_cfg.aq_cfg])
  2. Update to 7.1.1, we fixed a conftest discovery problem that might be related.
RonnyPfannschmidt commented 2 years ago

Please try with the latest releases which has a bugfix wrt conftests finding

nyh commented 2 years ago

By the way, the original issue above was about configuration files (pytest.ini at all), not conftest.py as is apparently your problem, so it's probably a different problem. Part of the problem in the original issue is arguably the fact there is separate code to find these two different things.

notamonad commented 2 years ago

I am encountering an issue that seemingly relates to this behavior and I wonder if there is a better way of handling it

I am maintaining a team wide internal automation test framework that lets users perform end to end and integration tests for company products. As a result the framework itself ships many tests and utility packages/modules alongside it. Due to its size, it also includes unit tests for these utility packages/modules and is organized as

src/
  device_tests
    pytest.ini
    conftest.py
    load_tests/
       test_1.py
       ...
  package_1
    file.py
    ...
tests/
  test_package_1.py
  ...
pytest.ini
pyproject.toml
setup.py
Dockerfile
...

When users launch device tests they often time supply a CLI argument that provides a path to some required bin file somewhere else in the system, often times in CI these bin files are put in the root directory of this project and as a result this path makes it so that the wrong pytest.ini is picked up. Setting --rootdir to directory of device tests doesn't seem to help, and I couldn't find a way to be able to specify path to configfile

One solution is what @nyh has pointed out is to put "=" sign for the troublesome CLI argument, but this is incredibly implicit Another is to make sure these required input files are placed in a different directory, but again this is implicit behavior that users need to be aware of

Are there perhaps better ways of solving this situation I am not aware of? Or maybe in the future there is a way to specify path to pytest.ini file explicitly?

Apologies if this is off topic, I found this issue when trying to debug this behavior