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

Don't attempt to parse dicts if not replacing #597

Merged
merged 2 commits into from
Sep 2, 2017

Conversation

cboylan
Copy link

@cboylan cboylan commented Sep 1, 2017

When reading in the config and processing dict content we were splitting
on = assuming we would always have key = value pairs. However in the
case where we don't want to replace because the testenv will not be used
this can result in ValueErrors as substitution variables are not string
key = value pairs.

Avoid all this by only attempting to process dict key = values if we are
replacing in the first place. Otherwise the content isn't needed and we
just return the default value or empty dict.

This should fix #595

Thanks for contributing a PR!

Here's a quick checklist of what we'd like you to think off:

  • Make sure to include one or more tests for your change;
  • [NA] if an enhancement PR please create docs and at best an example
  • Add yourself to CONTRIBUTORS;
  • make a descriptive Pull Request text (it will be used for changelog)

When reading in the config and processing dict content we were splitting
on = assuming we would always have key = value pairs. However in the
case where we don't want to replace because the testenv will not be used
this can result in ValueErrors as substitution variables are not string
key = value pairs.

Avoid all this by only attempting to process dict key = values if we are
replacing in the first place. Otherwise the content isn't needed and we
just return the default value or empty dict.

This should fix tox-dev#595
We also need to avoid trying to parse bool types when not replacing as
they may be unsubstituted variables that are not a valid bool type. Just
don't check them if we are not replacing and use the default value
instead.
@obestwalter obestwalter merged commit 55a72ae into tox-dev:master Sep 2, 2017
@obestwalter
Copy link
Member

Great, thank you! I will fix up whatever is missing and push a bug fix release

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 this pull request may close these issues.

2.8.0: {[testenv]setenv} substitution broken
2 participants