Merge pull request #13845 from bluetech/config-tweaks

config: some config file/ini cleanups and tweaks

Author: bluetech

src/_pytest/config/__init__.py

@@ -1647,7 +1647,7 @@ def _getini(self, name: str):
             raise ValueError(f"unknown configuration value: {name!r}") from e
 
         # Collect all possible values (canonical name + aliases) from inicfg.
-        # Each candidate is (IniValue, is_canonical).
+        # Each candidate is (ConfigValue, is_canonical).
         candidates = []
         if canonical_name in self.inicfg:
             candidates.append((self.inicfg[canonical_name], True))
@@ -1661,8 +1661,8 @@ def _getini(self, name: str):
         # Pick the best candidate based on precedence:
         # 1. CLI override takes precedence over file, then
         # 2. Canonical name takes precedence over alias.
-        ini_value = max(candidates, key=lambda x: (x[0].origin == "override", x[1]))[0]
-        value = ini_value.value
+        selected = max(candidates, key=lambda x: (x[0].origin == "override", x[1]))[0]
+        value = selected.value
 
         # Coerce the values based on types.
         #
@@ -1710,8 +1710,6 @@ def _getini(self, name: str):
                     f"Expected a float string for option {name} of type float, but got: {value!r}"
                 ) from None
             return float(value)
-        elif type is None:
-            return value
         else:
             return self._getini_unknown_type(name, type, value)
 

src/_pytest/config/argparsing.py

