pytest.Config.getoption `default` argument not working as expected

[ryancausey]

• a detailed description of the bug or problem you are having
• output of pip list from the virtual environment you are using
• pytest and operating system versions
• minimal example if possible

I'm running into an issue where the default argument to pytest.Config.getoption is not working as expected, or at least not as it is documented.

Reproduction steps

1. define the pytest_generate_tests hook in a conftest.py file.
2. use the metafunc argument to access the pytest.Config object via metafunc.config.
3. try to call getoption on an option that was not passed while specifying a default, E.G. metafunc.config.getoption(name="foobaz", default={})

Exepected behavior

The empty dictionary is returned as it is set as the default.

Actual behavior

None is returned instead.

Possible fix

The issue appears to be the check inside getoption on line 1541:

(Pdb) ll
1529        def getoption(self, name: str, default=notset, skip: bool = False):
1530            """Return command line option value.
1531
1532            :param name: Name of the option.  You may also specify
1533                the literal ``--OPT`` option instead of the "dest" option name.
1534            :param default: Default value if no option of that name exists.
1535            :param skip: If True, raise pytest.skip if option does not exists
1536                or has a None value.
1537            """
1538            name = self._opt2dest.get(name, name)
1539            try:
1540                val = getattr(self.option, name)
1541 ->             if val is None and skip:
1542                    raise AttributeError(name)
1543                return val
1544            except AttributeError as e:
1545                if default is not notset:
1546                    return default
1547                if skip:
1548                    import pytest
1549
1550                    pytest.skip(f"no {name!r} option found")
1551                raise ValueError(f"no option named {name!r}") from e
(Pdb) print(val)
None
(Pdb) print(skip)
False
(Pdb)

Since skip is not passed, it defaults to False which means even if a default is set to something other than notset, that default value will never be returned unless skip is also set to True. I believe the fix is to change line 1541 to:

if val is None:

This will be a breaking change, however, as it would mean not passing default would lead to a ValueError being raised when skip is False.

Minimal reproduction example

# conftest.py
def pytest_generate_tests(metafunc):
    my_cool_option = metafunc.config.getoption(name="foobaz", default={})
    assert my_cool_option is not None

Pip list

Actually it's `poetry show`

