Sanitize ini-options default handling #11282 (#11594)

Fixes #11282
This commit is contained in:
Sharad Nair 2023-11-11 21:38:18 +05:30 committed by GitHub
parent 6fe43912be
commit 7c7bdf4574
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 126 additions and 6 deletions

View File

@ -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.

View File

@ -1495,6 +1495,27 @@ class Config:
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.
@ -1521,11 +1542,7 @@ class Config:
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.

View File

@ -27,6 +27,14 @@ from _pytest.deprecated import check_ispytest
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.
@ -176,7 +184,7 @@ class Parser:
type: Optional[
Literal["string", "paths", "pathlist", "args", "linelist", "bool"]
] = None,
default: Any = None,
default: Any = NOT_SET,
) -> None:
"""Register an ini-file option.
@ -203,10 +211,30 @@ class Parser:
: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."""

View File

@ -5,6 +5,7 @@ import re
import sys
import textwrap
from pathlib import Path
from typing import Any
from typing import Dict
from typing import List
from typing import Sequence
@ -21,6 +22,7 @@ from _pytest.config import Config
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
@ -857,6 +859,68 @@ class TestConfigAPI:
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: "