Skip to content
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

Override appending doesn't override empty settings #3100

Closed
stefanor opened this issue Aug 20, 2023 · 0 comments · Fixed by #3101
Closed

Override appending doesn't override empty settings #3100

stefanor opened this issue Aug 20, 2023 · 0 comments · Fixed by #3101

Comments

@stefanor
Copy link
Contributor

Issue

In my work in #3088, I got appending working, but didn't correctly handle the case where there is no existing configuration parameter to append to.

It wasn't obvious that a KeyError gets raised early in load() in this case, and bypasses my appending logic.

This breaks my entire use-case for #3087. Whoops!

Minimal example

$ cat > setup.cfg <<EOL
[metadata]
name = foo

[options]
py_modules = foo
EOL
$ cat > tox.ini <<EOL
[tox]
[testenv]
install_command = python -c 'import os; assert "foo" in os.environ'
EOL
$ tox -e py311 -x testenv.setenv=foo=bar
...
  py311: OK (0.48 seconds)
  congratulations :) (0.60 seconds)
$ tox -e py311 -x testenv.setenv+=foo=bar
...
py311: install_package> python -c 'import os; assert "foo" in os.environ' --force-reinstall --no-deps /tmp/toxhax/.tox/.tmp/package/3/foo-0.0.0.tar.gz
Traceback (most recent call last):
  File "<string>", line 1, in <module>
AssertionError
...
$ cat >> tox.ini << EOL
> setenv =
>   bar=baz
> EOL
$ tox -e py311 -x testenv.setenv+=foo=bar
...
  py311: OK (0.45 seconds)
  congratulations :) (0.57 seconds)
stefanor added a commit to stefanor/tox that referenced this issue Aug 20, 2023
The override logic in tox-dev#3088 didn't work quite correctly, because I'd
missed that KeyError would be raised early in the `load()` method,
before the appending logic.

Handle this, and test!

Fixes: tox-dev#3100
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant