some comments to the patch http://bugs.python.org/review/16278/diff/7565/Lib/test/test_os.py File Lib/test/test_os.py (right): http://bugs.python.org/review/16278/diff/7565/Lib/test/test_os.py#newcode220... Lib/test/test_os.py:2209: class RenameTests(unittest.TestCase): I have some comments to this patch. I think would be better to add two separate test case RenameFileTests and RenameDirTests or something like that. http://bugs.python.org/review/16278/diff/7565/Lib/test/test_os.py#newcode221... Lib/test/test_os.py:2215: self.rename_src_file = "%s_%d_tmp" % ("test_rename_src_file", All variables has prefix ``rename_`` (rename_src_file, rename_dst_file and so on), so can use them without this prefix (just src_file, dst_file ...) http://bugs.python.org/review/16278/diff/7565/Lib/test/test_os.py#newcode221... Lib/test/test_os.py:2216: os.getpid()) should not call this method everywhere, use a variable http://bugs.python.org/review/16278/diff/7565/Lib/test/test_os.py#newcode221... Lib/test/test_os.py:2217: if os.path.exists(self.rename_src_file): There is remove_file_or_directory method, use it for remove exist files/dirs. http://bugs.python.org/review/16278/diff/7565/Lib/test/test_os.py#newcode222... Lib/test/test_os.py:2220: os.write(fd, b"beans\n") Really need to write many lines in a file and then split the text on the line in tests? Perhaps enough only one. http://bugs.python.org/review/16278/diff/7565/Lib/test/test_os.py#newcode227... Lib/test/test_os.py:2277: if (os.path.isfile(file)): outer parentheses are unnecessary http://bugs.python.org/review/16278/diff/7565/Lib/test/test_os.py#newcode228... Lib/test/test_os.py:2289: self.assertRaises(OSError, os.rename, src, dst) I do not see where `src` and `dst` variables are declared http://bugs.python.org/review/16278/diff/7565/Lib/test/test_os.py#newcode229... Lib/test/test_os.py:2295: self.remove_file_or_directory(self.rename_dst_file) I think it would be better to create the files/directories in separate methods and call them in tests methods instead of create them in setUp method and remove in each test (except for those that are not removed in tests methods, they can be left in the setUp). http://bugs.python.org/review/16278/diff/7565/Lib/test/test_os.py#newcode232... Lib/test/test_os.py:2326: self.assertEqual(os.path.exists(self.rename_dst_directory), True) There are unittest.TestCase.assertTrue and unittest.TestCase.assertFalse methods. http://bugs.python.org/review/16278/diff/7565/Lib/test/test_os.py#newcode240... Lib/test/test_os.py:2402: src = self.rename_src_file + "does_not_exist" I don't see where `src` and `dst` are used. http://bugs.python.org/review/16278/