From d24a7e6c5a415ece9ad377d13b20daefc944fdd4 Mon Sep 17 00:00:00 2001
From: Bruno Oliveira <nicoddemus@gmail.com>
Date: Sat, 29 Sep 2018 19:55:04 -0300
Subject: [PATCH] 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`.
---
 changelog/4056.bugfix.rst   |  4 ++++
 src/_pytest/monkeypatch.py  | 14 ++++++++++++++
 testing/test_monkeypatch.py | 26 ++++++++++++++++++++++++++
 3 files changed, 44 insertions(+)
 create mode 100644 changelog/4056.bugfix.rst

diff --git a/changelog/4056.bugfix.rst b/changelog/4056.bugfix.rst
new file mode 100644
index 000000000..f018a4776
--- /dev/null
+++ b/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``.
diff --git a/src/_pytest/monkeypatch.py b/src/_pytest/monkeypatch.py
index 67279003f..60feb0802 100644
--- a/src/_pytest/monkeypatch.py
+++ b/src/_pytest/monkeypatch.py
@@ -4,9 +4,12 @@ from __future__ import absolute_import, division, print_function
 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,6 +212,15 @@ class MonkeyPatch(object):
             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
@@ -216,6 +228,7 @@ class MonkeyPatch(object):
         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 @@ class MonkeyPatch(object):
         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):
diff --git a/testing/test_monkeypatch.py b/testing/test_monkeypatch.py
index adf10e4d0..f0f63023d 100644
--- a/testing/test_monkeypatch.py
+++ b/testing/test_monkeypatch.py
@@ -3,6 +3,8 @@ import os
 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