$ poetry show
astroid               2.12.10   An abstract syntax tree for Python with inference support.
attrs                 22.1.0    Classes Without Boilerplate
aws-xray-sdk          2.10.0    The AWS X-Ray SDK for Python (the SDK) enables Python developers to record and emit information from within their applications to the AWS X-Ray se...
awscrt                0.14.0    A common runtime for AWS Python projects
backoff               2.1.2     Function decoration for backoff and retry
black                 22.8.0    The uncompromising code formatter.
boto3                 1.26.6    The AWS SDK for Python
botocore              1.29.6    Low-level, data-driven core of boto 3.
certifi               2022.9.24 Python package for providing Mozilla's CA Bundle.
cfgv                  3.3.1     Validate configuration and produce human readable error messages.
charset-normalizer    2.1.1     The Real First Universal Charset Detector. Open, modern and actively maintained alternative to Chardet.
click                 8.1.3     Composable command line interface toolkit
click-log             0.4.0     Logging integration for Click
cloup                 1.0.2     Adds features to Click: option groups, constraints, subcommand sections and help themes.
coverage              6.4.4     Code coverage measurement for Python
dill                  0.3.5.1   serialize all of python
distlib               0.3.6     Distribution utilities
dodgy                 0.2.1     Dodgy: Searches for dodgy looking lines in Python code
exceptiongroup        1.0.4     Backport of PEP 654 (exception groups)
execnet               1.9.0     execnet: rapid multi-Python deployment
faunadb               4.3.1     FaunaDB Python driver
filelock              3.8.0     A platform independent file lock.
flake8                2.3.0     the modular source code checker: pep8, pyflakes and co
flake8-polyfill       1.0.2     Polyfill package for Flake8 plugins
fluctuate             1.0.1     A simple migration utility for Fauna DB.
future                0.18.2    Clean single-source support for Python 3 and 2
h2                    2.6.2     HTTP/2 State-Machine based protocol implementation
hpack                 3.0.0     Pure-Python HPACK header compression
hyper                 0.7.0     HTTP/2 Client for Python
hyperframe            3.2.0     HTTP/2 framing layer for Python
identify              2.5.6     File identification library for Python
idna                  3.4       Internationalized Domain Names in Applications (IDNA)
iniconfig             1.1.1     iniconfig: brain-dead simple config-ini parsing
iso8601               1.0.2     Simple module to parse ISO 8601 dates
isort                 5.10.1    A Python utility / library to sort Python imports.
jmespath              1.0.1     JSON Matching Expressions
lazy-object-proxy     1.7.1     A fast and thorough lazy object proxy.
mccabe                0.6.1     McCabe checker, plugin for flake8
mypy-extensions       0.4.3     Experimental type system extensions for programs checked with the mypy typechecker.
nodeenv               1.7.0     Node.js virtual environment builder
packaging             21.3      Core utilities for Python packages
pathspec              0.10.1    Utility library for gitignore style pattern matching of file paths.
pep8                  1.7.1     Python style guide checker
pep8-naming           0.10.0    Check PEP-8 naming conventions, plugin for flake8
platformdirs          2.5.2     A small Python module for determining appropriate platform-specific dirs, e.g. a "user data dir".
pluggy                1.0.0     plugin and hook calling mechanisms for python
pre-commit            2.20.0    A framework for managing and maintaining multi-language pre-commit hooks.
prospector            1.7.7
py                    1.11.0    library with cross-python path, ini-parsing, io, code, log facilities
pycodestyle           2.8.0     Python style guide checker
pydocstyle            6.1.1     Python docstring style checker
pyflakes              2.5.0     passive checker of Python programs
pylint                2.15.3    python code static checker
pylint-celery         0.3       pylint-celery is a Pylint plugin to aid Pylint in recognising and understandingerrors caused when using the Celery library
pylint-django         2.5.3     A Pylint plugin to help Pylint understand the Django web framework
pylint-flask          0.6       pylint-flask is a Pylint plugin to aid Pylint in recognizing and understanding errors caused when using Flask
pylint-plugin-utils   0.7       Utilities and helpers for writing Pylint plugins
pyparsing             3.0.9     pyparsing module - Classes and methods to define and execute parsing grammars
pytest                7.2.0     pytest: simple powerful testing with Python
pytest-cov            3.0.0     Pytest plugin for measuring coverage.
pytest-env            0.6.2     py.test plugin that allows you to add environment variables.
pytest-forked         1.4.0     run tests in isolated forked subprocesses
pytest-xdist          2.5.0     pytest xdist plugin for distributed testing and loop-on-failing modes
python-dateutil       2.8.2     Extensions to the standard Python datetime module
python-decouple       3.6       Strict separation of settings from code.
PyYAML                6.0       YAML parser and emitter for Python
requests              2.28.1    Python HTTP for Humans.
requirements-detector 0.7       Python tool to find and list requirements of a Python project
s3transfer            0.6.0     An Amazon S3 Transfer Manager
sentry-sdk            1.9.9     Python client for Sentry (https://sentry.io)
setoptconf-tmp        0.3.1     A module for retrieving program settings from various sources in a consistant method.
setuptools            65.5.0    Easily download, build, install, upgrade, and uninstall Python packages
six                   1.16.0    Python 2 and 3 compatibility utilities
snowballstemmer       2.2.0     This package provides 29 stemmers for 28 languages generated from Snowball algorithms.
toml                  0.10.2    Python Library for Tom's Obvious, Minimal Language
tomli                 2.0.1     A lil' TOML parser
tomlkit               0.11.4    Style preserving TOML library
typing-extensions     4.3.0     Backported and Experimental Type Hints for Python 3.7+
urllib3               1.26.12   HTTP library with thread-safe connection pooling, file post, and more.
virtualenv            20.16.5   Virtual Python Environment builder
wrapt                 1.14.1    Module for decorators, wrappers and monkey patching.

pytest and OS versions

$ poetry show pytest
 name         : pytest
 version      : 7.2.0
 description  : pytest: simple powerful testing with Python

dependencies
 - attrs >=19.2.0
 - colorama *
 - exceptiongroup >=1.0.0rc8
 - iniconfig *
 - packaging *
 - pluggy >=0.12,<2.0
 - tomli >=1.0.0

required by
 - pytest-cov >=4.6
 - pytest-env >=2.6.0
 - pytest-forked >=3.10
 - pytest-xdist >=6.2.0

$ lsb_release -a
No LSB modules are available.
Distributor ID: Ubuntu
Description:    Ubuntu 22.04.1 LTS
Release:        22.04
Codename:       jammy

[nicoddemus]

Definitely a bug, and surprising one at that. Not sure we need to consider it a breaking change, given that the current behavior is definitely wrong according to the docs, but I would probably only release a fix in the next minor version to avoid any disruption.

[sus-pe]

Hi, has this been fixed already? May I fix this?

[nicoddemus]

Nobody has worked on it @sus-pe, you are welcome to give it a shot! 👍

[sus-pe]

Nobody has worked on it @sus-pe, you are welcome to give it a shot! 👍

Cool I'm doing it :)

[sus-pe]

Specification of Issue

The definition of "option existence"

Before talking about the issue it is important to well define the term of an option existing vs not existing, as the docs distinguish between the two when defining its expected behaviour.

As we have observed, an option "exists" if the parser has been taught to expect it using parser.addoption (in pytest_addoption hook).
Otherwise, the option is considered as a "non-existent".

Docs vs Observed Behaviour

Below, we present our observation of getoption behaviour under all possible circumstances, partitioned by the option's existence. Each table's cell represents getoption behaviour under a certain combination of getoption's parameters skip, default, and option-existence of the given name parameter.

If we have observed a mismatch between the expected behaviour as documented (to the best of our ability) and how the code actually behaves, we have added such mention (in bold at the relevant cell) by mentioning "doc:" to signal that this is how we understand the documented behaviour, and "code:" to signal that this is the behaviour observed by running the method.

skip means that pytest.skip is invoked.
ValueError means it was raised.
{} means that the default value was returned.
value means that the option's value as specified by the user was returned.
None means that None was returned.

option doesn't exist

getoption	skip=True	skip=False
default={}	doc: Ambiguous if should skip or {} code: {}	{}
default=<NOTSET>	skip	ValueError

option exists and is None (value not provided in command line)

getoption	skip=True	skip=False
default={}	doc: skip code: {}	None
default=<NOTSET>	skip	None

option exists and is not None (value provided by in command line)

getoption	skip=True	skip=False
default={}	value	value
default=<NOTSET>	value	value

Oddity of the default param of getoption

As we understand, the key problem is the ambiguity of how to handle both default and skip params being set.

From our understanding, the default value is only relevant given that the option specified for getoption is non-existent. In this case, the "added value" of the default param is that it allows you to not care if it exists or not, just use the given default value.

However, if an option is non-existent, then it is probably a programming error to ask for that option's value from getoption in the first place, hence it is probably better to skip \ raise to let the user know that something is wrong.

In a case where a user wishes for a default value for his option, we believe it would be best to use parser.addoption's default param, as it is a single-source of authority, whereas each caller of getoption might use it's own default causing chaos.

Therefore, we believe that the default parameter of getoption is better off not existing in the first place. However, we are ignorant of the impact of breaking such interface. And perhaps there are legitimate usecases that we have not thought off, any insight is welcomed.

Possible Resolutions

For the first mismatch, we offer that the documentation will be amended to remove the ambiguity in the non-existent + skip + default being set usecase. In that case, we observed the default being returned, so it could be documented that the default it given priority over skip, or do not allow both being set.

As for the second mismatch, it is documented that the default is only relevant when the option doesn't exist. So in this case, we'd expect a skip. We could amend the implementation to skip.
Also, we could amend the documentation instead, to explicitly let the user know default will be used even when the option does exist but is None, but only if skip is set.
This is very weird and we do not recommend this since the documentation gives the impression that the default parameter is only used when the option doesn't exist. It would also mean that we keep this confusing behaviour that when skip=False you get None and with skip=True you get the default.

Conclusion

We ask that a maintainer will verify our understanding, and let us know how we should proceed with this, as some paths are ambiguous. Thank you.

[nicoddemus]

Thanks @sus-pe for the detailed summary!

TBH I'm not sure that this very confusing API is salvageable. This function has been existed since forever, so it probably has grown organically.

My thoughts:

• skip seems a really weird parameter; trying to get an undefined option should be a matter of explicitly getting an error. The fact that it treats None specially is also confusing (None can be a valid value for an option after all).
• default also seems strange, as you mention, you should define default during addoption to have a single source of truth.

Perhaps we would do better to deprecate getoption altogether, and introduce a new variant (get or get_option) which more sane semantics. I propose:

def get(self, name: str) -> Any:
    """"
    Gets the option named ``name`` (you can also use the user facing variant ``--name``), or raises 
    `OptionNotDefinedError`.
    """"

With that the API is very straightforward for the user to use, and they can catch the OptionNotDefinedError and call pytest.skip if they really want pytest to stop if an option is not defined (which should be really rare).

Let's see what others think.

[sus-pe]

I like your suggestion, raising an indicative exception is great to fail the test and ultimately the CI to catch the bug, unlike the skip which would not.

[nicoddemus]

Pinging @The-Compiler @bluetech @RonnyPfannschmidt @asottile to see what they think.

[RonnyPfannschmidt]

i'd love to replace the combination of options/arguments/ini alltogether, but i havent gotten around actually writing a testable version of the new idea, so we can go for such a api first

[asottile]

Perhaps we would do better to deprecate getoption altogether, and introduce a new variant (get or get_option) which more sane semantics. I propose:

I like this 👍

[nicoddemus]

One thing I realized later, which might be weird/confusing (or not):

Assuming with go with get, we will then have 3 ways to get an option:

1. Access the option as an attribute via config.option.foobar, which when undefined raises AttributeError: .
2. The deprecated config.getoption, with its complicated API.
3. The new config.get("foobar"), which when undefined raises OptionNotDefinedError.

I think 2) is fine as is legacy/deprecated, but does the similarity between 1) and 3) bother anyone? I think it is fine (3 is better as it conveys foobar is not statically defined in option), but wanted to know if this might be a problem/confusing.

