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

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

Open
nyh opened this issue Mar 10, 2022 · 7 comments
Open

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

nyh opened this issue Mar 10, 2022 · 7 comments
Labels
topic: config related to config handling, argument parsing and config file

Comments

@nyh
Copy link

nyh commented Mar 10, 2022

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
Copy link
Author

nyh commented Mar 10, 2022

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
Copy link
Member

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

@Zac-HD Zac-HD added the topic: config related to config handling, argument parsing and config file label Mar 15, 2022
@pranjalihande
Copy link

pranjalihande commented Mar 21, 2022

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
Copy link
Member

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
Copy link
Member

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

@nyh
Copy link
Author

nyh commented Mar 21, 2022

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

notamonad commented Jul 22, 2022

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

amezin added a commit to ddterm/gnome-shell-extension-ddterm that referenced this issue Oct 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: config related to config handling, argument parsing and config file
Projects
None yet
Development

No branches or pull requests

6 participants