Merge pull request #1339 from RonnyPfannschmidt/fix-1338

Fix 1338

CHANGELOG

@@ -1,6 +1,7 @@
 2.8.7.dev1
 ----------
 
+- fix #1338: use predictable object resolution for monkeypatch'
 
 2.8.6
 -----

_pytest/monkeypatch.py

@@ -5,7 +5,6 @@ import re
 
 from py.builtin import _basestring
 
-
 RE_IMPORT_ERROR_NAME = re.compile("^No module named (.*)$")
 
 
@@ -32,62 +31,71 @@ def pytest_funcarg__monkeypatch(request):
     return mpatch
 
 
+def resolve(name):
+    # simplified from zope.dottedname
+    parts = name.split('.')
+
+    used = parts.pop(0)
+    found = __import__(used)
+    for part in parts:
+        used += '.' + part
+        try:
+            found = getattr(found, part)
+        except AttributeError:
+            pass
+        else:
+            continue
+        # we use explicit un-nesting of the handling block in order
+        # to avoid nested exceptions on python 3
+        try:
+            __import__(used)
+        except ImportError as ex:
+            # str is used for py2 vs py3
+            expected = str(ex).split()[-1]
+            if expected == used:
+                raise
+            else:
+                raise ImportError(
+                    'import error in %s: %s' % (used, ex)
+                )
+        found = annotated_getattr(found, part, used)
+    return found
+
+
+def annotated_getattr(obj, name, ann):
+    try:
+        obj = getattr(obj, name)
+    except AttributeError:
+        raise AttributeError(
+                '%r object at %s has no attribute %r' % (
+                    type(obj).__name__, ann, name
+                )
+        )
+    return obj
+
 
 def derive_importpath(import_path, raising):
-    import pytest
     if not isinstance(import_path, _basestring) or "." not in import_path:
         raise TypeError("must be absolute import path string, not %r" %
                         (import_path,))
-    rest = []
+    module, attr = import_path.rsplit('.', 1)
-    target = import_path
+    target = resolve(module)
-    target_parts = set(target.split("."))
+    if raising:
-    while target:
+        annotated_getattr(target, attr, ann=module)
-        try:
+    return attr, target
-            obj = __import__(target, None, None, "__doc__")
-        except ImportError as ex:
-            if hasattr(ex, 'name'):
-                # Python >= 3.3
-                failed_name = ex.name
-            else:
-                match = RE_IMPORT_ERROR_NAME.match(ex.args[0])
-                assert match
-                failed_name = match.group(1)
-
-            if "." not in target:
-                __tracebackhide__ = True
-                pytest.fail("could not import any sub part: %s" %
-                            import_path)
-            elif failed_name != target \
-                    and not any(p == failed_name for p in target_parts):
-                # target is importable but causes ImportError itself
-                __tracebackhide__ = True
-                pytest.fail("import error in %s: %s" % (target, ex.args[0]))
-            target, name = target.rsplit(".", 1)
-            rest.append(name)
-        else:
-            assert rest
-            try:
-                while len(rest) > 1:
-                    attr = rest.pop()
-                    obj = getattr(obj, attr)
-                attr = rest[0]
-                if raising:
-                    getattr(obj, attr)
-            except AttributeError:
-                __tracebackhide__ = True
-                pytest.fail("object %r has no attribute %r" % (obj, attr))
-            return attr, obj
-
 
 
 class Notset:
     def __repr__(self):
         return "<notset>"
 
+
 notset = Notset()
 
+
 class monkeypatch:
     """ Object keeping a record of setattr/item/env/syspath changes. """
+
     def __init__(self):
         self._setattr = []
         self._setitem = []
@@ -114,14 +122,14 @@ class monkeypatch:
         if value is notset:
             if not isinstance(target, _basestring):
                 raise TypeError("use setattr(target, name, value) or "
-                   "setattr(target, value) with target being a dotted "
+                                "setattr(target, value) with target being a dotted "
-                   "import string")
+                                "import string")
             value = name
             name, target = derive_importpath(target, raising)
 
         oldval = getattr(target, name, notset)
         if raising and oldval is notset:
