Ignore empty env variable values when updating them (#1552)

Right now, if the variable is not set, prerun routines will preped the
: to the variable value, which is not something we want. This commit
makes sure we properly handle the aforementioned situation and ignore
any empty values.

src/ansiblelint/prerun.py

@@ -321,7 +321,10 @@ def _write_module_stub(
 def _update_env(varname: str, value: List[str], default: str = "") -> None:
     """Update colon based environment variable if needed. by appending."""
     if value:
-        value = [*os.environ.get(varname, default=default).split(':'), *value]
+        orig_value = os.environ.get(varname, default=default)
+        if orig_value:
+            # Prepend original or default variable content to custom content.
+            value = [*orig_value.split(':'), *value]
         value_str = ":".join(value)
         if value_str != os.environ.get(varname, ""):
             os.environ[varname] = value_str

test/test_prerun.py

@@ -1,8 +1,10 @@
 """Tests related to prerun part of the linter."""
 import os
 
+import pytest
 from flaky import flaky
 
+from ansiblelint import prerun
 from ansiblelint.testing import run_ansible_lint
 
 
@@ -35,3 +37,97 @@ def test_prerun_reqs_v2() -> None:
     assert "Running ansible-galaxy role install" in result.stderr, result.stderr
     assert "Running ansible-galaxy collection install" in result.stderr, result.stderr
     assert result.returncode == 0, result
+
+
+def test__update_env_no_old_value_no_default_no_value(monkeypatch):
+    """Make sure empty value does not touch environment."""
+    monkeypatch.delenv("DUMMY_VAR", raising=False)
+
+    prerun._update_env("DUMMY_VAR", [])
+
+    assert "DUMMY_VAR" not in os.environ
+
+
+def test__update_env_no_old_value_no_value(monkeypatch):
+    """Make sure empty value does not touch environment."""
+    monkeypatch.delenv("DUMMY_VAR", raising=False)
+
+    prerun._update_env("DUMMY_VAR", [], "a:b")
+
+    assert "DUMMY_VAR" not in os.environ
+
+
+def test__update_env_no_default_no_value(monkeypatch):
+    """Make sure empty value does not touch environment."""
+    monkeypatch.setenv("DUMMY_VAR", "a:b")
+
+    prerun._update_env("DUMMY_VAR", [])
+
+    assert os.environ["DUMMY_VAR"] == "a:b"
+
+
+@pytest.mark.parametrize(
+    ("value", "result"),
+    (
+        (["a"], "a"),
+        (["a", "b"], "a:b"),
+        (["a", "b", "c"], "a:b:c"),
+    ),
+)
+def test__update_env_no_old_value_no_default(monkeypatch, value, result):
+    """Values are concatenated using : as the separator."""
+    monkeypatch.delenv("DUMMY_VAR", raising=False)
+
+    prerun._update_env("DUMMY_VAR", value)
+
+    assert os.environ["DUMMY_VAR"] == result
+
+
+@pytest.mark.parametrize(
+    ("default", "value", "result"),
+    (
+        ("a:b", ["c"], "a:b:c"),
+        ("a:b", ["c:d"], "a:b:c:d"),
+    ),
+)
+def test__update_env_no_old_value(monkeypatch, default, value, result):
+    """Values are appended to default value."""
+    monkeypatch.delenv("DUMMY_VAR", raising=False)
+
+    prerun._update_env("DUMMY_VAR", value, default)
+
+    assert os.environ["DUMMY_VAR"] == result
+
+
+@pytest.mark.parametrize(
+    ("old_value", "value", "result"),
+    (
+        ("a:b", ["c"], "a:b:c"),
+        ("a:b", ["c:d"], "a:b:c:d"),
+    ),
+)
+def test__update_env_no_default(monkeypatch, old_value, value, result):
+    """Values are appended to preexisting value."""
+    monkeypatch.setenv("DUMMY_VAR", old_value)
+
+    prerun._update_env("DUMMY_VAR", value)
+
+    assert os.environ["DUMMY_VAR"] == result
+
+
+@pytest.mark.parametrize(
+    ("old_value", "default", "value", "result"),
+    (
+        ("", "", ["e"], "e"),
+        ("a", "", ["e"], "a:e"),
+        ("", "c", ["e"], "e"),
+        ("a", "c", ["e:f"], "a:e:f"),
+    ),
+)
+def test__update_env(monkeypatch, old_value, default, value, result):
+    """Defaults are ignored when preexisting value is present."""
+    monkeypatch.setenv("DUMMY_VAR", old_value)
+
+    prerun._update_env("DUMMY_VAR", value)
+
+    assert os.environ["DUMMY_VAR"] == result