Issue warning if Monkeypatch.setenv/delenv receive non-strings in Python 2

Fixes the bug described in:

	https://github.com/tox-dev/tox/pull/1025#discussion_r221273830

Which is more evident when using `unicode_literals`.
This commit is contained in:
Bruno Oliveira 2018-09-29 19:55:04 -03:00
parent 5d2d64c190
commit d24a7e6c5a
3 changed files with 44 additions and 0 deletions

View File

@ -0,0 +1,4 @@
``MonkeyPatch.setenv`` and ``MonkeyPatch.delenv`` issue a warning if the environment variable name is not ``str`` on Python 2.
In Python 2, adding ``unicode`` keys to ``os.environ`` causes problems with ``subprocess`` (and possible other modules),
making this a subtle bug specially susceptible when used with ``from __future__ import unicode_literals``.

View File

@ -4,9 +4,12 @@ from __future__ import absolute_import, division, print_function
import os import os
import sys import sys
import re import re
import warnings
from contextlib import contextmanager from contextlib import contextmanager
import six import six
import pytest
from _pytest.fixtures import fixture from _pytest.fixtures import fixture
RE_IMPORT_ERROR_NAME = re.compile("^No module named (.*)$") RE_IMPORT_ERROR_NAME = re.compile("^No module named (.*)$")
@ -209,6 +212,15 @@ class MonkeyPatch(object):
self._setitem.append((dic, name, dic.get(name, notset))) self._setitem.append((dic, name, dic.get(name, notset)))
del dic[name] del dic[name]
def _warn_if_env_name_is_not_str(self, name):
"""On Python 2, warn if the given environment variable name is not a native str (#4056)"""
if six.PY2 and not isinstance(name, str):
warnings.warn(
pytest.PytestWarning(
"Environment variable name {!r} should be str".format(name)
)
)
def setenv(self, name, value, prepend=None): def setenv(self, name, value, prepend=None):
""" Set environment variable ``name`` to ``value``. If ``prepend`` """ Set environment variable ``name`` to ``value``. If ``prepend``
is a character, read the current environment variable value is a character, read the current environment variable value
@ -216,6 +228,7 @@ class MonkeyPatch(object):
value = str(value) value = str(value)
if prepend and name in os.environ: if prepend and name in os.environ:
value = value + prepend + os.environ[name] value = value + prepend + os.environ[name]
self._warn_if_env_name_is_not_str(name)
self.setitem(os.environ, name, value) self.setitem(os.environ, name, value)
def delenv(self, name, raising=True): def delenv(self, name, raising=True):
@ -225,6 +238,7 @@ class MonkeyPatch(object):
If ``raising`` is set to False, no exception will be raised if the If ``raising`` is set to False, no exception will be raised if the
environment variable is missing. environment variable is missing.
""" """
self._warn_if_env_name_is_not_str(name)
self.delitem(os.environ, name, raising=raising) self.delitem(os.environ, name, raising=raising)
def syspath_prepend(self, path): def syspath_prepend(self, path):

View File

@ -3,6 +3,8 @@ import os
import sys import sys
import textwrap import textwrap
import six
import pytest import pytest
from _pytest.monkeypatch import MonkeyPatch from _pytest.monkeypatch import MonkeyPatch
@ -192,6 +194,30 @@ def test_delenv():
del os.environ[name] del os.environ[name]
@pytest.mark.skipif(six.PY3, reason="Python 2 only test")
class TestEnvironKeysWarning(object):
"""
os.environ needs keys to be native strings, otherwise it will cause problems with other modules (notably
subprocess). We only test this behavior on Python 2, because Python 3 raises an error right away.
"""
VAR_NAME = u"PYTEST_INTERNAL_MY_VAR"
def test_setenv_unicode_key(self, monkeypatch):
with pytest.warns(
pytest.PytestWarning,
match="Environment variable name {!r} should be str".format(self.VAR_NAME),
):
monkeypatch.setenv(self.VAR_NAME, "2")
def test_delenv_unicode_key(self, monkeypatch):
with pytest.warns(
pytest.PytestWarning,
match="Environment variable name {!r} should be str".format(self.VAR_NAME),
):
monkeypatch.delenv(self.VAR_NAME, raising=False)
def test_setenv_prepend(): def test_setenv_prepend():
import os import os