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

Unexpected setenv behavior #137

Closed
pytoxbot opened this Issue Sep 17, 2016 · 5 comments

Comments

2 participants
@pytoxbot

pytoxbot commented Sep 17, 2016

The following behavior was unexpected to me. With an ini section like the following:

[testenv]
setenv = FOO = bar
commands = echo {env:FOO}

The environment variable FOO set via setenv is not reflected in the {env:FOO} substitution. I think the documentation should clarify this because it may be surprising to some. Also, there is a typo here (unkown -> unknown).

@pytoxbot

This comment has been minimized.

Show comment
Hide comment
@pytoxbot

pytoxbot Sep 17, 2016

Original comment by @cjerdonek

I think the trickiest thing will be deciding the rules for what behavior you want. For example, should it be procedurally evaluated (i.e. dependent on the order in which directives are processed), or declarative (i.e. order independent). An example of this is whether it matters if the setenv directives come before or after the substitutions they might affect.

pytoxbot commented Sep 17, 2016

Original comment by @cjerdonek

I think the trickiest thing will be deciding the rules for what behavior you want. For example, should it be procedurally evaluated (i.e. dependent on the order in which directives are processed), or declarative (i.e. order independent). An example of this is whether it matters if the setenv directives come before or after the substitutions they might affect.

@pytoxbot

This comment has been minimized.

Show comment
Hide comment
@pytoxbot

pytoxbot Sep 17, 2016

Original comment by @hpk42

You are right. The implementation might not be a few lines of change.

FYI I am actually unhappy about the "up front" expansion on all configs. It should be more lazy. For example, i once had a specific test environment that depended on an environment variable. The environment was not part of envlist, though. I only wanted to execute it sometimes and would be fine if tox gets back to me saying it needs that env variable. As it stands, however, this doesn't work because all environments need to properly expand up-front even if some of them are not actually needed. I haven't really looked into what is needed to make things more lazy. Hopefully it is a change that can be confined to config.py mostly.

pytoxbot commented Sep 17, 2016

Original comment by @hpk42

You are right. The implementation might not be a few lines of change.

FYI I am actually unhappy about the "up front" expansion on all configs. It should be more lazy. For example, i once had a specific test environment that depended on an environment variable. The environment was not part of envlist, though. I only wanted to execute it sometimes and would be fine if tox gets back to me saying it needs that env variable. As it stands, however, this doesn't work because all environments need to properly expand up-front even if some of them are not actually needed. I haven't really looked into what is needed to make things more lazy. Hopefully it is a change that can be confined to config.py mostly.

@pytoxbot

This comment has been minimized.

Show comment
Hide comment
@pytoxbot

pytoxbot Sep 17, 2016

Original comment by @cjerdonek

Hmm, this might become subtle then, or not so simple to document. If {env:*} needs to respect setenv, then the behavior could get complicated with things like the following (here is a simple case):

setenv =
  FOO = 1000
  BAR = {env:FOO}

Currently, because {env:*} expands to the values at start-up, there is never any ambiguity.

pytoxbot commented Sep 17, 2016

Original comment by @cjerdonek

Hmm, this might become subtle then, or not so simple to document. If {env:*} needs to respect setenv, then the behavior could get complicated with things like the following (here is a simple case):

setenv =
  FOO = 1000
  BAR = {env:FOO}

Currently, because {env:*} expands to the values at start-up, there is never any ambiguity.

@pytoxbot

This comment has been minimized.

Show comment
Hide comment
@pytoxbot

pytoxbot Sep 17, 2016

Original comment by @hpk42

Agreed, this should be fixed, actually. setenv should work for substitutions in commands.

pytoxbot commented Sep 17, 2016

Original comment by @hpk42

Agreed, this should be fixed, actually. setenv should work for substitutions in commands.

@obestwalter obestwalter added the bug label Dec 21, 2016

@obestwalter obestwalter added this to backlog in Squash all the bugs Mar 10, 2017

@obestwalter obestwalter moved this from backlog to ready in Squash all the bugs Mar 10, 2017

@ghost ghost referenced this issue Jul 11, 2017

Merged

Issue #137 test #547

obestwalter added a commit that referenced this issue Jul 11, 2017

Merge pull request #547 from sifuraz/envvars
Verify that Issue #137 is fixed.
@obestwalter

This comment has been minimized.

Show comment
Hide comment
@obestwalter

obestwalter Jul 11, 2017

Member

As the merged test from @sifuraz shows, this bug has been fixed already.

Member

obestwalter commented Jul 11, 2017

As the merged test from @sifuraz shows, this bug has been fixed already.

@obestwalter obestwalter moved this from ready to done in Squash all the bugs Jul 11, 2017

@obestwalter obestwalter added this to closed issues in [released]2.8.x Aug 25, 2017

@obestwalter obestwalter moved this from done to released/wontfix in Squash all the bugs Sep 16, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment