commit 85c81eea7fdad016384e734791cccf0e97132fc1 Author: Damian Johnson <atagar@torproject.org> Date: Sat Jan 4 15:25:14 2020 -0800 Replace mkdtemp() with TemporaryDirectory() Minor python 3.x simplification now that we can use this builtin. --- stem/descriptor/collector.py | 13 ++--- stem/manual.py | 9 ++-- test/integ/control/controller.py | 105 +++++++++++++++++++-------------------- test/integ/process.py | 31 ++++-------- test/integ/util/system.py | 11 ++-- test/unit/manual.py | 16 +++--- 6 files changed, 82 insertions(+), 103 deletions(-) diff --git a/stem/descriptor/collector.py b/stem/descriptor/collector.py index c2bf8179..7aeb298b 100644 --- a/stem/descriptor/collector.py +++ b/stem/descriptor/collector.py @@ -55,7 +55,6 @@ import hashlib import json import os import re -import shutil import tempfile import time @@ -264,15 +263,9 @@ class File(object): if self._downloaded_to and os.path.exists(self._downloaded_to): directory = os.path.dirname(self._downloaded_to) else: - # TODO: The following can be replaced with simpler usage of - # tempfile.TemporaryDirectory when we drop python 2.x support. - - tmp_directory = tempfile.mkdtemp() - - for desc in self.read(tmp_directory, descriptor_type, start, end, document_handler, timeout, retries): - yield desc - - shutil.rmtree(tmp_directory) + with tempfile.TemporaryDirectory() as tmp_directory: + for desc in self.read(tmp_directory, descriptor_type, start, end, document_handler, timeout, retries): + yield desc return diff --git a/stem/manual.py b/stem/manual.py index fe49e3de..c1b4bd8f 100644 --- a/stem/manual.py +++ b/stem/manual.py @@ -288,11 +288,10 @@ def download_man_page(path = None, file_handle = None, url = GITWEB_MANUAL_URL, elif not stem.util.system.is_available('a2x'): raise IOError('We require a2x from asciidoc to provide a man page') - dirpath = tempfile.mkdtemp() - asciidoc_path = os.path.join(dirpath, 'tor.1.txt') - manual_path = os.path.join(dirpath, 'tor.1') + with tempfile.TemporaryDirectory() as dirpath: + asciidoc_path = os.path.join(dirpath, 'tor.1.txt') + manual_path = os.path.join(dirpath, 'tor.1') - try: try: with open(asciidoc_path, 'wb') as asciidoc_file: request = urllib.request.urlopen(url, timeout = timeout) @@ -325,8 +324,6 @@ def download_man_page(path = None, file_handle = None, url = GITWEB_MANUAL_URL, with open(manual_path, 'rb') as manual_file: shutil.copyfileobj(manual_file, file_handle) file_handle.flush() - finally: - shutil.rmtree(dirpath) class Manual(object): diff --git a/test/integ/control/controller.py b/test/integ/control/controller.py index f87c0817..257d9fbc 100644 --- a/test/integ/control/controller.py +++ b/test/integ/control/controller.py @@ -754,71 +754,70 @@ class TestController(unittest.TestCase): """ runner = test.runner.get_runner() - tmpdir = tempfile.mkdtemp() - with runner.get_tor_controller() as controller: - try: - # successfully set a single option - connlimit = int(controller.get_conf('ConnLimit')) - controller.set_conf('connlimit', str(connlimit - 1)) - self.assertEqual(connlimit - 1, int(controller.get_conf('ConnLimit'))) - - # successfully set a single list option - exit_policy = ['accept *:7777', 'reject *:*'] - controller.set_conf('ExitPolicy', exit_policy) - self.assertEqual(exit_policy, controller.get_conf('ExitPolicy', multiple = True)) + with tempfile.TemporaryDirectory() as tmpdir: - # fail to set a single option + with runner.get_tor_controller() as controller: try: - controller.set_conf('invalidkeyboo', 'abcde') - self.fail() - except stem.InvalidArguments as exc: - self.assertEqual(['invalidkeyboo'], exc.arguments) + # successfully set a single option + connlimit = int(controller.get_conf('ConnLimit')) + controller.set_conf('connlimit', str(connlimit - 1)) + self.assertEqual(connlimit - 1, int(controller.get_conf('ConnLimit'))) - # resets configuration parameters - controller.reset_conf('ConnLimit', 'ExitPolicy') - self.assertEqual(connlimit, int(controller.get_conf('ConnLimit'))) - self.assertEqual(None, controller.get_conf('ExitPolicy')) + # successfully set a single list option + exit_policy = ['accept *:7777', 'reject *:*'] + controller.set_conf('ExitPolicy', exit_policy) + self.assertEqual(exit_policy, controller.get_conf('ExitPolicy', multiple = True)) - # successfully sets multiple config options - controller.set_options({ - 'connlimit': str(connlimit - 2), - 'contactinfo': 'stem@testing', - }) + # fail to set a single option + try: + controller.set_conf('invalidkeyboo', 'abcde') + self.fail() + except stem.InvalidArguments as exc: + self.assertEqual(['invalidkeyboo'], exc.arguments) - self.assertEqual(connlimit - 2, int(controller.get_conf('ConnLimit'))) - self.assertEqual('stem@testing', controller.get_conf('contactinfo')) + # resets configuration parameters + controller.reset_conf('ConnLimit', 'ExitPolicy') + self.assertEqual(connlimit, int(controller.get_conf('ConnLimit'))) + self.assertEqual(None, controller.get_conf('ExitPolicy')) - # fail to set multiple config options - try: + # successfully sets multiple config options controller.set_options({ + 'connlimit': str(connlimit - 2), 'contactinfo': 'stem@testing', - 'bombay': 'vadapav', }) - self.fail() - except stem.InvalidArguments as exc: - self.assertEqual(['bombay'], exc.arguments) - # context-sensitive keys (the only retched things for which order matters) - controller.set_options(( - ('HiddenServiceDir', tmpdir), - ('HiddenServicePort', '17234 127.0.0.1:17235'), - )) + self.assertEqual(connlimit - 2, int(controller.get_conf('ConnLimit'))) + self.assertEqual('stem@testing', controller.get_conf('contactinfo')) - self.assertEqual(tmpdir, controller.get_conf('HiddenServiceDir')) - self.assertEqual('17234 127.0.0.1:17235', controller.get_conf('HiddenServicePort')) - finally: - # reverts configuration changes - - controller.set_options(( - ('ExitPolicy', 'reject *:*'), - ('ConnLimit', None), - ('ContactInfo', None), - ('HiddenServiceDir', None), - ('HiddenServicePort', None), - ), reset = True) - - shutil.rmtree(tmpdir) + # fail to set multiple config options + try: + controller.set_options({ + 'contactinfo': 'stem@testing', + 'bombay': 'vadapav', + }) + self.fail() + except stem.InvalidArguments as exc: + self.assertEqual(['bombay'], exc.arguments) + + # context-sensitive keys (the only retched things for which order matters) + controller.set_options(( + ('HiddenServiceDir', tmpdir), + ('HiddenServicePort', '17234 127.0.0.1:17235'), + )) + + self.assertEqual(tmpdir, controller.get_conf('HiddenServiceDir')) + self.assertEqual('17234 127.0.0.1:17235', controller.get_conf('HiddenServicePort')) + finally: + # reverts configuration changes + + controller.set_options(( + ('ExitPolicy', 'reject *:*'), + ('ConnLimit', None), + ('ContactInfo', None), + ('HiddenServiceDir', None), + ('HiddenServicePort', None), + ), reset = True) @test.require.controller def test_set_conf_for_usebridges(self): diff --git a/test/integ/process.py b/test/integ/process.py index e7d2cae3..e40c501a 100644 --- a/test/integ/process.py +++ b/test/integ/process.py @@ -9,7 +9,6 @@ import hashlib import os import random import re -import shutil import subprocess import tempfile import threading @@ -53,18 +52,8 @@ def random_port(): @contextmanager -def tmp_directory(): - tmp_dir = tempfile.mkdtemp() - - try: - yield tmp_dir - finally: - shutil.rmtree(tmp_dir) - - -@contextmanager def torrc(): - with tmp_directory() as data_directory: + with tempfile.TemporaryDirectory() as data_directory: torrc_path = os.path.join(data_directory, 'torrc') with open(torrc_path, 'w') as torrc_file: @@ -228,7 +217,7 @@ class TestProcess(unittest.TestCase): output = run_tor(tor_cmd, '--list-fingerprint', with_torrc = True, expect_failure = True) assert_in("Clients don't have long-term identity keys. Exiting.", output, 'Should fail to start due to lacking an ORPort') - with tmp_directory() as data_directory: + with tempfile.TemporaryDirectory() as data_directory: torrc_path = os.path.join(data_directory, 'torrc') with open(torrc_path, 'w') as torrc_file: @@ -324,7 +313,7 @@ class TestProcess(unittest.TestCase): if test.tor_version() < stem.version.Requirement.TORRC_VIA_STDIN: skip('(requires %s)' % stem.version.Requirement.TORRC_VIA_STDIN) - with tmp_directory() as data_directory: + with tempfile.TemporaryDirectory() as data_directory: torrc = BASIC_RELAY_TORRC % data_directory output = run_tor(tor_cmd, '-f', '-', '--dump-config', 'short', stdin = torrc) assert_equal(sorted(torrc.splitlines()), sorted(output.splitlines())) @@ -382,7 +371,7 @@ class TestProcess(unittest.TestCase): it isn't. """ - with tmp_directory() as data_directory: + with tempfile.TemporaryDirectory() as data_directory: # Tries running tor in another thread with the given timeout argument. This # issues an invalid torrc so we terminate right away if we get to the point # of actually invoking tor. @@ -435,7 +424,7 @@ class TestProcess(unittest.TestCase): Exercises launch_tor_with_config when we write a torrc to disk. """ - with tmp_directory() as data_directory: + with tempfile.TemporaryDirectory() as data_directory: control_port = random_port() control_socket, tor_process = None, None @@ -479,7 +468,7 @@ class TestProcess(unittest.TestCase): if test.tor_version() < stem.version.Requirement.TORRC_VIA_STDIN: skip('(requires %s)' % stem.version.Requirement.TORRC_VIA_STDIN) - with tmp_directory() as data_directory: + with tempfile.TemporaryDirectory() as data_directory: control_port = random_port() control_socket, tor_process = None, None @@ -521,7 +510,7 @@ class TestProcess(unittest.TestCase): # [warn] Failed to parse/validate config: Failed to bind one of the listener ports. # [err] Reading config failed--see warnings above. - with tmp_directory() as data_directory: + with tempfile.TemporaryDirectory() as data_directory: both_ports = random_port() try: @@ -543,7 +532,7 @@ class TestProcess(unittest.TestCase): Runs launch_tor where it times out before completing. """ - with tmp_directory() as data_directory: + with tempfile.TemporaryDirectory() as data_directory: start_time = time.time() try: @@ -575,7 +564,7 @@ class TestProcess(unittest.TestCase): elif test.tor_version() < stem.version.Requirement.TAKEOWNERSHIP: skip('(requires %s)' % stem.version.Requirement.TAKEOWNERSHIP) - with tmp_directory() as data_directory: + with tempfile.TemporaryDirectory() as data_directory: sleep_process = subprocess.Popen(['sleep', '60']) tor_process = stem.process.launch_tor_with_config( @@ -619,7 +608,7 @@ class TestProcess(unittest.TestCase): if test.tor_version() < stem.version.Requirement.TAKEOWNERSHIP: skip('(requires %s)' % stem.version.Requirement.TAKEOWNERSHIP) - with tmp_directory() as data_directory: + with tempfile.TemporaryDirectory() as data_directory: control_port = random_port() tor_process = stem.process.launch_tor_with_config( diff --git a/test/integ/util/system.py b/test/integ/util/system.py index 4d83d695..2a7ac07a 100644 --- a/test/integ/util/system.py +++ b/test/integ/util/system.py @@ -361,13 +361,14 @@ class TestSystem(unittest.TestCase): Checks the stem.util.system.pid_by_open_file function. """ + # check a directory that doesn't exist + + self.assertEqual(None, stem.util.system.pid_by_open_file('/no/such/path')) + # check a directory that exists, but isn't claimed by any application - tmpdir = tempfile.mkdtemp() - self.assertEqual(None, stem.util.system.pid_by_open_file(tmpdir)) - # check a directory that doesn't exist - os.rmdir(tmpdir) - self.assertEqual(None, stem.util.system.pid_by_open_file(tmpdir)) + with tempfile.TemporaryDirectory() as tmpdir: + self.assertEqual(None, stem.util.system.pid_by_open_file(tmpdir)) @require_path def test_pids_by_user(self): diff --git a/test/unit/manual.py b/test/unit/manual.py index eb76d6e9..e1891ca4 100644 --- a/test/unit/manual.py +++ b/test/unit/manual.py @@ -76,6 +76,10 @@ EXPECTED_CONFIG_OPTIONS['Bridge'] = stem.manual.ConfigOption( CACHED_MANUAL = None +TEMP_DIR_MOCK = Mock() +TEMP_DIR_MOCK.__enter__ = Mock(return_value = '/no/such/path') +TEMP_DIR_MOCK.__exit__ = Mock(return_value = False) + def _cached_manual(): global CACHED_MANUAL @@ -230,16 +234,14 @@ class TestManual(unittest.TestCase): exc_msg = 'We require a2x from asciidoc to provide a man page' self.assertRaisesWith(IOError, exc_msg, stem.manual.download_man_page, '/tmp/no_such_file') - @patch('tempfile.mkdtemp', Mock(return_value = '/no/such/path')) - @patch('shutil.rmtree', Mock()) + @patch('tempfile.TemporaryDirectory', Mock(return_value = TEMP_DIR_MOCK)) @patch('stem.manual.open', Mock(side_effect = IOError('unable to write to file')), create = True) @patch('stem.util.system.is_available', Mock(return_value = True)) def test_download_man_page_when_unable_to_write(self): exc_msg = "Unable to download tor's manual from https://gitweb.torproject.org/tor.git/plain/doc/tor.1.txt to /no/such/path/tor.1.txt: unable to write to file" self.assertRaisesWith(IOError, exc_msg, stem.manual.download_man_page, '/tmp/no_such_file') - @patch('tempfile.mkdtemp', Mock(return_value = '/no/such/path')) - @patch('shutil.rmtree', Mock()) + @patch('tempfile.TemporaryDirectory', Mock(return_value = TEMP_DIR_MOCK)) @patch('stem.manual.open', Mock(return_value = io.BytesIO()), create = True) @patch('stem.util.system.is_available', Mock(return_value = True)) @patch('urllib.request.urlopen', Mock(side_effect = urllib.request.URLError('<urlopen error [Errno -2] Name or service not known>'))) @@ -247,8 +249,7 @@ class TestManual(unittest.TestCase): exc_msg = "Unable to download tor's manual from https://www.atagar.com/foo/bar to /no/such/path/tor.1.txt: <urlopen error <urlopen error [Errno -2] Name or service not known>>" self.assertRaisesWith(IOError, exc_msg, stem.manual.download_man_page, '/tmp/no_such_file', url = 'https://www.atagar.com/foo/bar') - @patch('tempfile.mkdtemp', Mock(return_value = '/no/such/path')) - @patch('shutil.rmtree', Mock()) + @patch('tempfile.TemporaryDirectory', Mock(return_value = TEMP_DIR_MOCK)) @patch('stem.manual.open', Mock(return_value = io.BytesIO()), create = True) @patch('stem.util.system.call', Mock(side_effect = stem.util.system.CallError('call failed', 'a2x -f manpage /no/such/path/tor.1.txt', 1, None, None, 'call failed'))) @patch('stem.util.system.is_available', Mock(return_value = True)) @@ -257,8 +258,7 @@ class TestManual(unittest.TestCase): exc_msg = "Unable to run 'a2x -f manpage /no/such/path/tor.1.txt': call failed" self.assertRaisesWith(IOError, exc_msg, stem.manual.download_man_page, '/tmp/no_such_file', url = 'https://www.atagar.com/foo/bar') - @patch('tempfile.mkdtemp', Mock(return_value = '/no/such/path')) - @patch('shutil.rmtree', Mock()) + @patch('tempfile.TemporaryDirectory', Mock(return_value = TEMP_DIR_MOCK)) @patch('stem.manual.open', create = True) @patch('stem.util.system.call') @patch('stem.util.system.is_available', Mock(return_value = True))