Issue warning if Monkeypatch.setenv/delenv receive non-strings in Pyt…

…hon 2

Fixes the bug described in:

	tox-dev/tox#1025 (comment)

Which is more evident when using `unicode_literals`.

changelog/4056.bugfix.rst

@@ -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``.

src/_pytest/monkeypatch.py

@@ -4,9 +4,12 @@
 import os
 import sys
 import re
+import warnings
 from contextlib import contextmanager
 
 import six
+
+import pytest
 from _pytest.fixtures import fixture
 
 RE_IMPORT_ERROR_NAME = re.compile("^No module named (.*)$")
@@ -209,13 +212,23 @@ def delitem(self, dic, name, raising=True):
             self._setitem.append((dic, name, dic.get(name, notset)))
             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):
         """ Set environment variable ``name`` to ``value``.  If ``prepend``
         is a character, read the current environment variable value
         and prepend the ``value`` adjoined with the ``prepend`` character."""
         value = str(value)
         if prepend and name in os.environ:
             value = value + prepend + os.environ[name]
+        self._warn_if_env_name_is_not_str(name)
         self.setitem(os.environ, name, value)
 
     def delenv(self, name, raising=True):
@@ -225,6 +238,7 @@ def delenv(self, name, raising=True):
         If ``raising`` is set to False, no exception will be raised if the
         environment variable is missing.
         """
+        self._warn_if_env_name_is_not_str(name)
         self.delitem(os.environ, name, raising=raising)
 
     def syspath_prepend(self, path):

testing/test_monkeypatch.py

@@ -3,6 +3,8 @@
 import sys
 import textwrap
 
+import six
+
 import pytest
 from _pytest.monkeypatch import MonkeyPatch
 
@@ -192,6 +194,30 @@ def test_delenv():
             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():
     import os