From 5588db0bd36f43d74d6c226bbd1d055551433b7e Mon Sep 17 00:00:00 2001 From: Anirudha Bose Date: Sat, 28 Jul 2018 13:17:48 +0200 Subject: [PATCH 1/6] bpo-33747: Avoid mutating the global sys.modules dict The test method test_patch_imports_lazily was injecting a mocked module to the sys.modules dict, which was reused in another test module called test_patch_propogrates_exc_on_exit. Hence, the other test module failed whenever it was launched independently. Implemented a context manager to make sure modifications to sys.modules are not persisted after the test methods finish execution. --- Lib/unittest/test/testmock/testpatch.py | 34 ++++++++++++++----------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/Lib/unittest/test/testmock/testpatch.py b/Lib/unittest/test/testmock/testpatch.py index fe4ecefd44058a8..0105c52e88f8819 100644 --- a/Lib/unittest/test/testmock/testpatch.py +++ b/Lib/unittest/test/testmock/testpatch.py @@ -2,19 +2,19 @@ # E-mail: fuzzyman AT voidspace DOT org DOT uk # http://www.voidspace.org.uk/python/mock/ +import contextlib import os import sys - import unittest -from unittest.test.testmock import support -from unittest.test.testmock.support import SomeClass, is_instance +from test.support import swap_attr from unittest.mock import ( NonCallableMock, CallableMixin, sentinel, MagicMock, Mock, NonCallableMagicMock, patch, _patch, DEFAULT, call, _get_target ) - +from unittest.test.testmock import support +from unittest.test.testmock.support import SomeClass, is_instance builtin_string = 'builtins' @@ -1658,23 +1658,25 @@ def test_mock_calls_with_patch(self): finally: p.stop() + @contextlib.contextmanager + def patch_sys_modules(self): + with swap_attr(sys, 'modules', sys.modules.copy()): + squizz = Mock() + sys.modules['squizz'] = squizz + yield squizz def test_patch_imports_lazily(self): - sys.modules.pop('squizz', None) - p1 = patch('squizz.squozz') self.assertRaises(ImportError, p1.start) - squizz = Mock() - squizz.squozz = 6 - sys.modules['squizz'] = squizz - p1 = patch('squizz.squozz') - squizz.squozz = 3 - p1.start() - p1.stop() + with self.patch_sys_modules() as squizz: + squizz.squozz = 6 + p1 = patch('squizz.squozz') + squizz.squozz = 3 + p1.start() + p1.stop() self.assertEqual(squizz.squozz, 3) - def test_patch_propogrates_exc_on_exit(self): class holder: exc_info = None, None, None @@ -1696,7 +1698,9 @@ def with_custom_patch(target): def test(mock): raise RuntimeError - self.assertRaises(RuntimeError, test) + with self.patch_sys_modules(): + self.assertRaises(RuntimeError, test) + self.assertIs(holder.exc_info[0], RuntimeError) self.assertIsNotNone(holder.exc_info[1], 'exception value not propgated') From db7a7ec2bb19a052a6bda0741937bc3375a53fe0 Mon Sep 17 00:00:00 2001 From: Anirudha Bose Date: Sat, 28 Jul 2018 16:40:21 +0200 Subject: [PATCH 2/6] Remove usage of a separate context manager for restoring sys.modules --- Lib/unittest/test/testmock/testpatch.py | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/Lib/unittest/test/testmock/testpatch.py b/Lib/unittest/test/testmock/testpatch.py index 0105c52e88f8819..f1575b9cc89d701 100644 --- a/Lib/unittest/test/testmock/testpatch.py +++ b/Lib/unittest/test/testmock/testpatch.py @@ -1658,18 +1658,14 @@ def test_mock_calls_with_patch(self): finally: p.stop() - @contextlib.contextmanager - def patch_sys_modules(self): - with swap_attr(sys, 'modules', sys.modules.copy()): - squizz = Mock() - sys.modules['squizz'] = squizz - yield squizz - def test_patch_imports_lazily(self): p1 = patch('squizz.squozz') self.assertRaises(ImportError, p1.start) - with self.patch_sys_modules() as squizz: + with swap_attr(sys, 'modules', sys.modules.copy()): + squizz = Mock() + sys.modules['squizz'] = squizz + squizz.squozz = 6 p1 = patch('squizz.squozz') squizz.squozz = 3 @@ -1698,7 +1694,10 @@ def with_custom_patch(target): def test(mock): raise RuntimeError - with self.patch_sys_modules(): + with swap_attr(sys, 'modules', sys.modules.copy()): + squizz = Mock() + sys.modules['squizz'] = squizz + self.assertRaises(RuntimeError, test) self.assertIs(holder.exc_info[0], RuntimeError) From 2d0dcb23876dffa7fb077f30972ee24c8416adea Mon Sep 17 00:00:00 2001 From: Anirudha Bose Date: Sat, 28 Jul 2018 16:40:57 +0200 Subject: [PATCH 3/6] Remove unused import --- Lib/unittest/test/testmock/testpatch.py | 1 - 1 file changed, 1 deletion(-) diff --git a/Lib/unittest/test/testmock/testpatch.py b/Lib/unittest/test/testmock/testpatch.py index f1575b9cc89d701..07f2ea4c74293c7 100644 --- a/Lib/unittest/test/testmock/testpatch.py +++ b/Lib/unittest/test/testmock/testpatch.py @@ -2,7 +2,6 @@ # E-mail: fuzzyman AT voidspace DOT org DOT uk # http://www.voidspace.org.uk/python/mock/ -import contextlib import os import sys import unittest From 4ea8da55177b3096668b70e02c80f738d04c51e5 Mon Sep 17 00:00:00 2001 From: Anirudha Bose Date: Sat, 28 Jul 2018 16:43:47 +0200 Subject: [PATCH 4/6] Revert PEP8 changes --- Lib/unittest/test/testmock/testpatch.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/Lib/unittest/test/testmock/testpatch.py b/Lib/unittest/test/testmock/testpatch.py index 07f2ea4c74293c7..41bfd21ec28e1e3 100644 --- a/Lib/unittest/test/testmock/testpatch.py +++ b/Lib/unittest/test/testmock/testpatch.py @@ -4,7 +4,10 @@ import os import sys + import unittest +from unittest.test.testmock import support +from unittest.test.testmock.support import SomeClass, is_instance from test.support import swap_attr from unittest.mock import ( @@ -12,8 +15,7 @@ MagicMock, Mock, NonCallableMagicMock, patch, _patch, DEFAULT, call, _get_target ) -from unittest.test.testmock import support -from unittest.test.testmock.support import SomeClass, is_instance + builtin_string = 'builtins' @@ -1657,6 +1659,7 @@ def test_mock_calls_with_patch(self): finally: p.stop() + def test_patch_imports_lazily(self): p1 = patch('squizz.squozz') self.assertRaises(ImportError, p1.start) From 300614e8bb42471133ded4a0b7d0775cbb8a76a9 Mon Sep 17 00:00:00 2001 From: Anirudha Bose Date: Sun, 29 Jul 2018 12:27:47 +0200 Subject: [PATCH 5/6] Use sys_modules_context instead of swap_attr to avoid modifying the reference of sys.modules --- Lib/unittest/test/testmock/testpatch.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Lib/unittest/test/testmock/testpatch.py b/Lib/unittest/test/testmock/testpatch.py index 41bfd21ec28e1e3..37b9d08a0299d85 100644 --- a/Lib/unittest/test/testmock/testpatch.py +++ b/Lib/unittest/test/testmock/testpatch.py @@ -9,7 +9,7 @@ from unittest.test.testmock import support from unittest.test.testmock.support import SomeClass, is_instance -from test.support import swap_attr +from test.test_importlib.test_namespace_pkgs import sys_modules_context from unittest.mock import ( NonCallableMock, CallableMixin, sentinel, MagicMock, Mock, NonCallableMagicMock, patch, _patch, @@ -1664,7 +1664,7 @@ def test_patch_imports_lazily(self): p1 = patch('squizz.squozz') self.assertRaises(ImportError, p1.start) - with swap_attr(sys, 'modules', sys.modules.copy()): + with sys_modules_context(): squizz = Mock() sys.modules['squizz'] = squizz @@ -1696,7 +1696,7 @@ def with_custom_patch(target): def test(mock): raise RuntimeError - with swap_attr(sys, 'modules', sys.modules.copy()): + with sys_modules_context(): squizz = Mock() sys.modules['squizz'] = squizz From fce4126ceb05c2489962b6e43ec14ddf93cecd7c Mon Sep 17 00:00:00 2001 From: Anirudha Bose Date: Thu, 6 Dec 2018 17:23:57 +0100 Subject: [PATCH 6/6] Replace usage of sys_modules_context in testpatch.py with uncache --- Lib/unittest/test/testmock/testpatch.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Lib/unittest/test/testmock/testpatch.py b/Lib/unittest/test/testmock/testpatch.py index 37b9d08a0299d85..f05225730dafb29 100644 --- a/Lib/unittest/test/testmock/testpatch.py +++ b/Lib/unittest/test/testmock/testpatch.py @@ -9,7 +9,7 @@ from unittest.test.testmock import support from unittest.test.testmock.support import SomeClass, is_instance -from test.test_importlib.test_namespace_pkgs import sys_modules_context +from test.test_importlib.util import uncache from unittest.mock import ( NonCallableMock, CallableMixin, sentinel, MagicMock, Mock, NonCallableMagicMock, patch, _patch, @@ -1664,7 +1664,7 @@ def test_patch_imports_lazily(self): p1 = patch('squizz.squozz') self.assertRaises(ImportError, p1.start) - with sys_modules_context(): + with uncache('squizz'): squizz = Mock() sys.modules['squizz'] = squizz @@ -1696,7 +1696,7 @@ def with_custom_patch(target): def test(mock): raise RuntimeError - with sys_modules_context(): + with uncache('squizz'): squizz = Mock() sys.modules['squizz'] = squizz