♻️ refactor(plugin): consolidate TOML config parsing (#193)

gaborbernat · web-flow · commit 16158a44d7f0 · 2026-02-11T11:32:23.000-08:00
diff --git a/pyproject.toml b/pyproject.toml
@@ -63,11 +63,12 @@ lint.select = [
   "ALL",
 ]
 lint.ignore = [
-  "ANN101", # no type annotation for self
   "COM812", # Conflict with formatter
   "CPY",    # No copyright statements
   "D203",   # `one-blank-line-before-class` (D203) and `no-blank-line-before-class` (D211) are incompatible
   "D212",   # `multi-line-summary-first-line` (D212) and `multi-line-summary-second-line` (D213) are incompatible
+  "DOC201", # `return` is not documented in docstring (we prefer minimal docs)
+  "DOC402", # `yield` is not documented in docstring (we prefer minimal docs)
   "ISC001", # Conflict with formatter
   "S104",   # Possible binding to all interface
 ]
diff --git a/src/pytest_env/plugin.py b/src/pytest_env/plugin.py
@@ -5,7 +5,6 @@
 import os
 import sys
 from dataclasses import dataclass
-from itertools import chain
 from typing import TYPE_CHECKING, Any
 
 import pytest
@@ -46,7 +45,11 @@ def pytest_load_initial_conftests(
     parser: pytest.Parser,  # noqa: ARG001
 ) -> None:
     """Load environment variables from configuration files."""
-    for env_file in _load_env_files(early_config):
+    env_files_list: list[str] = []
+    if toml_config := _find_toml_config(early_config):
+        env_files_list, _ = _load_toml_config(toml_config)
+
+    for env_file in _load_env_files(early_config, env_files_list):
         for key, value in dotenv_values(env_file).items():
             if value is not None:
                 os.environ[key] = value
@@ -56,77 +59,62 @@ def pytest_load_initial_conftests(
         elif entry.skip_if_set and entry.key in os.environ:
             continue
         else:
-            # transformation -> replace environment variables, e.g. TEST_DIR={USER}/repo_test_dir.
             os.environ[entry.key] = entry.value.format(**os.environ) if entry.transform else entry.value
 
 
-def _env_files_from_toml(early_config: pytest.Config) -> list[str]:
-    for path in chain.from_iterable([[early_config.rootpath], early_config.rootpath.parents]):
-        for pytest_toml_name in ("pytest.toml", ".pytest.toml", "pyproject.toml"):
-            pytest_toml_file = path / pytest_toml_name
-            if not pytest_toml_file.exists():
-                continue
-            with pytest_toml_file.open("rb") as file_handler:
-                try:
-                    config = tomllib.load(file_handler)
-                except tomllib.TOMLDecodeError:
-                    return []
-                if pytest_toml_name == "pyproject.toml":
-                    config = config.get("tool", {})
-                if (
-                    (pytest_env := config.get("pytest_env"))
-                    and isinstance(pytest_env, dict)
-                    and (raw := pytest_env.get("env_files"))
-                ):
-                    return [str(f) for f in (raw if isinstance(raw, list) else [raw])]
-                return []
-    return []
-
-
-def _load_env_files(early_config: pytest.Config) -> Generator[Path, None, None]:
-    if not (env_files := _env_files_from_toml(early_config)):
-        env_files = list(early_config.getini("env_files"))
-    for env_file_str in env_files:
-        if (resolved := early_config.rootpath / env_file_str).is_file():
-            yield resolved
+def _find_toml_config(early_config: pytest.Config) -> Path | None:
+    """Find TOML config file by checking inipath first, then walking up the tree."""
+    if (
+        early_config.inipath
+        and early_config.inipath.suffix == ".toml"
+        and early_config.inipath.name in {"pytest.toml", ".pytest.toml", "pyproject.toml"}
+    ):
+        return early_config.inipath
 
+    start_path = early_config.inipath.parent if early_config.inipath is not None else early_config.rootpath
+    for current_path in [start_path, *start_path.parents]:
+        for toml_name in ("pytest.toml", ".pytest.toml", "pyproject.toml"):
+            toml_file = current_path / toml_name
+            if toml_file.exists():
+                return toml_file
+    return None
 
-def _parse_toml_config(config: dict[str, Any]) -> Generator[Entry, None, None]:
-    for key, entry in config.items():
-        if key == "env_files" and isinstance(entry, list):
-            continue
-        if isinstance(entry, dict):
-            unset = bool(entry.get("unset"))
-            value = str(entry.get("value", "")) if not unset else ""
-            transform, skip_if_set = bool(entry.get("transform")), bool(entry.get("skip_if_set"))
-        else:
-            value, transform, skip_if_set, unset = str(entry), False, False, False
-        yield Entry(key, value, transform, skip_if_set, unset=unset)
 
+def _load_toml_config(config_path: Path) -> tuple[list[str], list[Entry]]:
+    """Load env_files and entries from TOML config file."""
+    with config_path.open("rb") as file_handler:
+        config = tomllib.load(file_handler)
 
-def _load_values(early_config: pytest.Config) -> Iterator[Entry]:
-    has_toml = False
-    start_path = early_config.inipath.parent if early_config.inipath is not None else early_config.rootpath
-    for path in chain.from_iterable([[start_path], start_path.parents]):
-        for pytest_toml_name in ("pytest.toml", ".pytest.toml", "pyproject.toml"):
-            pytest_toml_file = path / pytest_toml_name
-            if pytest_toml_file.exists():
-                with pytest_toml_file.open("rb") as file_handler:
-                    config = tomllib.load(file_handler)
+    if config_path.name == "pyproject.toml":
+        config = config.get("tool", {})
+
+    pytest_env_config = config.get("pytest_env", {})
+    if not pytest_env_config:
+        return [], []
 
-                    if pytest_toml_name == "pyproject.toml":  # in pyproject.toml the path is tool.pytest_env
-                        config = config.get("tool", {})
+    raw_env_files = pytest_env_config.get("env_files")
+    env_files = [str(f) for f in raw_env_files] if isinstance(raw_env_files, list) else []
 
-                    if "pytest_env" in config:
-                        has_toml = True
-                        yield from _parse_toml_config(config["pytest_env"])
+    entries = list(_parse_toml_config(pytest_env_config))
+    return env_files, entries
 
-                break  # breaks the pytest_toml_name forloop
-        if has_toml:  # breaks the path forloop
-            break
 
-    if has_toml:
-        return
+def _load_env_files(early_config: pytest.Config, env_files: list[str]) -> Generator[Path, None, None]:
+    """Resolve and yield existing env files."""
+    if not env_files:
+        env_files = list(early_config.getini("env_files"))
+    for env_file_str in env_files:
+        if (resolved := early_config.rootpath / env_file_str).is_file():
+            yield resolved
+
+
+def _load_values(early_config: pytest.Config) -> Iterator[Entry]:
+    """Load env entries from config, preferring TOML over INI."""
+    if toml_config := _find_toml_config(early_config):
+        _, entries = _load_toml_config(toml_config)
+        if entries:
+            yield from entries
+            return
 
     for line in early_config.getini("env"):
         # INI lines e.g. D:R:NAME=VAL has two flags (R and D), NAME key, and VAL value
@@ -142,3 +130,16 @@ def _load_values(early_config: pytest.Config) -> Iterator[Entry]:
         key = ini_key_parts[-1].strip()
         value = parts[2].strip()
         yield Entry(key, value, transform, skip_if_set, unset=unset)
+
+
+def _parse_toml_config(config: dict[str, Any]) -> Generator[Entry, None, None]:
+    for key, entry in config.items():
+        if key == "env_files" and isinstance(entry, list):
+            continue
+        if isinstance(entry, dict):
+            unset = bool(entry.get("unset"))
+            value = str(entry.get("value", "")) if not unset else ""
+            transform, skip_if_set = bool(entry.get("transform")), bool(entry.get("skip_if_set"))
+        else:
+            value, transform, skip_if_set, unset = str(entry), False, False, False
+        yield Entry(key, value, transform, skip_if_set, unset=unset)
diff --git a/tests/test_env.py b/tests/test_env.py
@@ -3,15 +3,11 @@
 import os
 import re
 from pathlib import Path
-from typing import TYPE_CHECKING
 from unittest import mock
 
 import pytest
 
-from pytest_env.plugin import _env_files_from_toml  # noqa: PLC2701
-
-if TYPE_CHECKING:
-    import pytest_mock
+from pytest_env.plugin import _load_toml_config  # noqa: PLC2701
 
 
 @pytest.mark.parametrize(
@@ -117,16 +113,15 @@
     ],
 )
 def test_env_via_pytest(
-    testdir: pytest.Testdir,
+    pytester: pytest.Pytester,
     env: dict[str, str],
     ini: str,
     expected_env: dict[str, str],
     request: pytest.FixtureRequest,
 ) -> None:
-    tmp_dir = Path(str(testdir.tmpdir))
     test_name = re.sub(r"\W|^(?=\d)", "_", request.node.callspec.id).lower()
-    Path(str(tmp_dir / f"test_{test_name}.py")).symlink_to(Path(__file__).parent / "template.py")
-    (tmp_dir / "pytest.ini").write_text(ini, encoding="utf-8")
+    (pytester.path / f"test_{test_name}.py").symlink_to(Path(__file__).parent / "template.py")
+    (pytester.path / "pytest.ini").write_text(ini, encoding="utf-8")
 
     new_env = {
         **env,
@@ -135,9 +130,8 @@ def test_env_via_pytest(
         "PYTEST_PLUGINS": "pytest_env.plugin",
     }
 
-    # monkeypatch persists env variables across parametrized tests, therefore using mock.patch.dict
     with mock.patch.dict(os.environ, new_env, clear=True):
-        result = testdir.runpytest()
+        result = pytester.runpytest()
 
     result.assert_outcomes(passed=1)
 
@@ -283,7 +277,7 @@ def test_env_via_pytest(
     ],
 )
 def test_env_via_toml(  # noqa: PLR0913, PLR0917
-    testdir: pytest.Testdir,
+    pytester: pytest.Pytester,
     env: dict[str, str],
     pyproject_toml: str,
     pytest_toml: str,
@@ -292,23 +286,22 @@ def test_env_via_toml(  # noqa: PLR0913, PLR0917
     pytest_toml_name: str | None,
     request: pytest.FixtureRequest,
 ) -> None:
-    tmp_dir = Path(str(testdir.tmpdir))
     test_name = re.sub(r"\W|^(?=\d)", "_", request.node.callspec.id).lower()
     if pyproject_toml:
-        (tmp_dir / "pyproject.toml").write_text(pyproject_toml, encoding="utf-8")
+        (pytester.path / "pyproject.toml").write_text(pyproject_toml, encoding="utf-8")
     if pytest_toml and pytest_toml_name:
-        toml_path = tmp_dir / pytest_toml_name
+        toml_path = pytester.path / pytest_toml_name
         toml_path.parent.mkdir(parents=True, exist_ok=True)
         toml_path.write_text(pytest_toml, encoding="utf-8")
 
     if pytest_toml_name and "/" in pytest_toml_name:
-        test_dir = tmp_dir / Path(pytest_toml_name).parent
+        test_dir = pytester.path / Path(pytest_toml_name).parent
     else:
-        test_dir = tmp_dir
+        test_dir = pytester.path
         if ini:
-            (tmp_dir / "pytest.ini").write_text(ini, encoding="utf-8")
+            (pytester.path / "pytest.ini").write_text(ini, encoding="utf-8")
 
-    Path(str(test_dir / f"test_{test_name}.py")).symlink_to(Path(__file__).parent / "template.py")
+    (test_dir / f"test_{test_name}.py").symlink_to(Path(__file__).parent / "template.py")
 
     new_env = {
         **env,
@@ -317,9 +310,8 @@ def test_env_via_toml(  # noqa: PLR0913, PLR0917
         "PYTEST_PLUGINS": "pytest_env.plugin",
     }
 
-    # monkeypatch persists env variables across parametrized tests, therefore using mock.patch.dict
     with mock.patch.dict(os.environ, new_env, clear=True):
-        result = testdir.runpytest(str(test_dir))
+        result = pytester.runpytest(str(test_dir))
 
     result.assert_outcomes(passed=1)
 
@@ -458,21 +450,20 @@ def test_env_via_toml(  # noqa: PLR0913, PLR0917
     ],
 )
 def test_env_via_env_file(  # noqa: PLR0913, PLR0917
-    testdir: pytest.Testdir,
+    pytester: pytest.Pytester,
     env: dict[str, str],
     env_file_content: str,
     config: str,
     expected_env: dict[str, str | None],
     config_type: str,
     request: pytest.FixtureRequest,
 ) -> None:
-    tmp_dir = Path(str(testdir.tmpdir))
     test_name = re.sub(r"\W|^(?=\d)", "_", request.node.callspec.id).lower()
-    Path(str(tmp_dir / f"test_{test_name}.py")).symlink_to(Path(__file__).parent / "template.py")
+    (pytester.path / f"test_{test_name}.py").symlink_to(Path(__file__).parent / "template.py")
     if env_file_content:
-        (tmp_dir / ".env").write_text(env_file_content, encoding="utf-8")
+        (pytester.path / ".env").write_text(env_file_content, encoding="utf-8")
     config_file_names = {"pyproject": "pyproject.toml", "pytest.toml": "pytest.toml", "ini": "pytest.ini"}
-    (tmp_dir / config_file_names[config_type]).write_text(config, encoding="utf-8")
+    (pytester.path / config_file_names[config_type]).write_text(config, encoding="utf-8")
 
     new_env = {
         **env,
@@ -482,24 +473,24 @@ def test_env_via_env_file(  # noqa: PLR0913, PLR0917
     }
 
     with mock.patch.dict(os.environ, new_env, clear=True):
-        result = testdir.runpytest()
+        result = pytester.runpytest()
 
     result.assert_outcomes(passed=1)
 
 
-def test_env_files_from_toml_bad_toml(tmp_path: Path, mocker: pytest_mock.MockerFixture) -> None:
-    (tmp_path / "pyproject.toml").write_text("bad toml", encoding="utf-8")
-    config = mocker.MagicMock()
-    config.rootpath = tmp_path
-    assert _env_files_from_toml(config) == []
+def test_env_files_from_toml_bad_toml(tmp_path: Path) -> None:
+    toml_file = tmp_path / "pyproject.toml"
+    toml_file.write_text("bad toml", encoding="utf-8")
+    with pytest.raises(Exception, match="Expected '=' after a key"):
+        _load_toml_config(toml_file)
 
 
 @pytest.mark.parametrize("toml_name", ["pytest.toml", ".pytest.toml", "pyproject.toml"])
-def test_env_via_pyproject_toml_bad(testdir: pytest.Testdir, toml_name: str) -> None:
-    toml_file = Path(str(testdir.tmpdir)) / toml_name
+def test_env_via_pyproject_toml_bad(pytester: pytest.Pytester, toml_name: str) -> None:
+    toml_file = pytester.path / toml_name
     toml_file.write_text("bad toml", encoding="utf-8")
 
-    result = testdir.runpytest()
+    result = pytester.runpytest()
     assert result.ret == 4
     assert result.errlines == [
         f"ERROR: {toml_file}: Expected '=' after a key in a key/value pair (at line 1, column 5)",

Original file line number	Diff line number	Diff line change
`@@ -63,11 +63,12 @@ lint.select = [`
`63`	`63`	`"ALL",`
`64`	`64`	`]`
`65`	`65`	`lint.ignore = [`
`66`		`- "ANN101", # no type annotation for self`
`67`	`66`	`"COM812", # Conflict with formatter`
`68`	`67`	`"CPY", # No copyright statements`
`69`	`68`	"D203", # `one-blank-line-before-class` (D203) and `no-blank-line-before-class` (D211) are incompatible
`70`	`69`	"D212", # `multi-line-summary-first-line` (D212) and `multi-line-summary-second-line` (D213) are incompatible
	`70`	+ "DOC201", # `return` is not documented in docstring (we prefer minimal docs)
	`71`	+ "DOC402", # `yield` is not documented in docstring (we prefer minimal docs)
`71`	`72`	`"ISC001", # Conflict with formatter`
`72`	`73`	`"S104", # Possible binding to all interface`
`73`	`74`	`]`