From 9d6211f1b3250430cd0e37ef5b6e51b2068eb4de Mon Sep 17 00:00:00 2001 From: "Brian C. Lane" Date: Tue, 6 Oct 2020 10:55:00 -0700 Subject: [PATCH] Fix unclosed files Python will eventually close open files, but it is recommended to explicitly close them instead of waiting for the gc or program exit. This fixes all the uses of open...read/write in the codebase, mostly in tests. --- src/pylorax/sysutils.py | 4 ++-- tests/lib.py | 3 ++- tests/pylorax/test_creator.py | 23 +++++++++++++++-------- tests/pylorax/test_executils.py | 3 ++- tests/pylorax/test_imgutils.py | 9 ++++++--- tests/pylorax/test_ltmpl.py | 8 ++++++-- tests/pylorax/test_sysutils.py | 13 +++++++++---- 7 files changed, 42 insertions(+), 21 deletions(-) diff --git a/src/pylorax/sysutils.py b/src/pylorax/sysutils.py index c445fae1..35f8b0fe 100644 --- a/src/pylorax/sysutils.py +++ b/src/pylorax/sysutils.py @@ -45,8 +45,8 @@ def joinpaths(*args, **kwargs): def touch(fname): - # python closes the file when it goes out of scope - open(fname, "w").write("") + with open(fname, "w") as f: + f.write("") def replace(fname, find, sub): diff --git a/tests/lib.py b/tests/lib.py index a57c866a..f1de4178 100644 --- a/tests/lib.py +++ b/tests/lib.py @@ -96,7 +96,8 @@ def create_git_repo(): test_results["second"].append(os.path.join("only-bps/", f)) # Add a dotfile as well - open(os.path.join(repodir, "only-bps/.bpsrc"), "w").write("dotfile test\n") + with open(os.path.join(repodir, "only-bps/.bpsrc"), "w") as f: + f.write("dotfile test\n") test_results["second"].append("only-bps/.bpsrc") test_results["second"] = sorted(test_results["second"]) diff --git a/tests/pylorax/test_creator.py b/tests/pylorax/test_creator.py index 6c00a182..c8836c31 100644 --- a/tests/pylorax/test_creator.py +++ b/tests/pylorax/test_creator.py @@ -37,8 +37,10 @@ from pylorax.sysutils import joinpaths def mkFakeBoot(root_dir): """Create a fake kernel and initrd""" os.makedirs(joinpaths(root_dir, "boot")) - open(joinpaths(root_dir, "boot", "vmlinuz-4.18.13-200.fc28.x86_64"), "w").write("I AM A FAKE KERNEL") - open(joinpaths(root_dir, "boot", "initramfs-4.18.13-200.fc28.x86_64.img"), "w").write("I AM A FAKE INITRD") + with open(joinpaths(root_dir, "boot", "vmlinuz-4.18.13-200.fc28.x86_64"), "w") as f: + f.write("I AM A FAKE KERNEL") + with open(joinpaths(root_dir, "boot", "initramfs-4.18.13-200.fc28.x86_64.img"), "w") as f: + f.write("I AM A FAKE INITRD") class CreatorTest(unittest.TestCase): @@ -92,13 +94,15 @@ class CreatorTest(unittest.TestCase): # A fake disk image with tempfile.NamedTemporaryFile(prefix="lorax.test.disk.") as disk_img: - open(disk_img.name, "wb").write(b"THIS IS A FAKE DISK IMAGE FILE") + with open(disk_img.name, "wb") as f: + f.write(b"THIS IS A FAKE DISK IMAGE FILE") with tempfile.NamedTemporaryFile(prefix="lorax.test.appliance.") as output_xml: make_appliance(disk_img.name, "test-appliance", appliance_template, output_xml.name, ["eth0", "eth1"], ram=4096, vcpus=8, arch="x86_64", title="Lorax Test", project="Fedora", releasever="30") - print(open(output_xml.name).read()) + with open(output_xml.name) as f: + print(f.read()) # Parse the XML and check for known fields tree = ET.parse(output_xml.name) image = tree.getroot() @@ -127,13 +131,16 @@ class CreatorTest(unittest.TestCase): template = joinpaths(lorax_templates, "pxe-live/pxe-config.tmpl") # Make a fake kernel and initrd - open(joinpaths(work_dir, "vmlinuz-4.18.13-200.fc28.x86_64"), "w").write("I AM A FAKE KERNEL") - open(joinpaths(work_dir, "initramfs-4.18.13-200.fc28.x86_64.img"), "w").write("I AM A FAKE INITRD") + with open(joinpaths(work_dir, "vmlinuz-4.18.13-200.fc28.x86_64"), "w") as f: + f.write("I AM A FAKE KERNEL") + with open(joinpaths(work_dir, "initramfs-4.18.13-200.fc28.x86_64.img"), "w") as f: + f.write("I AM A FAKE INITRD") # Create the PXE_CONFIG in work_dir create_pxe_config(template, work_dir, live_image_name, add_pxe_args) - print(open(joinpaths(work_dir, "PXE_CONFIG")).read()) - pxe_config = open(joinpaths(work_dir, "PXE_CONFIG")).read() + with open(joinpaths(work_dir, "PXE_CONFIG")) as f: + pxe_config = f.read() + print(pxe_config) self.assertTrue("vmlinuz-4.18.13-200.fc28.x86_64" in pxe_config) self.assertTrue("initramfs-4.18.13-200.fc28.x86_64.img" in pxe_config) self.assertTrue("/live-rootfs.squashfs.img ostree=/mnt/sysimage/" in pxe_config) diff --git a/tests/pylorax/test_executils.py b/tests/pylorax/test_executils.py index dc88b8ac..f9d1866f 100644 --- a/tests/pylorax/test_executils.py +++ b/tests/pylorax/test_executils.py @@ -66,7 +66,8 @@ class ExecUtilsTest(unittest.TestCase): self.assertEqual(rc, 1) tmp_f.file.close() - logged_text = open(tmp_f.name, "r").readlines()[-1].strip() + with open(tmp_f.name, "r") as f: + logged_text = f.readlines()[-1].strip() self.assertEqual(logged_text, "The Once-ler was here.") finally: os.unlink(tmp_f.name) diff --git a/tests/pylorax/test_imgutils.py b/tests/pylorax/test_imgutils.py index 954b620a..76889913 100644 --- a/tests/pylorax/test_imgutils.py +++ b/tests/pylorax/test_imgutils.py @@ -45,7 +45,8 @@ def mkfakerootdir(rootdir): for f in files: if not os.path.isdir(joinpaths(rootdir, os.path.dirname(f))): os.makedirs(joinpaths(rootdir, os.path.dirname(f))) - open(joinpaths(rootdir, f), "w").write("I AM FAKE FILE %s" % f.upper()) + with open(joinpaths(rootdir, f), "w") as ff: + ff.write("I AM FAKE FILE %s" % f.upper()) def mkfakebootdir(bootdir): """Populate a fake /boot directory with a kernel and initrd @@ -53,8 +54,10 @@ def mkfakebootdir(bootdir): :param bootdir: An existing directory to create files/dirs under :type bootdir: str """ - open(joinpaths(bootdir, "vmlinuz-4.18.13-200.fc28.x86_64"), "w").write("I AM A FAKE KERNEL") - open(joinpaths(bootdir, "initramfs-4.18.13-200.fc28.x86_64.img"), "w").write("I AM A FAKE INITRD") + with open(joinpaths(bootdir, "vmlinuz-4.18.13-200.fc28.x86_64"), "w") as f: + f.write("I AM A FAKE KERNEL") + with open(joinpaths(bootdir, "initramfs-4.18.13-200.fc28.x86_64.img"), "w") as f: + f.write("I AM A FAKE INITRD") def mkfakediskimg(disk_img): """Create a fake partitioned disk image diff --git a/tests/pylorax/test_ltmpl.py b/tests/pylorax/test_ltmpl.py index 86bebf2b..8d1871a3 100644 --- a/tests/pylorax/test_ltmpl.py +++ b/tests/pylorax/test_ltmpl.py @@ -177,7 +177,9 @@ class LoraxTemplateRunnerTestCase(unittest.TestCase): """Test append, and install template commands""" self.runner.run("install-cmd.tmpl") self.assertTrue(os.path.exists(joinpaths(self.root_dir, "/etc/lorax-test"))) - self.assertEqual(open(joinpaths(self.root_dir, "/etc/lorax-test")).read(), "TESTING LORAX TEMPLATES\n") + with open(joinpaths(self.root_dir, "/etc/lorax-test")) as f: + data = f.read() + self.assertEqual(data, "TESTING LORAX TEMPLATES\n") self.assertTrue(os.path.exists(joinpaths(self.root_dir, "/etc/lorax-test-dest"))) def test_installimg(self): @@ -194,7 +196,9 @@ class LoraxTemplateRunnerTestCase(unittest.TestCase): """Test append, and replace template command""" self.runner.run("replace-cmd.tmpl") self.assertTrue(os.path.exists(joinpaths(self.root_dir, "/etc/lorax-replace"))) - self.assertEqual(open(joinpaths(self.root_dir, "/etc/lorax-replace")).read(), "Running 1.2.3 for lorax\n") + with open(joinpaths(self.root_dir, "/etc/lorax-replace")) as f: + data = f.read() + self.assertEqual(data, "Running 1.2.3 for lorax\n") def test_treeinfo(self): """Test treeinfo template command""" diff --git a/tests/pylorax/test_sysutils.py b/tests/pylorax/test_sysutils.py index 07158b35..5edf3f4f 100644 --- a/tests/pylorax/test_sysutils.py +++ b/tests/pylorax/test_sysutils.py @@ -27,7 +27,8 @@ class SysUtilsTest(unittest.TestCase): self.assertEqual(joinpaths("foo", "bar", "baz"), "foo/bar/baz") with tempfile.TemporaryDirectory() as tdname: - open(os.path.join(tdname, "real-file"), "w").write("lorax test file") + with open(os.path.join(tdname, "real-file"), "w") as f: + f.write("lorax test file") os.symlink(os.path.join(tdname, "real-file"), os.path.join(tdname, "link-file")) self.assertEqual(joinpaths(tdname, "link-file", follow_symlinks=True), @@ -46,7 +47,9 @@ class SysUtilsTest(unittest.TestCase): f.close() replace(f.name, "@AARDVARKS@", "ant eaters") - self.assertEqual(open(f.name).readline(), "A few words to apply ant eaters testing\n") + with open(f.name) as fr: + line = fr.readline() + self.assertEqual(line, "A few words to apply ant eaters testing\n") os.unlink(f.name) @unittest.skipUnless(os.geteuid() == 0, "requires root privileges") @@ -61,7 +64,8 @@ class SysUtilsTest(unittest.TestCase): def test_remove(self): remove_file="/var/tmp/lorax-test-remove-file" - open(remove_file, "w").write("test was here") + with open(remove_file, "w") as f: + f.write("test was here") remove(remove_file) self.assertFalse(os.path.exists(remove_file)) @@ -69,7 +73,8 @@ class SysUtilsTest(unittest.TestCase): with tempfile.TemporaryDirectory() as tdname: path = os.path.join("one", "two", "three") os.makedirs(os.path.join(tdname, path)) - open(os.path.join(tdname, path, "lorax-link-test-file"), "w").write("test was here") + with open(os.path.join(tdname, path, "lorax-link-test-file"), "w") as f: + f.write("test was here") linktree(os.path.join(tdname, "one"), os.path.join(tdname, "copy"))