From Dan Kenigsberg <dan...@redhat.com>: Dan Kenigsberg has posted comments on this change.
Change subject: pylint: Add the missing tests for oop.os.rename() ...................................................................... Patch Set 2: (2 comments) https://gerrit.ovirt.org/#/c/75841/2/tests/storage_outofprocess_test.py File tests/storage_outofprocess_test.py: PS2, Line 116: src = os.path.join(tmpdir, "src") : src_subdir = os.path.join(src, "subdir") : os.makedirs(src_subdir) : src_file = os.path.join(src_subdir, "file") : with io.open(src_file, "wb") as f: : f.write(b"it works") : dst = os.path.join(tmpdir, "dst") this begs to be placed in a helper function, to be used by the next test as well (if we actually need the next test)... Line 127: dst_file = os.path.join(tmpdir, "dst", "subdir", "file") Line 128: with io.open(dst_file, "rb") as f: Line 129: self.assertEqual(f.read(), b"it works") Line 130: Line 131: @xfail("Fails when destination directory not empty") ... but the real question is why do we even need this convoluted logic. It was originally a bad idea to modify the os.rename semantics instead of returning ENOTEMPTY. I'd argue that raising AttributeError is closer to the correct semantics than overwriting nonempty destination dir. Line 132: def testOsRenameDestExists(self): Line 133: with namedTemporaryDir() as tmpdir: Line 134: src = os.path.join(tmpdir, "src") Line 135: src_subdir = os.path.join(src, "subdir") -- To view, visit https://gerrit.ovirt.org/75841 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib34e94ee0d5fcb14f808fe5f1763d165a10d441f Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer <nsof...@redhat.com> Gerrit-Reviewer: Adam Litke <ali...@redhat.com> Gerrit-Reviewer: Ala Hino <ah...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com> Gerrit-Reviewer: Freddy Rolland <froll...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com> Gerrit-Reviewer: gerrit-hooks <automat...@ovirt.org> Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
