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

passenv=FOO passes on foo #1534

Closed
blueyed opened this issue Mar 4, 2020 · 6 comments · Fixed by #1723
Closed

passenv=FOO passes on foo #1534

blueyed opened this issue Mar 4, 2020 · 6 comments · Fixed by #1723
Labels
bug:normal affects many people or has quite an impact

Comments

@blueyed
Copy link

blueyed commented Mar 4, 2020

When using passenv = FOO it willl pass-through foo.

Code ref: https://github.com/blueyed/tox/blob/2cfc0b218acec5cc8b2ff07e4351c72f4945c30e/src/tox/config/__init__.py#L720-L723

@blueyed blueyed added the bug:normal affects many people or has quite an impact label Mar 4, 2020
@gaborbernat
Copy link
Member

Feel free to open a PR against master and fix it. My available efforts at the moment are aimed at fixing this as part of #1394, but that probably will take a while (ETA September).

@gnikonorov
Copy link
Contributor

Hey @gaborbernat, could I submit a pr for this?

@gaborbernat
Copy link
Member

Sure 👍

@gnikonorov
Copy link
Contributor

To elaborate on the description, it seems that the value passed to envlist in the tox.ini file will take all environment variables that match, regardless of the case of the letters.

Environment variables:

gnikonorov:~/temp/test_tox$ echo $FOO
big foo
gnikonorov:~/temp/test_tox$ echo $foo
little foo
gnikonorov:~/temp/test_tox$ echo $fOo
mixed case foo
gnikonorov:~/temp/test_tox$

tox.ini

gnikonorov:~/temp/test_tox$ cat tox.ini
[tox]
envlist = py36
isolated_build = True

[testenv]
passenv = foo
commands = pytest --capture=no test_this.py
gnikonorov:~/temp/test_tox$

sample test file:

gnikonorov:~/temp/test_tox$ cat test_this.py
import os

def test_get_foo():
    print(os.environ.get("FOO", "FOO variable not found"))
    print(os.environ.get("fOo", "fOo variable not found"))
    print(os.environ.get("foo", "foo variable not found"))
gnikonorov:~/temp/test_tox$

Test run

py37 run-test: commands[0] | pytest --capture=no test_this.py
...
=========================================================================================================== test session starts ============================================================================================================
platform linux -- Python 3.6.9, pytest-6.0.1, py-1.9.0, pluggy-0.13.1
cachedir: .tox/py37/.pytest_cache
rootdir: /home/gnikonorov/temp/test_tox
plugins: dynamicrerun-1.0.4
collected 1 item

test_this.py big foo
mixed case foo
little foo
.

...
_________________________________________________________________________________________________________________ summary __________________________________________________________________________________________________________________
  py37: commands succeeded
  congratulations :)
gnikonorov:~/temp/test_tox$

@jayvdb
Copy link

jayvdb commented Oct 26, 2020

I have a slight preference for it remaining case insensitive on all platforms, as platform neutral behaviour is a feature of tox, so projects may have build scripts and published docs which rely on the current behaviour.

Also worth noting that we dont have info about other platforms which tox may be used on.

IMO it would be useful to emit a warning if the environment name has a different case to the casing used in tox.ini. Possibly the warning could note the behaviour of case insensitive matching is deprecated.

@gnikonorov
Copy link
Contributor

gnikonorov commented Oct 27, 2020

Noting that as per #1718 we will not change behavior, but instead document the current behavior.

@tox-dev tox-dev locked and limited conversation to collaborators Jan 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug:normal affects many people or has quite an impact
Projects
None yet
4 participants