From 505b33d22fca5d49cb3efdb8b7204f3a3cbaaa3e Mon Sep 17 00:00:00 2001 From: Lorenz Mende Date: Sat, 30 Jun 2018 23:55:36 +0200 Subject: [PATCH 1/2] bpo-32942: Fix environment dependent test_script_helper result of function interpreter_requires_environment depends on os.environ - this was not covered by the tests, leading to fail in some environments --- Lib/test/test_script_helper.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/Lib/test/test_script_helper.py b/Lib/test/test_script_helper.py index a7680f886a8d741..9f84084b6947ae0 100644 --- a/Lib/test/test_script_helper.py +++ b/Lib/test/test_script_helper.py @@ -81,6 +81,7 @@ def tearDown(self): # Reset the private cached state. script_helper.__dict__['__cached_interp_requires_environment'] = None + @mock.patch('os.environ', {}) @mock.patch('subprocess.check_call') def test_interpreter_requires_environment_true(self, mock_check_call): mock_check_call.side_effect = subprocess.CalledProcessError('', '') @@ -88,6 +89,7 @@ def test_interpreter_requires_environment_true(self, mock_check_call): self.assertTrue(script_helper.interpreter_requires_environment()) self.assertEqual(1, mock_check_call.call_count) + @mock.patch('os.environ', {}) @mock.patch('subprocess.check_call') def test_interpreter_requires_environment_false(self, mock_check_call): # The mocked subprocess.check_call fakes a no-error process. @@ -95,6 +97,7 @@ def test_interpreter_requires_environment_false(self, mock_check_call): self.assertFalse(script_helper.interpreter_requires_environment()) self.assertEqual(1, mock_check_call.call_count) + @mock.patch('os.environ', {}) @mock.patch('subprocess.check_call') def test_interpreter_requires_environment_details(self, mock_check_call): script_helper.interpreter_requires_environment() @@ -105,6 +108,14 @@ def test_interpreter_requires_environment_details(self, mock_check_call): self.assertEqual(sys.executable, check_call_command[0]) self.assertIn('-E', check_call_command) + @mock.patch('os.environ', {'PYTHONHOME': 'MockedHome'}) + @mock.patch('subprocess.check_call') + def test_interpreter_requires_environment_with_pythonhome(self, mock_check_call): + # There is PYTHONHOME in the environment, expecting no call to check_call + self.assertTrue(script_helper.interpreter_requires_environment()) + self.assertTrue(script_helper.interpreter_requires_environment()) + self.assertEqual(0, mock_check_call.call_count) + if __name__ == '__main__': unittest.main() From 5206d652a4d9d523513fd9f30d129ce475b96f43 Mon Sep 17 00:00:00 2001 From: Lorenz Mende Date: Mon, 2 Jul 2018 21:02:19 +0200 Subject: [PATCH 2/2] bpo-32942: Reworked mocking of os.environ using a modified dict --- Lib/test/test_script_helper.py | 54 ++++++++++++++++++---------------- 1 file changed, 29 insertions(+), 25 deletions(-) diff --git a/Lib/test/test_script_helper.py b/Lib/test/test_script_helper.py index 9f84084b6947ae0..4ade2cbc0d4b184 100644 --- a/Lib/test/test_script_helper.py +++ b/Lib/test/test_script_helper.py @@ -2,6 +2,7 @@ import subprocess import sys +import os from test.support import script_helper import unittest from unittest import mock @@ -73,7 +74,7 @@ class TestScriptHelperEnvironment(unittest.TestCase): def setUp(self): self.assertTrue( - hasattr(script_helper, '__cached_interp_requires_environment')) + hasattr(script_helper, '__cached_interp_requires_environment')) # Reset the private cached state. script_helper.__dict__['__cached_interp_requires_environment'] = None @@ -81,40 +82,43 @@ def tearDown(self): # Reset the private cached state. script_helper.__dict__['__cached_interp_requires_environment'] = None - @mock.patch('os.environ', {}) @mock.patch('subprocess.check_call') def test_interpreter_requires_environment_true(self, mock_check_call): - mock_check_call.side_effect = subprocess.CalledProcessError('', '') - self.assertTrue(script_helper.interpreter_requires_environment()) - self.assertTrue(script_helper.interpreter_requires_environment()) - self.assertEqual(1, mock_check_call.call_count) + with mock.patch.dict(os.environ): + os.environ.pop('PYTHONHOME', None) + mock_check_call.side_effect = subprocess.CalledProcessError('', '') + self.assertTrue(script_helper.interpreter_requires_environment()) + self.assertTrue(script_helper.interpreter_requires_environment()) + self.assertEqual(1, mock_check_call.call_count) - @mock.patch('os.environ', {}) @mock.patch('subprocess.check_call') def test_interpreter_requires_environment_false(self, mock_check_call): - # The mocked subprocess.check_call fakes a no-error process. - script_helper.interpreter_requires_environment() - self.assertFalse(script_helper.interpreter_requires_environment()) - self.assertEqual(1, mock_check_call.call_count) + with mock.patch.dict(os.environ): + os.environ.pop('PYTHONHOME', None) + # The mocked subprocess.check_call fakes a no-error process. + script_helper.interpreter_requires_environment() + self.assertFalse(script_helper.interpreter_requires_environment()) + self.assertEqual(1, mock_check_call.call_count) - @mock.patch('os.environ', {}) @mock.patch('subprocess.check_call') def test_interpreter_requires_environment_details(self, mock_check_call): - script_helper.interpreter_requires_environment() - self.assertFalse(script_helper.interpreter_requires_environment()) - self.assertFalse(script_helper.interpreter_requires_environment()) - self.assertEqual(1, mock_check_call.call_count) - check_call_command = mock_check_call.call_args[0][0] - self.assertEqual(sys.executable, check_call_command[0]) - self.assertIn('-E', check_call_command) - - @mock.patch('os.environ', {'PYTHONHOME': 'MockedHome'}) + with mock.patch.dict(os.environ): + os.environ.pop('PYTHONHOME', None) + script_helper.interpreter_requires_environment() + self.assertFalse(script_helper.interpreter_requires_environment()) + self.assertFalse(script_helper.interpreter_requires_environment()) + self.assertEqual(1, mock_check_call.call_count) + check_call_command = mock_check_call.call_args[0][0] + self.assertEqual(sys.executable, check_call_command[0]) + self.assertIn('-E', check_call_command) + @mock.patch('subprocess.check_call') def test_interpreter_requires_environment_with_pythonhome(self, mock_check_call): - # There is PYTHONHOME in the environment, expecting no call to check_call - self.assertTrue(script_helper.interpreter_requires_environment()) - self.assertTrue(script_helper.interpreter_requires_environment()) - self.assertEqual(0, mock_check_call.call_count) + with mock.patch.dict(os.environ): + os.environ['PYTHONHOME'] = 'MockedHome' + self.assertTrue(script_helper.interpreter_requires_environment()) + self.assertTrue(script_helper.interpreter_requires_environment()) + self.assertEqual(0, mock_check_call.call_count) if __name__ == '__main__':