-            raise AttributeError("%r has no attribute %r" %(target, name))
+            raise AttributeError("%r has no attribute %r" % (target, name))
 
         # avoid class descriptors like staticmethod/classmethod
         if inspect.isclass(target):
@@ -233,7 +241,7 @@ class monkeypatch:
                 try:
                     del dictionary[name]
                 except KeyError:
-                    pass # was already deleted, so we have the desired state
+                    pass  # was already deleted, so we have the desired state
             else:
                 dictionary[name] = value
         self._setitem[:] = []

testing/test_monkeypatch.py

@@ -1,9 +1,11 @@
-import os, sys
+import os
+import sys
 import textwrap
 
 import pytest
 from _pytest.monkeypatch import monkeypatch as MonkeyPatch
 
+
 def pytest_funcarg__mp(request):
     cwd = os.getcwd()
     sys_path = list(sys.path)
@@ -15,9 +17,11 @@ def pytest_funcarg__mp(request):
     request.addfinalizer(cleanup)
     return MonkeyPatch()
 
+
 def test_setattr():
     class A:
         x = 1
+
     monkeypatch = MonkeyPatch()
     pytest.raises(AttributeError, "monkeypatch.setattr(A, 'notexists', 2)")
     monkeypatch.setattr(A, 'y', 2, raising=False)
@@ -34,9 +38,10 @@ def test_setattr():
     assert A.x == 1
 
     A.x = 5
-    monkeypatch.undo() # double-undo makes no modification
+    monkeypatch.undo()  # double-undo makes no modification
     assert A.x == 5
 
+
 class TestSetattrWithImportPath:
     def test_string_expression(self, monkeypatch):
         monkeypatch.setattr("os.path.abspath", lambda x: "hello2")
@@ -57,11 +62,11 @@ class TestSetattrWithImportPath:
         pytest.raises(TypeError, lambda: monkeypatch.setattr(None, None))
 
     def test_unknown_import(self, monkeypatch):
