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

Fixes #11282

Author: SharadNair7

changelog/11282.breaking.rst

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

src/_pytest/config/__init__.py

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

src/_pytest/config/argparsing.py

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

testing/test_config.py

@@ -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
@@ -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
@@ -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: "