[docs] os.rename documentation slightly inaccurate (issue 16278)

vitaliy.stepanov at gmail.com vitaliy.stepanov at gmail.com
Mon Mar 11 00:45:35 CET 2013


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#newcode2209
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#newcode2215
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#newcode2216
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#newcode2217
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#newcode2220
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#newcode2277
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#newcode2289
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#newcode2295
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#newcode2326
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#newcode2402
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/


More information about the docs mailing list