@@ -50,8 +50,7 @@ def __init__(
         self._groups: list[OptionGroup] = []
         self._processopt = processopt
         self._usage = usage
-        self._inidict: dict[str, tuple[str, str | None, Any]] = {}
-        self._ininames: list[str] = []
+        self._inidict: dict[str, tuple[str, str, Any]] = {}
         # Maps alias -> canonical name.
         self._ini_aliases: dict[str, str] = {}
         self.extra_info: dict[str, Any] = {}
@@ -238,11 +237,12 @@ def addini(
             "int",
             "float",
         )
+        if type is None:
+            type = "string"
         if default is NOT_SET:
             default = get_ini_default_for_type(type)
 
         self._inidict[name] = (help, type, default)
-        self._ininames.append(name)
 
         for alias in aliases:
             if alias in self._inidict:
@@ -255,16 +255,13 @@ def addini(
 def get_ini_default_for_type(
     type: Literal[
         "string", "paths", "pathlist", "args", "linelist", "bool", "int", "float"
-    ]
-    | None,
+    ],
 ) -> 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"):
+    if type in ("paths", "pathlist", "args", "linelist"):
         return []
     elif type == "bool":
         return False

src/_pytest/config/findpaths.py

@@ -3,6 +3,7 @@
 from collections.abc import Iterable
 from collections.abc import Sequence
 from dataclasses import dataclass
+from dataclasses import KW_ONLY
 import os
 from pathlib import Path
 import sys
@@ -19,8 +20,8 @@
 
 
 @dataclass(frozen=True)
-class IniValue:
-    """Represents an ini configuration value with its origin.
+class ConfigValue:
+    """Represents a configuration value with its origin.
 
     This allows tracking whether a value came from a configuration file
     or from a CLI override (--override-ini), which is important for
@@ -31,10 +32,11 @@ class IniValue:
     # str/list[str] during parsing to maintain compatibility with the rest of
     # the configuration system.
     value: str | list[str]
+    _: KW_ONLY
     origin: Literal["file", "override"]
 
 
-ConfigDict: TypeAlias = dict[str, IniValue]
+ConfigDict: TypeAlias = dict[str, ConfigValue]
 
 
 def _parse_ini_config(path: Path) -> iniconfig.IniConfig:
@@ -61,7 +63,9 @@ def load_config_dict_from_file(
         iniconfig = _parse_ini_config(filepath)
 
         if "pytest" in iniconfig:
-            return {k: IniValue(v, "file") for k, v in iniconfig["pytest"].items()}
+            return {
+                k: ConfigValue(v, origin="file") for k, v in iniconfig["pytest"].items()
+            }
         else:
             # "pytest.ini" files are always the source of configuration, even if empty.
             if filepath.name == "pytest.ini":
@@ -72,7 +76,10 @@ def load_config_dict_from_file(
         iniconfig = _parse_ini_config(filepath)
 
         if "tool:pytest" in iniconfig.sections:
-            return {k: IniValue(v, "file") for k, v in iniconfig["tool:pytest"].items()}
+            return {
+                k: ConfigValue(v, origin="file")
+                for k, v in iniconfig["tool:pytest"].items()
+            }
         elif "pytest" in iniconfig.sections:
             # If a setup.cfg contains a "[pytest]" section, we raise a failure to indicate users that
             # plain "[pytest]" sections in setup.cfg files is no longer supported (#3086).
@@ -99,7 +106,9 @@ def load_config_dict_from_file(
             def make_scalar(v: object) -> str | list[str]:
                 return v if isinstance(v, list) else str(v)
 
-            return {k: IniValue(make_scalar(v), "file") for k, v in result.items()}
+            return {
+                k: ConfigValue(make_scalar(v), origin="file") for k, v in result.items()
+            }
 
     return None
 
@@ -215,7 +224,7 @@ def parse_override_ini(override_ini: Sequence[str] | None) -> ConfigDict:
                 f"-o/--override-ini expects option=value style (got: {ini_config!r})."
             ) from e
         else:
-            overrides[key] = IniValue(user_ini_value, "override")
+            overrides[key] = ConfigValue(user_ini_value, origin="override")
     return overrides
 
 

src/_pytest/helpconfig.py

@@ -184,10 +184,8 @@ def showhelp(config: Config) -> None:
     columns = tw.fullwidth  # costly call
     indent_len = 24  # based on argparse's max_help_position=24
     indent = " " * indent_len
-    for name in config._parser._ininames:
+    for name in config._parser._inidict:
         help, type, _default = config._parser._inidict[name]
-        if type is None:
-            type = "string"
         if help is None:
             raise TypeError(f"help argument cannot be None for {name}")
         spec = f"{name} ({type}):"

testing/test_config.py

@@ -23,9 +23,9 @@
 from _pytest.config.argparsing import get_ini_default_for_type
 from _pytest.config.argparsing import Parser
 from _pytest.config.exceptions import UsageError
+from _pytest.config.findpaths import ConfigValue
 from _pytest.config.findpaths import determine_setup
 from _pytest.config.findpaths import get_common_ancestor
-from _pytest.config.findpaths import IniValue
 from _pytest.config.findpaths import locate_config
 from _pytest.monkeypatch import MonkeyPatch
 from _pytest.pathlib import absolutepath
@@ -58,9 +58,9 @@ def test_getcfg_and_config(
             encoding="utf-8",
         )
         _, _, cfg, _ = locate_config(Path.cwd(), [sub])
-        assert cfg["name"] == IniValue("value", "file")
+        assert cfg["name"] == ConfigValue("value", origin="file")
         config = pytester.parseconfigure(str(sub))
-        assert config.inicfg["name"] == IniValue("value", "file")
+        assert config.inicfg["name"] == ConfigValue("value", origin="file")
 
     def test_setupcfg_uses_toolpytest_with_pytest(self, pytester: Pytester) -> None:
         p1 = pytester.makepyfile("def test(): pass")
@@ -1314,7 +1314,7 @@ def test_inifilename(self, tmp_path: Path) -> None:
 
         # this indicates this is the file used for getting configuration values
         assert config.inipath == inipath
-        assert config.inicfg.get("name") == IniValue("value", "file")
+        assert config.inicfg.get("name") == ConfigValue("value", origin="file")
         assert config.inicfg.get("should_not_be_set") is None
 
 
@@ -1808,7 +1808,7 @@ def test_with_ini(self, tmp_path: Path, name: str, contents: str) -> None:
         )
         assert rootpath == tmp_path
         assert parsed_inipath == inipath
-        assert ini_config["x"] == IniValue("10", "file")
+        assert ini_config["x"] == ConfigValue("10", origin="file")
 
     @pytest.mark.parametrize("name", ["setup.cfg", "tox.ini"])
     def test_pytestini_overrides_empty_other(self, tmp_path: Path, name: str) -> None:
@@ -1882,7 +1882,7 @@ def test_with_specific_inifile(
         )
         assert rootpath == tmp_path
         assert inipath == p
-        assert ini_config["x"] == IniValue("10", "file")
+        assert ini_config["x"] == ConfigValue("10", origin="file")
 
     def test_explicit_config_file_sets_rootdir(
         self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch
@@ -2152,7 +2152,9 @@ def test_addopts_before_initini(
         monkeypatch.setenv("PYTEST_ADDOPTS", f"-o cache_dir={cache_dir}")
         config = _config_for_test
         config._preparse([], addopts=True)
-        assert config.inicfg.get("cache_dir") == IniValue(cache_dir, "override")
+        assert config.inicfg.get("cache_dir") == ConfigValue(
+            cache_dir, origin="override"
+        )
 
     def test_addopts_from_env_not_concatenated(
         self, monkeypatch: MonkeyPatch, _config_for_test
@@ -2190,7 +2192,9 @@ def test_override_ini_does_not_contain_paths(
         """Check that -o no longer swallows all options after it (#3103)"""
         config = _config_for_test
         config._preparse(["-o", "cache_dir=/cache", "/some/test/path"])
-        assert config.inicfg.get("cache_dir") == IniValue("/cache", "override")
+        assert config.inicfg.get("cache_dir") == ConfigValue(
+            "/cache", origin="override"
+        )
 
     def test_multiple_override_ini_options(self, pytester: Pytester) -> None:
         """Ensure a file path following a '-o' option does not generate an error (#3103)"""

testing/test_findpaths.py

@@ -6,9 +6,9 @@
 from textwrap import dedent
 
 from _pytest.config import UsageError
+from _pytest.config.findpaths import ConfigValue
 from _pytest.config.findpaths import get_common_ancestor
 from _pytest.config.findpaths import get_dirs_from_args
-from _pytest.config.findpaths import IniValue
 from _pytest.config.findpaths import is_fs_root
 from _pytest.config.findpaths import load_config_dict_from_file
 import pytest
@@ -25,13 +25,13 @@ def test_pytest_ini(self, tmp_path: Path) -> None:
         """[pytest] section in pytest.ini files is read correctly"""
         fn = tmp_path / "pytest.ini"
         fn.write_text("[pytest]\nx=1", encoding="utf-8")
-        assert load_config_dict_from_file(fn) == {"x": IniValue("1", "file")}
+        assert load_config_dict_from_file(fn) == {"x": ConfigValue("1", origin="file")}
 
     def test_custom_ini(self, tmp_path: Path) -> None:
         """[pytest] section in any .ini file is read correctly"""
         fn = tmp_path / "custom.ini"
         fn.write_text("[pytest]\nx=1", encoding="utf-8")
-        assert load_config_dict_from_file(fn) == {"x": IniValue("1", "file")}
+        assert load_config_dict_from_file(fn) == {"x": ConfigValue("1", origin="file")}
 
     def test_custom_ini_without_section(self, tmp_path: Path) -> None:
         """Custom .ini files without [pytest] section are not considered for configuration"""
@@ -49,7 +49,7 @@ def test_valid_cfg_file(self, tmp_path: Path) -> None:
         """Custom .cfg files with [tool:pytest] section are read correctly"""
         fn = tmp_path / "custom.cfg"
         fn.write_text("[tool:pytest]\nx=1", encoding="utf-8")
-        assert load_config_dict_from_file(fn) == {"x": IniValue("1", "file")}
+        assert load_config_dict_from_file(fn) == {"x": ConfigValue("1", origin="file")}
 
     def test_unsupported_pytest_section_in_cfg_file(self, tmp_path: Path) -> None:
         """.cfg files with [pytest] section are no longer supported and should fail to alert users"""
@@ -97,11 +97,11 @@ def test_valid_toml_file(self, tmp_path: Path) -> None:
             encoding="utf-8",
         )
         assert load_config_dict_from_file(fn) == {
-            "x": IniValue("1", "file"),
-            "y": IniValue("20.0", "file"),
-            "values": IniValue(["tests", "integration"], "file"),
-            "name": IniValue("foo", "file"),
-            "heterogeneous_array": IniValue([1, "str"], "file"),  # type: ignore[list-item]
+            "x": ConfigValue("1", origin="file"),
+            "y": ConfigValue("20.0", origin="file"),
+            "values": ConfigValue(["tests", "integration"], origin="file"),
+            "name": ConfigValue("foo", origin="file"),
+            "heterogeneous_array": ConfigValue([1, "str"], origin="file"),  # type: ignore[list-item]
         }