-        pytest.raises(pytest.fail.Exception,
+        pytest.raises(ImportError,
                       lambda: monkeypatch.setattr("unkn123.classx", None))
 
     def test_unknown_attr(self, monkeypatch):
-        pytest.raises(pytest.fail.Exception,
+        pytest.raises(AttributeError,
                       lambda: monkeypatch.setattr("os.path.qweqwe", None))
 
     def test_unknown_attr_non_raising(self, monkeypatch):
@@ -75,9 +80,11 @@ class TestSetattrWithImportPath:
         monkeypatch.undo()
         assert os.path.abspath
 
+
 def test_delattr():
     class A:
         x = 1
+
     monkeypatch = MonkeyPatch()
     monkeypatch.delattr(A, 'x')
     assert not hasattr(A, 'x')
@@ -93,6 +100,7 @@ def test_delattr():
     monkeypatch.undo()
     assert A.x == 1
 
+
 def test_setitem():
     d = {'x': 1}
     monkeypatch = MonkeyPatch()
@@ -110,6 +118,7 @@ def test_setitem():
     monkeypatch.undo()
     assert d['x'] == 5
 
+
 def test_setitem_deleted_meanwhile():
     d = {}
     monkeypatch = MonkeyPatch()
@@ -118,6 +127,7 @@ def test_setitem_deleted_meanwhile():
     monkeypatch.undo()
     assert not d
 
+
 @pytest.mark.parametrize("before", [True, False])
 def test_setenv_deleted_meanwhile(before):
     key = "qwpeoip123"
@@ -133,6 +143,7 @@ def test_setenv_deleted_meanwhile(before):
     else:
         assert key not in os.environ
 
+
 def test_delitem():
     d = {'x': 1}
     monkeypatch = MonkeyPatch()
@@ -149,6 +160,7 @@ def test_delitem():
     monkeypatch.undo()
     assert d == {'hello': 'world', 'x': 1}
 
+
 def test_setenv():
     monkeypatch = MonkeyPatch()
     monkeypatch.setenv('XYZ123', 2)
@@ -157,6 +169,7 @@ def test_setenv():
     monkeypatch.undo()
     assert 'XYZ123' not in os.environ
 
+
 def test_delenv():
     name = 'xyz1234'
     assert name not in os.environ
@@ -177,6 +190,7 @@ def test_delenv():
         if name in os.environ:
             del os.environ[name]
 
+
 def test_setenv_prepend():
     import os
     monkeypatch = MonkeyPatch()
@@ -187,6 +201,7 @@ def test_setenv_prepend():
     monkeypatch.undo()
     assert 'XYZ123' not in os.environ
 
+
 def test_monkeypatch_plugin(testdir):
     reprec = testdir.inline_runsource("""
         def test_method(monkeypatch):
@@ -195,6 +210,7 @@ def test_monkeypatch_plugin(testdir):
     res = reprec.countoutcomes()
     assert tuple(res) == (1, 0, 0), res
 
+
 def test_syspath_prepend(mp):
     old = list(sys.path)
     mp.syspath_prepend('world')
@@ -206,6 +222,7 @@ def test_syspath_prepend(mp):
     mp.undo()
     assert sys.path == old
 
+
 def test_syspath_prepend_double_undo(mp):
     mp.syspath_prepend('hello world')
     mp.undo()
@@ -213,20 +230,24 @@ def test_syspath_prepend_double_undo(mp):
     mp.undo()
     assert sys.path[-1] == 'more hello world'
 
+
 def test_chdir_with_path_local(mp, tmpdir):
     mp.chdir(tmpdir)
     assert os.getcwd() == tmpdir.strpath
 
+
 def test_chdir_with_str(mp, tmpdir):
     mp.chdir(tmpdir.strpath)
     assert os.getcwd() == tmpdir.strpath
 
+
 def test_chdir_undo(mp, tmpdir):
     cwd = os.getcwd()
     mp.chdir(tmpdir)
     mp.undo()
     assert os.getcwd() == cwd
 
+
 def test_chdir_double_undo(mp, tmpdir):
     mp.chdir(tmpdir.strpath)
     mp.undo()
@@ -234,6 +255,7 @@ def test_chdir_double_undo(mp, tmpdir):
     mp.undo()
     assert os.getcwd() == tmpdir.strpath
 
+
 def test_issue185_time_breaks(testdir):
     testdir.makepyfile("""
         import time
@@ -247,6 +269,7 @@ def test_issue185_time_breaks(testdir):
         *1 passed*
     """)
 
+
 def test_importerror(testdir):
     p = testdir.mkpydir("package")
     p.join("a.py").write(textwrap.dedent("""\
@@ -260,7 +283,7 @@ def test_importerror(testdir):
     """))
     result = testdir.runpytest()
     result.stdout.fnmatch_lines("""
-        *import error in package.a.x: No module named {0}doesnotexist{0}*
+        *import error in package.a: No module named {0}doesnotexist{0}*
     """.format("'" if sys.version_info > (3, 0) else ""))
 
 
@@ -275,11 +298,12 @@ class SampleNewInherit(SampleNew):
 
 
 class SampleOld:
-    #oldstyle on python2
+    # oldstyle on python2
     @staticmethod
     def hello():
         return True
 
+
 class SampleOldInherit(SampleOld):
     pass
 
@@ -297,4 +321,10 @@ def test_issue156_undo_staticmethod(Sample):
     monkeypatch.undo()
     assert Sample.hello()
 
-
+def test_issue1338_name_resolving():
+    pytest.importorskip('requests')
+    monkeypatch = MonkeyPatch()
+    try:
+         monkeypatch.delattr('requests.sessions.Session.request')
+    finally:
+        monkeypatch.undo()

tox.ini

@@ -12,6 +12,7 @@ passenv = USER USERNAME
 deps=
     nose
     mock
+    requests
 
 [testenv:py26]
 commands= py.test --lsof -rfsxX {posargs:testing}