From e52b7f8ed03196aa911f37128b36b0d89e9927e1 Mon Sep 17 00:00:00 2001 From: Abedyn Date: Wed, 4 Nov 2026 18:00:52 +0200 Subject: [PATCH 1/2] fix(util): tolerate deleted temporary environment variables When a `patch_env()` body deletes a variable that was initially absent, cleanup raises `KeyError` and can mask an exception from the body. Use `os.environ.pop(name, None)` to restore absence idempotently while preserving restoration of existing values. Add 18 regression cases for absent, empty and populated original values, keep/replace/delete body mutations and normal/exceptional exits. The focused CLI-backend tests changed from 2 failed and 16 passed to 18 passed. Ruff lint/format and `git diff --check` passed. The full suite was not run because its required `GixPython` Windows wheel was unavailable. Agent-Model: unknown --- git/util.py | 2 +- test/test_patch_env.py | 39 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) create mode 100644 test/test_patch_env.py diff --git a/git/util.py b/git/util.py index 46891ec30..90a498241 100644 --- a/git/util.py +++ b/git/util.py @@ -175,7 +175,7 @@ def patch_env(name: str, value: str) -> Generator[None, None, None]: yield finally: if old_value is None: - del os.environ[name] + os.environ.pop(name, None) else: os.environ[name] = old_value diff --git a/test/test_patch_env.py b/test/test_patch_env.py new file mode 100644 index 000000000..c5c8fec15 --- /dev/null +++ b/test/test_patch_env.py @@ -0,0 +1,39 @@ +"""Regression tests for temporary environment variable changes.""" + +import os + +import pytest + +from git.util import patch_env + + +@pytest.mark.parametrize("original", [None, "", "original"]) +@pytest.mark.parametrize("change", ["keep", "replace", "delete"]) +@pytest.mark.parametrize("raise_error", [False, True]) +def test_patch_env_restores_original_state(monkeypatch, original, change, raise_error): + name = "GITPYTHON_TEST_PATCH_ENV" + if original is None: + monkeypatch.delenv(name, raising=False) + else: + monkeypatch.setenv(name, original) + + def change_environment(): + with patch_env(name, "temporary"): + assert os.environ[name] == "temporary" + if change == "replace": + os.environ[name] = "changed" + elif change == "delete": + del os.environ[name] + if raise_error: + raise RuntimeError("context body failed") + + if raise_error: + with pytest.raises(RuntimeError, match="context body failed"): + change_environment() + else: + change_environment() + + if original is None: + assert name not in os.environ + else: + assert os.environ[name] == original From 32b4d8e7005eee2b5e927adf5dc9151e8b34001a Mon Sep 17 00:00:00 2001 From: Byron Date: Thu, 5 Nov 2026 04:53:43 +0100 Subject: [PATCH 2/2] review --- test/test_patch_env.py | 39 --------------------------------------- 1 file changed, 39 deletions(-) delete mode 100644 test/test_patch_env.py diff --git a/test/test_patch_env.py b/test/test_patch_env.py deleted file mode 100644 index c5c8fec15..000000000 --- a/test/test_patch_env.py +++ /dev/null @@ -1,39 +0,0 @@ -"""Regression tests for temporary environment variable changes.""" - -import os - -import pytest - -from git.util import patch_env - - -@pytest.mark.parametrize("original", [None, "", "original"]) -@pytest.mark.parametrize("change", ["keep", "replace", "delete"]) -@pytest.mark.parametrize("raise_error", [False, True]) -def test_patch_env_restores_original_state(monkeypatch, original, change, raise_error): - name = "GITPYTHON_TEST_PATCH_ENV" - if original is None: - monkeypatch.delenv(name, raising=False) - else: - monkeypatch.setenv(name, original) - - def change_environment(): - with patch_env(name, "temporary"): - assert os.environ[name] == "temporary" - if change == "replace": - os.environ[name] = "changed" - elif change == "delete": - del os.environ[name] - if raise_error: - raise RuntimeError("context body failed") - - if raise_error: - with pytest.raises(RuntimeError, match="context body failed"): - change_environment() - else: - change_environment() - - if original is None: - assert name not in os.environ - else: - assert os.environ[name] == original