Skip to content

Fix for tox4 regression issue with setenv file and substitutions (#2435) #3521

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 23 additions & 8 deletions src/tox/config/set_env.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ def __init__( # noqa: C901, PLR0912
return
for line in raw.splitlines(): # noqa: PLR1702
if line.strip():
if line.startswith("file|"): # environment files to be handled later
self._env_files.append(line[len("file|") :])
if self._is_file_line(line):
self._env_files.append(self._parse_file_line(line))
else:
try:
key, value = self._extract_key_value(line)
Expand All @@ -52,12 +52,20 @@ def __init__( # noqa: C901, PLR0912
else:
self._raw[key] = value

@staticmethod
def _is_file_line(line: str) -> bool:
return line.startswith("file|")

@staticmethod
def _parse_file_line(line: str) -> str:
return line[len("file|") :]

def use_replacer(self, value: Replacer, args: ConfigLoadArgs) -> None:
self._replacer = value
for filename in self._env_files:
self._read_env_file(filename, args)
self._raw.update(self._stream_env_file(filename, args))

def _read_env_file(self, filename: str, args: ConfigLoadArgs) -> None:
def _stream_env_file(self, filename: str, args: ConfigLoadArgs) -> Iterator[tuple[str, str]]:
# Our rules in the documentation, some upstream environment file rules (we follow mostly the docker one):
# - https://www.npmjs.com/package/dotenv#rules
# - https://docs.docker.com/compose/env-file/
Expand All @@ -70,8 +78,7 @@ def _read_env_file(self, filename: str, args: ConfigLoadArgs) -> None:
env_line = env_line.strip() # noqa: PLW2901
if not env_line or env_line.startswith("#"):
continue
key, value = self._extract_key_value(env_line)
self._raw[key] = value
yield self._extract_key_value(env_line)

@staticmethod
def _extract_key_value(line: str) -> tuple[str, str]:
Expand Down Expand Up @@ -100,10 +107,18 @@ def __iter__(self) -> Iterator[str]:
# start with the materialized ones, maybe we don't need to materialize the raw ones
yield from self._materialized.keys()
yield from list(self._raw.keys()) # iterating over this may trigger materialization and change the dict
args = ConfigLoadArgs([], self._name, self._env_name)
while self._needs_replacement:
line = self._needs_replacement.pop(0)
expanded_line = self._replacer(line, ConfigLoadArgs([], self._name, self._env_name))
sub_raw = dict(self._extract_key_value(sub_line) for sub_line in expanded_line.splitlines() if sub_line)
expanded_line = self._replacer(line, args)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain the change here and why you done it this way?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • [+] wrote descriptive pull request text

This part you didn't really address. Writing a descriptive text involves the approach you've taken and why.

sub_raw: dict[str, str] = {}
for sub_line in filter(None, expanded_line.splitlines()):
if not self._is_file_line(sub_line):
sub_raw.__setitem__(*self._extract_key_value(sub_line))
else:
for k, v in self._stream_env_file(self._parse_file_line(sub_line), args):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No single letter variables outside of comprehensions.

if k not in self._raw:
sub_raw[k] = v # noqa: PERF403
self._raw.update(sub_raw)
self.changed = True # loading while iterating can cause these values to be missed
yield from sub_raw.keys()
Expand Down
48 changes: 48 additions & 0 deletions tests/config/test_set_env.py
Original file line number Diff line number Diff line change
Expand Up @@ -240,3 +240,51 @@ def test_set_env_environment_file_missing(tox_project: ToxProjectCreator) -> Non
result = project.run("r")
result.assert_failed()
assert f"py: failed with {project.path / 'magic.txt'} does not exist for set_env" in result.out


# https://github.com/tox-dev/tox/issues/2435
def test_set_env_environment_with_file_and_expanded_substitution(
tox_project: ToxProjectCreator, monkeypatch: MonkeyPatch
) -> None:
conf = {
"tox.ini": """
[tox]
envlist =
check

[testenv]
setenv =
file|.env
PRECENDENCE_TEST_1=1_expanded_precedence

[testenv:check]
setenv =
{[testenv]setenv}
PRECENDENCE_TEST_1=1_self_precedence
PRECENDENCE_TEST_2=2_self_precedence
""",
".env": """
PRECENDENCE_TEST_1=1_file_precedence
PRECENDENCE_TEST_2=2_file_precedence
PRECENDENCE_TEST_3=3_file_precedence
""",
}
monkeypatch.setenv("env_file", ".env")
project = tox_project(conf)

result = project.run("c", "-k", "set_env", "-e", "check")
result.assert_success()
set_env = result.env_conf("check")["set_env"]
content = {k: set_env.load(k) for k in set_env}
assert content == {
"PIP_DISABLE_PIP_VERSION_CHECK": "1",
"PYTHONHASHSEED": ANY,
"PYTHONIOENCODING": "utf-8",
"PRECENDENCE_TEST_1": "1_expanded_precedence",
"PRECENDENCE_TEST_2": "2_self_precedence",
"PRECENDENCE_TEST_3": "3_file_precedence",
}

result = project.run("r", "-e", "check")
result.assert_success()
assert "check: OK" in result.out