[sus-pe]

I see your point @nicoddemus, it is confusing that we'd have two different exceptions to indicate the same problem, would it be better to choose just a single approach? Either choose option (1) or (3) and let everything else be consistent with the chosen approach.

Something that I notice is that both (1) and (3) are dynamic bindings, as config.option.foobar would only be generated at runtime, and config.get("foobar") is also dynamic in nature as I am passing a string to represent a config option. Could we find a static solution? Perhaps a way for the developer to define options in typed way, something like

class MySpecialOptions(PytestOption):
      FOOBAR = ...
      KISHKEBAB = ...

def test_foobar(config):
     config.get(MySpecialOptions.FOOBAR)

The config.get would be defined to accept any PytestOption

def get(self, name: PytestOption) -> Any:
    ...

[nicoddemus]

@sus-pe I would not change attribute access (config.option.foobar) due to backward compatibility -- and probably would suggest to not even issue a deprecation warning, supporting it indefinitely: while it is too magic/dynamic, the semantics are clear, contrary to config.getoption with its confusing parameters/variants.

Taking advantage of introducing a new API to also introduce a more type-checked API is an interesting idea, however:

• It would be much more work than just adding a new config.get variant.
• On the other hand, it might introduce a much more modern option API, something we have wanted for awhile now.

Throwing some quick ideas around to see where we can at.

