Skip to content

Sanitize ini-options default handling #11282 #11594

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

Merged
merged 6 commits into from
Nov 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions changelog/11282.breaking.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
Sanitized the handling of the ``default`` parameter when defining configuration options.

Previously if ``default`` was not supplied for :meth:`parser.addini <pytest.Parser.addini>` and the configuration option value was not defined in a test session, then calls to :func:`config.getini <pytest.Config.getini>` returned an *empty list* or an *empty string* depending on whether ``type`` was supplied or not respectively, which is clearly incorrect. Also, ``None`` was not honored even if ``default=None`` was used explicitly while defining the option.

Now the behavior of :meth:`parser.addini <pytest.Parser.addini>` is as follows:

* If ``default`` is NOT passed but ``type`` is provided, then a type-specific default will be returned. For example ``type=bool`` will return ``False``, ``type=str`` will return ``""``, etc.
* If ``default=None`` is passed and the option is not defined in a test session, then ``None`` will be returned, regardless of the ``type``.
* If neither ``default`` nor ``type`` are provided, assume ``type=str`` and return ``""`` as default (this is as per previous behavior).

The team decided to not introduce a deprecation period for this change, as doing so would be complicated both in terms of communicating this to the community as well as implementing it, and also because the team believes this change should not break existing plugins except in rare cases.
27 changes: 22 additions & 5 deletions src/_pytest/config/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -1495,6 +1495,27 @@ def addinivalue_line(self, name: str, line: str) -> None:
def getini(self, name: str):
"""Return configuration value from an :ref:`ini file <configfiles>`.

If a configuration value is not defined in an
:ref:`ini file <configfiles>`, then the ``default`` value provided while
registering the configuration through
:func:`parser.addini <pytest.Parser.addini>` will be returned.
Please note that you can even provide ``None`` as a valid
default value.

If ``default`` is not provided while registering using
:func:`parser.addini <pytest.Parser.addini>`, then a default value
based on the ``type`` parameter passed to
:func:`parser.addini <pytest.Parser.addini>` will be returned.
The default values based on ``type`` are:
``paths``, ``pathlist``, ``args`` and ``linelist`` : empty list ``[]``
``bool`` : ``False``
``string`` : empty string ``""``

If neither the ``default`` nor the ``type`` parameter is passed
while registering the configuration through
:func:`parser.addini <pytest.Parser.addini>`, then the configuration
is treated as a string and a default empty string '' is returned.

If the specified name hasn't been registered through a prior
:func:`parser.addini <pytest.Parser.addini>` call (usually from a
plugin), a ValueError is raised.
Expand All @@ -1521,11 +1542,7 @@ def _getini(self, name: str):
try:
value = self.inicfg[name]
except KeyError:
if default is not None:
return default
if type is None:
return ""
return []
return default
else:
value = override_value
# Coerce the values based on types.
Expand Down
30 changes: 29 additions & 1 deletion src/_pytest/config/argparsing.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,14 @@
FILE_OR_DIR = "file_or_dir"


class NotSet:
def __repr__(self) -> str:
return "<notset>"


NOT_SET = NotSet()


@final
class Parser:
"""Parser for command line arguments and ini-file values.
Expand Down Expand Up @@ -176,7 +184,7 @@ def addini(
type: Optional[
Literal["string", "paths", "pathlist", "args", "linelist", "bool"]
] = None,
default: Any = None,
default: Any = NOT_SET,
) -> None:
"""Register an ini-file option.

Expand All @@ -203,10 +211,30 @@ def addini(
:py:func:`config.getini(name) <pytest.Config.getini>`.
"""
assert type in (None, "string", "paths", "pathlist", "args", "linelist", "bool")
if default is NOT_SET:
default = get_ini_default_for_type(type)

self._inidict[name] = (help, type, default)
self._ininames.append(name)


def get_ini_default_for_type(
type: Optional[Literal["string", "paths", "pathlist", "args", "linelist", "bool"]]
) -> Any:
"""
Used by addini to get the default value for a given ini-option type, when
default is not supplied.
"""
if type is None:
return ""
elif type in ("paths", "pathlist", "args", "linelist"):
return []
elif type == "bool":
return False
else:
return ""


class ArgumentError(Exception):
"""Raised if an Argument instance is created with invalid or
inconsistent arguments."""
Expand Down
64 changes: 64 additions & 0 deletions testing/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import sys
import textwrap
from pathlib import Path
from typing import Any
from typing import Dict
from typing import List
from typing import Sequence
Expand All @@ -21,6 +22,7 @@
from _pytest.config import ConftestImportFailure
from _pytest.config import ExitCode
from _pytest.config import parse_warning_filter
from _pytest.config.argparsing import get_ini_default_for_type
from _pytest.config.exceptions import UsageError
from _pytest.config.findpaths import determine_setup
from _pytest.config.findpaths import get_common_ancestor
Expand Down Expand Up @@ -857,6 +859,68 @@ def pytest_addoption(parser):
assert len(values) == 2
assert values == ["456", "123"]

def test_addini_default_values(self, pytester: Pytester) -> None:
"""Tests the default values for configuration based on
config type
"""

pytester.makeconftest(
"""
def pytest_addoption(parser):
parser.addini("linelist1", "", type="linelist")
parser.addini("paths1", "", type="paths")
parser.addini("pathlist1", "", type="pathlist")
parser.addini("args1", "", type="args")
parser.addini("bool1", "", type="bool")
parser.addini("string1", "", type="string")
parser.addini("none_1", "", type="linelist", default=None)
parser.addini("none_2", "", default=None)
parser.addini("no_type", "")
"""
)

config = pytester.parseconfig()
# default for linelist, paths, pathlist and args is []
value = config.getini("linelist1")
assert value == []
value = config.getini("paths1")
assert value == []
value = config.getini("pathlist1")
assert value == []
value = config.getini("args1")
assert value == []
# default for bool is False
value = config.getini("bool1")
assert value is False
# default for string is ""
value = config.getini("string1")
assert value == ""
# should return None if None is explicity set as default value
# irrespective of the type argument
value = config.getini("none_1")
assert value is None
value = config.getini("none_2")
assert value is None
# in case no type is provided and no default set
# treat it as string and default value will be ""
value = config.getini("no_type")
assert value == ""

@pytest.mark.parametrize(
"type, expected",
[
pytest.param(None, "", id="None"),
pytest.param("string", "", id="string"),
pytest.param("paths", [], id="paths"),
pytest.param("pathlist", [], id="pathlist"),
pytest.param("args", [], id="args"),
pytest.param("linelist", [], id="linelist"),
pytest.param("bool", False, id="bool"),
],
)
def test_get_ini_default_for_type(self, type: Any, expected: Any) -> None:
assert get_ini_default_for_type(type) == expected

def test_confcutdir_check_isdir(self, pytester: Pytester) -> None:
"""Give an error if --confcutdir is not a valid directory (#2078)"""
exp_match = r"^--confcutdir must be a directory, given: "
Expand Down