From 36e8d748a15044047054577f341f37bff621f63d Mon Sep 17 00:00:00 2001 From: Erica Portnoy Date: Thu, 1 Nov 2018 16:39:54 -0700 Subject: [PATCH] Clean up many warnings --- .../certbot_apache/tests/configurator_test.py | 6 ++-- certbot/cli.py | 4 ++- certbot/plugins/standalone_test.py | 2 ++ certbot/renewal.py | 4 ++- certbot/storage.py | 2 ++ certbot/tests/cert_manager_test.py | 5 ++-- certbot/tests/display/util_test.py | 1 + certbot/tests/log_test.py | 1 + certbot/tests/storage_test.py | 5 ++-- certbot/tests/util_test.py | 28 ++++++++++++++----- 10 files changed, 41 insertions(+), 17 deletions(-) diff --git a/certbot-apache/certbot_apache/tests/configurator_test.py b/certbot-apache/certbot_apache/tests/configurator_test.py index f18d3aac8..a77dbf637 100644 --- a/certbot-apache/certbot_apache/tests/configurator_test.py +++ b/certbot-apache/certbot_apache/tests/configurator_test.py @@ -1651,7 +1651,8 @@ class MultiVhostsTest(util.ApacheTest): self.assertTrue(self.config.parser.find_dir( "RewriteEngine", "on", ssl_vhost.path, False)) - conf_text = open(ssl_vhost.filep).read() + with open(ssl_vhost.filep) as the_file: + conf_text = the_file.read() commented_rewrite_rule = ("# RewriteRule \"^/secrets/(.+)\" " "\"https://new.example.com/docs/$1\" [R,L]") uncommented_rewrite_rule = ("RewriteRule \"^/docs/(.+)\" " @@ -1667,7 +1668,8 @@ class MultiVhostsTest(util.ApacheTest): ssl_vhost = self.config.make_vhost_ssl(self.vh_truth[3]) - conf_lines = open(ssl_vhost.filep).readlines() + with open(ssl_vhost.filep) as the_file: + conf_lines = the_file.readlines() conf_line_set = [l.strip() for l in conf_lines] not_commented_cond1 = ("RewriteCond " "%{DOCUMENT_ROOT}/%{REQUEST_FILENAME} !-f") diff --git a/certbot/cli.py b/certbot/cli.py index 99bf33180..6502d2987 100644 --- a/certbot/cli.py +++ b/certbot/cli.py @@ -286,7 +286,9 @@ def read_file(filename, mode="rb"): """ try: filename = os.path.abspath(filename) - return filename, open(filename, mode).read() + with open(filename, mode) as the_file: + contents = the_file.read() + return filename, contents except IOError as exc: raise argparse.ArgumentTypeError(exc.strerror) diff --git a/certbot/plugins/standalone_test.py b/certbot/plugins/standalone_test.py index 47f44ff77..9b741fc6f 100644 --- a/certbot/plugins/standalone_test.py +++ b/certbot/plugins/standalone_test.py @@ -72,6 +72,8 @@ class ServerManagerTest(unittest.TestCase): errors.StandaloneBindError, self.mgr.run, port, challenge_type=challenges.HTTP01) self.assertEqual(self.mgr.running(), {}) + some_server.close() + maybe_another_server.close() class SupportedChallengesActionTest(unittest.TestCase): diff --git a/certbot/renewal.py b/certbot/renewal.py index a1508fa60..5b2e00740 100644 --- a/certbot/renewal.py +++ b/certbot/renewal.py @@ -276,8 +276,10 @@ def _avoid_invalidating_lineage(config, lineage, original_server): "Do not renew a valid cert with one from a staging server!" # Some lineages may have begun with --staging, but then had production certs # added to them + with open(lineage.cert) as the_file: + contents = the_file.read() latest_cert = OpenSSL.crypto.load_certificate( - OpenSSL.crypto.FILETYPE_PEM, open(lineage.cert).read()) + OpenSSL.crypto.FILETYPE_PEM, contents) # all our test certs are from happy hacker fake CA, though maybe one day # we should test more methodically now_valid = "fake" not in repr(latest_cert.get_issuer()).lower() diff --git a/certbot/storage.py b/certbot/storage.py index 2aadaa67a..2a7ccee02 100644 --- a/certbot/storage.py +++ b/certbot/storage.py @@ -1034,9 +1034,11 @@ class RenewableCert(object): archive = full_archive_path(None, cli_config, lineagename) live_dir = _full_live_path(cli_config, lineagename) if os.path.exists(archive): + config_file.close() raise errors.CertStorageError( "archive directory exists for " + lineagename) if os.path.exists(live_dir): + config_file.close() raise errors.CertStorageError( "live directory exists for " + lineagename) os.mkdir(archive) diff --git a/certbot/tests/cert_manager_test.py b/certbot/tests/cert_manager_test.py index 7c3f4c85a..84774ca77 100644 --- a/certbot/tests/cert_manager_test.py +++ b/certbot/tests/cert_manager_test.py @@ -39,9 +39,8 @@ class BaseCertManagerTest(test_util.ConfigTestCase): # We also create a file that isn't a renewal config in the same # location to test that logic that reads in all-and-only renewal # configs will ignore it and NOT attempt to parse it. - junk = open(os.path.join(self.config.renewal_configs_dir, "IGNORE.THIS"), "w") - junk.write("This file should be ignored!") - junk.close() + with open(os.path.join(self.config.renewal_configs_dir, "IGNORE.THIS"), "w") as junk: + junk.write("This file should be ignored!") def _set_up_config(self, domain, custom_archive): # TODO: maybe provide NamespaceConfig.make_dirs? diff --git a/certbot/tests/display/util_test.py b/certbot/tests/display/util_test.py index eccffa47a..60131d853 100644 --- a/certbot/tests/display/util_test.py +++ b/certbot/tests/display/util_test.py @@ -49,6 +49,7 @@ class InputWithTimeoutTest(unittest.TestCase): stdin.listen(1) with mock.patch("certbot.display.util.sys.stdin", stdin): self.assertRaises(errors.Error, self._call, timeout=0.001) + stdin.close() class FileOutputDisplayTest(unittest.TestCase): diff --git a/certbot/tests/log_test.py b/certbot/tests/log_test.py index 6588bf5ca..b82cc6ca1 100644 --- a/certbot/tests/log_test.py +++ b/certbot/tests/log_test.py @@ -86,6 +86,7 @@ class PostArgParseSetupTest(test_util.ConfigTestCase): self.memory_handler.close() self.stream_handler.close() self.temp_handler.close() + self.devnull.close() super(PostArgParseSetupTest, self).tearDown() def test_common(self): diff --git a/certbot/tests/storage_test.py b/certbot/tests/storage_test.py index 912cd7984..484ad7e16 100644 --- a/certbot/tests/storage_test.py +++ b/certbot/tests/storage_test.py @@ -73,9 +73,8 @@ class BaseRenewableCertTest(test_util.ConfigTestCase): # We also create a file that isn't a renewal config in the same # location to test that logic that reads in all-and-only renewal # configs will ignore it and NOT attempt to parse it. - junk = open(os.path.join(self.config.config_dir, "renewal", "IGNORE.THIS"), "w") - junk.write("This file should be ignored!") - junk.close() + with open(os.path.join(self.config.config_dir, "renewal", "IGNORE.THIS"), "w") as junk: + junk.write("This file should be ignored!") self.defaults = configobj.ConfigObj() diff --git a/certbot/tests/util_test.py b/certbot/tests/util_test.py index 45cc55249..6685b88c6 100644 --- a/certbot/tests/util_test.py +++ b/certbot/tests/util_test.py @@ -210,16 +210,21 @@ class UniqueFileTest(test_util.TempDirTestCase): fd, name = self._call() fd.write("bar") fd.close() - self.assertEqual(open(name).read(), "bar") + with open(name) as f: + self.assertEqual(f.read(), "bar") def test_right_mode(self): - self.assertTrue(compat.compare_file_modes(0o700, os.stat(self._call(0o700)[1]).st_mode)) - self.assertTrue(compat.compare_file_modes(0o600, os.stat(self._call(0o600)[1]).st_mode)) + fd1, name1 = self._call(0o700) + fd2, name2 = self._call(0o600) + self.assertTrue(compat.compare_file_modes(0o700, os.stat(name1).st_mode)) + self.assertTrue(compat.compare_file_modes(0o600, os.stat(name2).st_mode)) + fd1.close() + fd2.close() def test_default_exists(self): - name1 = self._call()[1] # create 0000_foo.txt - name2 = self._call()[1] - name3 = self._call()[1] + fd1, name1 = self._call() # create 0000_foo.txt + fd2, name2 = self._call() + fd3, name3 = self._call() self.assertNotEqual(name1, name2) self.assertNotEqual(name1, name3) @@ -236,6 +241,10 @@ class UniqueFileTest(test_util.TempDirTestCase): basename3 = os.path.basename(name3) self.assertTrue(basename3.endswith("foo.txt")) + fd1.close() + fd2.close() + fd3.close() + try: file_type = file @@ -255,13 +264,18 @@ class UniqueLineageNameTest(test_util.TempDirTestCase): f, path = self._call("wow") self.assertTrue(isinstance(f, file_type)) self.assertEqual(os.path.join(self.tempdir, "wow.conf"), path) + f.close() def test_multiple(self): + items = [] for _ in six.moves.range(10): - f, name = self._call("wow") + items.append(self._call("wow")) + f, name = items[-1] self.assertTrue(isinstance(f, file_type)) self.assertTrue(isinstance(name, six.string_types)) self.assertTrue("wow-0009.conf" in name) + for f, _ in items: + f.close() @mock.patch("certbot.util.os.fdopen") def test_failure(self, mock_fdopen):