From 861bbd7d93a50967271d1ef6eeec6908cfeaa815 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bern=C3=A1t=20G=C3=A1bor?= Date: Wed, 11 Feb 2026 11:24:36 -0800 Subject: [PATCH 1/2] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20refactor(plugin):=20co?= =?UTF-8?q?nsolidate=20TOML=20config=20parsing?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The plugin was parsing TOML files twice during startup: once to extract env_files and again to load environment entries. This created ~40 lines of duplicate directory-walking logic and inconsistent error handling where TOMLDecodeError was silently swallowed when reading env_files but allowed to propagate when reading env entries. Consolidated into a single _load_toml_config() function that parses once and returns both env_files and entries. Added _find_toml_config() to handle discovery, checking early_config.inipath first before walking the tree. This maintains pytest-env's precedence model (TOML over INI) while avoiding redundant work. Also migrated test fixtures from deprecated testdir to modern pytester for future compatibility, and removed obsolete ANN101 lint rule from config. --- pyproject.toml | 1 - src/pytest_env/plugin.py | 105 +++++++++++++++++++-------------------- tests/test_env.py | 61 ++++++++++------------- 3 files changed, 77 insertions(+), 90 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index db7be9e..b2a2a27 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -63,7 +63,6 @@ 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 diff --git a/src/pytest_env/plugin.py b/src/pytest_env/plugin.py index 2f38904..784b7cb 100644 --- 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 @@ -39,6 +38,24 @@ class Entry: unset: bool = False +def _find_toml_config(early_config: pytest.Config) -> Path | None: + """Find TOML config file by checking inipath first, then walking up the tree.""" # noqa: DOC201 + 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 + + @pytest.hookimpl(tryfirst=True) def pytest_load_initial_conftests( args: list[str], # noqa: ARG001 @@ -46,7 +63,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,35 +77,31 @@ 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)): +def _load_toml_config(config_path: Path) -> tuple[list[str], list[Entry]]: + """Load env_files and entries from TOML config file.""" # noqa: DOC201 + with config_path.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 [], [] + + 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 [] + + entries = list(_parse_toml_config(pytest_env_config)) + return env_files, entries + + +def _load_env_files(early_config: pytest.Config, env_files: list[str]) -> Generator[Path, None, None]: + """Resolve and yield existing env files.""" # noqa: DOC402 + 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(): @@ -105,39 +122,19 @@ def _parse_toml_config(config: dict[str, Any]) -> Generator[Entry, None, None]: 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 pytest_toml_name == "pyproject.toml": # in pyproject.toml the path is tool.pytest_env - config = config.get("tool", {}) - - if "pytest_env" in config: - has_toml = True - yield from _parse_toml_config(config["pytest_env"]) - - break # breaks the pytest_toml_name forloop - if has_toml: # breaks the path forloop - break - - if has_toml: - return + """Load env entries from config, preferring TOML over INI.""" # noqa: DOC402 + 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 parts = line.partition("=") ini_key_parts = parts[0].split(":") flags = {k.strip().upper() for k in ini_key_parts[:-1]} - # R: is a way to designate whether to use raw value -> perform no transformation of the value transform = "R" not in flags - # D: is a way to mark the value to be set only if it does not exist yet skip_if_set = "D" in flags - # U: is a way to unset (remove) an environment variable unset = "U" in flags key = ini_key_parts[-1].strip() value = parts[2].strip() diff --git a/tests/test_env.py b/tests/test_env.py index 3351fb3..90efa54 100644 --- 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,7 +450,7 @@ 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, @@ -466,13 +458,12 @@ def test_env_via_env_file( # noqa: PLR0913, PLR0917 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)", From 21bb70349cdfbab27b9bd9b7ac3495e1a4de43c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bern=C3=A1t=20G=C3=A1bor?= Date: Wed, 11 Feb 2026 11:29:36 -0800 Subject: [PATCH 2/2] =?UTF-8?q?=F0=9F=92=84=20style(plugin):=20reorder=20f?= =?UTF-8?q?unctions=20and=20restore=20flag=20comments?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reordered functions to follow topological usage order with the main hook entrypoint at the top, making the code easier to read from top to bottom. Restored comments explaining the R:, D:, and U: flags in INI config parsing. These document the user-facing API and help explain why certain transformations happen, not just how the code works. Moved DOC201 and DOC402 linting rules to global ignore config instead of using per-function noqa comments, keeping the codebase cleaner. --- pyproject.toml | 2 ++ src/pytest_env/plugin.py | 72 +++++++++++++++++++++------------------- 2 files changed, 40 insertions(+), 34 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index b2a2a27..344ad6e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -67,6 +67,8 @@ lint.ignore = [ "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 index 784b7cb..e6a06bd 100644 --- a/src/pytest_env/plugin.py +++ b/src/pytest_env/plugin.py @@ -38,24 +38,6 @@ class Entry: unset: bool = False -def _find_toml_config(early_config: pytest.Config) -> Path | None: - """Find TOML config file by checking inipath first, then walking up the tree.""" # noqa: DOC201 - 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 - - @pytest.hookimpl(tryfirst=True) def pytest_load_initial_conftests( args: list[str], # noqa: ARG001 @@ -80,8 +62,26 @@ def pytest_load_initial_conftests( os.environ[entry.key] = entry.value.format(**os.environ) if entry.transform else entry.value +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 _load_toml_config(config_path: Path) -> tuple[list[str], list[Entry]]: - """Load env_files and entries from TOML config file.""" # noqa: DOC201 + """Load env_files and entries from TOML config file.""" with config_path.open("rb") as file_handler: config = tomllib.load(file_handler) @@ -100,7 +100,7 @@ def _load_toml_config(config_path: Path) -> tuple[list[str], list[Entry]]: def _load_env_files(early_config: pytest.Config, env_files: list[str]) -> Generator[Path, None, None]: - """Resolve and yield existing env files.""" # noqa: DOC402 + """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: @@ -108,21 +108,8 @@ def _load_env_files(early_config: pytest.Config, env_files: list[str]) -> Genera yield resolved -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_values(early_config: pytest.Config) -> Iterator[Entry]: - """Load env entries from config, preferring TOML over INI.""" # noqa: DOC402 + """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: @@ -130,12 +117,29 @@ def _load_values(early_config: pytest.Config) -> Iterator[Entry]: 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 parts = line.partition("=") ini_key_parts = parts[0].split(":") flags = {k.strip().upper() for k in ini_key_parts[:-1]} + # R: is a way to designate whether to use raw value -> perform no transformation of the value transform = "R" not in flags + # D: is a way to mark the value to be set only if it does not exist yet skip_if_set = "D" in flags + # U: is a way to unset (remove) an environment variable unset = "U" in flags 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)