Review comments incorporated for issue #11282

This commit is contained in:
Sharad Nair 2023-11-08 01:48:57 +05:30
parent bf3f655437
commit 96915746ad
4 changed files with 51 additions and 17 deletions

View File

@ -0,0 +1,11 @@
Sanitized the default value for configuration options.
Previously if a default was not supplied during :meth:`parser.addini <pytest.Parser.addini>` and the option value was not defined, then a call to :func:`config.getini <pytest.Config.getini>` to get the config option value either returned an *empty list* or an *empty string* depending on whether a type is supplied or not respectively, which is clearly incorrect. Also, ``None`` was not honored as ``default`` even if ``default=None`` was used explicitly while defining the option.
Now the behavior is as follows:
* If a default value is NOT set but the type for the option is provided, then a type specific default will be returned. For example ``type=bool`` will return ``False``, ``type=str`` will return ``""``, etc.
* If a default value of ``None`` is supplied, 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

@ -1,3 +0,0 @@
Fixed returning "None" as the default value for a config option when "None" is explicitly
provided as the default while creating the config option. If no default value is provided
at the time of creating the config option, then a default based on the option type is returned

View File

@ -1495,6 +1495,27 @@ class Config:
def getini(self, name: str): def getini(self, name: str):
"""Return configuration value from an :ref:`ini file <configfiles>`. """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 If the specified name hasn't been registered through a prior
:func:`parser.addini <pytest.Parser.addini>` call (usually from a :func:`parser.addini <pytest.Parser.addini>` call (usually from a
plugin), a ValueError is raised. plugin), a ValueError is raised.

View File

@ -27,12 +27,12 @@ from _pytest.deprecated import check_ispytest
FILE_OR_DIR = "file_or_dir" FILE_OR_DIR = "file_or_dir"
class Notset: class NotSet:
def __repr__(self) -> str: def __repr__(self) -> str:
return "<notset>" return "<notset>"
notset = Notset() NOT_SET = NotSet()
@final @final
@ -184,7 +184,7 @@ class Parser:
type: Optional[ type: Optional[
Literal["string", "paths", "pathlist", "args", "linelist", "bool"] Literal["string", "paths", "pathlist", "args", "linelist", "bool"]
] = None, ] = None,
default: Any = notset, default: Any = NOT_SET,
) -> None: ) -> None:
"""Register an ini-file option. """Register an ini-file option.
@ -211,21 +211,26 @@ class Parser:
:py:func:`config.getini(name) <pytest.Config.getini>`. :py:func:`config.getini(name) <pytest.Config.getini>`.
""" """
assert type in (None, "string", "paths", "pathlist", "args", "linelist", "bool") assert type in (None, "string", "paths", "pathlist", "args", "linelist", "bool")
if default is notset: if default is NOT_SET:
if type is None: default = self._get_ini_default_for_type(type)
default = ""
else:
if type in ["paths", "pathlist", "args", "linelist"]:
default = []
elif type == "bool":
default = False
else:
# for string type
default = ""
self._inidict[name] = (help, type, default) self._inidict[name] = (help, type, default)
self._ininames.append(name) self._ininames.append(name)
def _get_ini_default_for_type(self, type) -> Any:
default = Any
if type is None:
default = ""
else:
if type in ["paths", "pathlist", "args", "linelist"]:
default = []
elif type == "bool":
default = False
else:
# for string type
default = ""
return default
class ArgumentError(Exception): class ArgumentError(Exception):
"""Raised if an Argument instance is created with invalid or """Raised if an Argument instance is created with invalid or