Skip to content

Commit

Permalink
Ignore empty env variable values when updating them
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Tadej Borovšak committed May 17, 2021
1 parent 9a2c156 commit 06f0e70
Show file tree
Hide file tree
Showing 2 changed files with 100 additions and 1 deletion.
5 changes: 4 additions & 1 deletion src/ansiblelint/prerun.py
Expand Up @@ -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
Expand Down
96 changes: 96 additions & 0 deletions 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


Expand Down Expand Up @@ -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

0 comments on commit 06f0e70

Please sign in to comment.