---

Using many options like you suggest will give some type-safety, however still returning Any is not ideal.

Here is how we define an option and use it:

def pytest_addoption(parser):
    parser.addoption(
        "--runslow", action="store_true", default=False, help="run slow tests"
    )

def pytest_collection_modifyitems(config, items):
    if config.getoption("--runslow"):
        # --runslow given in cli: do not skip slow tests
        return

We could make options a first-class citizen, which would could then be used to both declare them and use them:

RunSlow = pytest.Option[bool](
    "--runslow", 
    action=pytest.OptionAction.StoreTrue, 
    default=False, 
    help="run slow tests",
)

def pytest_register_options(config):  # new hook
    config.register_option(RunSlow)

def pytest_collection_modifyitems(config, items):
    if config.get(RunSlow):
        # --runslow given in cli: do not skip slow tests
        return    

pytest.Option is Generic, so config.get(RunSlowOption) can be made to return the correct type.

---

As I said just throwing some quick ideas, but going this route is is opening a whole can of worms and might require some good discussion before we can agree on an API.

But I personally would like to make it clear that even if we decide on a new API, I would not deprecate the old APIs¹, at least not for a few years.

Footnotes

1. well perhaps issue a deprecation warning for config.getoption if skip and/or default are passed in, but even that is arguable. ↩