From 7fc91fe2c5e23ad6a17aab6a0c22b7036468c048 Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Sat, 25 Oct 2025 12:07:16 +0300 Subject: [PATCH 1/4] config: convert ini type `None` to `string` earlier Do it early so later code doesn't have to deal with `type=None`. --- src/_pytest/config/__init__.py | 2 -- src/_pytest/config/argparsing.py | 11 +++++------ src/_pytest/helpconfig.py | 2 -- 3 files changed, 5 insertions(+), 10 deletions(-) diff --git a/src/_pytest/config/__init__.py b/src/_pytest/config/__init__.py index 2af60fa9c3c..c82b33a7235 100644 --- a/src/_pytest/config/__init__.py +++ b/src/_pytest/config/__init__.py @@ -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) diff --git a/src/_pytest/config/argparsing.py b/src/_pytest/config/argparsing.py index afd1892b222..e30883acb8a 100644 --- a/src/_pytest/config/argparsing.py +++ b/src/_pytest/config/argparsing.py @@ -50,7 +50,7 @@ def __init__( self._groups: list[OptionGroup] = [] self._processopt = processopt self._usage = usage - self._inidict: dict[str, tuple[str, str | None, Any]] = {} + self._inidict: dict[str, tuple[str, str, Any]] = {} self._ininames: list[str] = [] # Maps alias -> canonical name. self._ini_aliases: dict[str, str] = {} @@ -238,6 +238,8 @@ def addini( "int", "float", ) + if type is None: + type = "string" if default is NOT_SET: default = get_ini_default_for_type(type) @@ -255,16 +257,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 diff --git a/src/_pytest/helpconfig.py b/src/_pytest/helpconfig.py index da5d06f83b0..b2158e4c116 100644 --- a/src/_pytest/helpconfig.py +++ b/src/_pytest/helpconfig.py @@ -186,8 +186,6 @@ def showhelp(config: Config) -> None: indent = " " * indent_len for name in config._parser._ininames: 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}):" From 1b89e4f32cce8e6b6c8c5a816930417f1d2e906b Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Sat, 25 Oct 2025 12:15:30 +0300 Subject: [PATCH 2/4] config: remove `Parser._ininames` Doesn't seem needed. Maybe it was useful before dicts had guaranteed iteration order? `pytest --help` output is unchanged. --- src/_pytest/config/argparsing.py | 2 -- src/_pytest/helpconfig.py | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/src/_pytest/config/argparsing.py b/src/_pytest/config/argparsing.py index e30883acb8a..e6e89e03a6b 100644 --- a/src/_pytest/config/argparsing.py +++ b/src/_pytest/config/argparsing.py @@ -51,7 +51,6 @@ def __init__( self._processopt = processopt self._usage = usage self._inidict: dict[str, tuple[str, str, Any]] = {} - self._ininames: list[str] = [] # Maps alias -> canonical name. self._ini_aliases: dict[str, str] = {} self.extra_info: dict[str, Any] = {} @@ -244,7 +243,6 @@ def addini( 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: diff --git a/src/_pytest/helpconfig.py b/src/_pytest/helpconfig.py index b2158e4c116..9e06a45ad52 100644 --- a/src/_pytest/helpconfig.py +++ b/src/_pytest/helpconfig.py @@ -184,7 +184,7 @@ 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 help is None: raise TypeError(f"help argument cannot be None for {name}") From 543d9aac933cd122ef509e3056cfc3959e128b3f Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Fri, 24 Oct 2025 22:34:02 +0300 Subject: [PATCH 3/4] config: rename `IniValue` to `ConfigValue` As we add native toml support, the name `IniValue` will be less appropriate. --- src/_pytest/config/__init__.py | 6 +++--- src/_pytest/config/findpaths.py | 16 +++++++++------- testing/test_config.py | 16 ++++++++-------- testing/test_findpaths.py | 18 +++++++++--------- 4 files changed, 29 insertions(+), 27 deletions(-) diff --git a/src/_pytest/config/__init__.py b/src/_pytest/config/__init__.py index c82b33a7235..29fe7d5dcbe 100644 --- a/src/_pytest/config/__init__.py +++ b/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. # diff --git a/src/_pytest/config/findpaths.py b/src/_pytest/config/findpaths.py index fcdcfa69f7d..8724db43352 100644 --- a/src/_pytest/config/findpaths.py +++ b/src/_pytest/config/findpaths.py @@ -19,8 +19,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 @@ -34,7 +34,7 @@ class IniValue: origin: Literal["file", "override"] -ConfigDict: TypeAlias = dict[str, IniValue] +ConfigDict: TypeAlias = dict[str, ConfigValue] def _parse_ini_config(path: Path) -> iniconfig.IniConfig: @@ -61,7 +61,7 @@ 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, "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 +72,9 @@ 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, "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 +101,7 @@ 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), "file") for k, v in result.items()} return None @@ -215,7 +217,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, "override") return overrides diff --git a/testing/test_config.py b/testing/test_config.py index 773bd2c927d..39cb2d5ed83 100644 --- a/testing/test_config.py +++ b/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", "file") config = pytester.parseconfigure(str(sub)) - assert config.inicfg["name"] == IniValue("value", "file") + assert config.inicfg["name"] == ConfigValue("value", "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", "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", "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", "file") def test_explicit_config_file_sets_rootdir( self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch @@ -2152,7 +2152,7 @@ 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, "override") def test_addopts_from_env_not_concatenated( self, monkeypatch: MonkeyPatch, _config_for_test @@ -2190,7 +2190,7 @@ 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", "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)""" diff --git a/testing/test_findpaths.py b/testing/test_findpaths.py index fed8c9d4838..8da24ea07eb 100644 --- a/testing/test_findpaths.py +++ b/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", "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", "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", "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", "file"), + "y": ConfigValue("20.0", "file"), + "values": ConfigValue(["tests", "integration"], "file"), + "name": ConfigValue("foo", "file"), + "heterogeneous_array": ConfigValue([1, "str"], "file"), # type: ignore[list-item] } From ab44bec1fae928052bdff86bd0094f212a8c7abe Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Fri, 24 Oct 2025 22:43:20 +0300 Subject: [PATCH 4/4] config: make `ConfigValue.origin` kw-only I am going to add another field, then it will start being ambiguous, so make it kw-only to ensure clarity. --- src/_pytest/config/findpaths.py | 15 +++++++++++---- testing/test_config.py | 18 +++++++++++------- testing/test_findpaths.py | 16 ++++++++-------- 3 files changed, 30 insertions(+), 19 deletions(-) diff --git a/src/_pytest/config/findpaths.py b/src/_pytest/config/findpaths.py index 8724db43352..db0b829a530 100644 --- a/src/_pytest/config/findpaths.py +++ b/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 @@ -31,6 +32,7 @@ class ConfigValue: # 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"] @@ -61,7 +63,9 @@ def load_config_dict_from_file( iniconfig = _parse_ini_config(filepath) if "pytest" in iniconfig: - return {k: ConfigValue(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": @@ -73,7 +77,8 @@ def load_config_dict_from_file( if "tool:pytest" in iniconfig.sections: return { - k: ConfigValue(v, "file") for k, v in iniconfig["tool:pytest"].items() + 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 @@ -101,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: ConfigValue(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 @@ -217,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] = ConfigValue(user_ini_value, "override") + overrides[key] = ConfigValue(user_ini_value, origin="override") return overrides diff --git a/testing/test_config.py b/testing/test_config.py index 39cb2d5ed83..cd8ed6b01d4 100644 --- a/testing/test_config.py +++ b/testing/test_config.py @@ -58,9 +58,9 @@ def test_getcfg_and_config( encoding="utf-8", ) _, _, cfg, _ = locate_config(Path.cwd(), [sub]) - assert cfg["name"] == ConfigValue("value", "file") + assert cfg["name"] == ConfigValue("value", origin="file") config = pytester.parseconfigure(str(sub)) - assert config.inicfg["name"] == ConfigValue("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") == ConfigValue("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"] == ConfigValue("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"] == ConfigValue("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") == ConfigValue(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") == ConfigValue("/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)""" diff --git a/testing/test_findpaths.py b/testing/test_findpaths.py index 8da24ea07eb..e8e70f9c6fb 100644 --- a/testing/test_findpaths.py +++ b/testing/test_findpaths.py @@ -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": ConfigValue("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": ConfigValue("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": ConfigValue("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": ConfigValue("1", "file"), - "y": ConfigValue("20.0", "file"), - "values": ConfigValue(["tests", "integration"], "file"), - "name": ConfigValue("foo", "file"), - "heterogeneous_array": ConfigValue([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] }