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

rovitotv at gmail.com rovitotv at gmail.com
Mon Mar 11 03:15:07 CET 2013

Thanks for the review...I think the patch is getting better and better. 
Soon I should have this tested on Windows, hopefully tonight.

File Lib/test/test_os.py (right):

Lib/test/test_os.py:2209: class RenameTests(unittest.TestCase):
On 2013/03/11 00:45:35, vstepanov wrote:
> I have some comments to this patch.
> I think would be better to add two separate test case RenameFileTests
> RenameDirTests or something like that.
I actually like it in a single class because alot of the functionality
is the same for testing the rename of a directory or file.  If we
separate it into two classes then I am afraid that we get lots of
repeating which leads to DRY test cases.  See earlier comments in issue.

Lib/test/test_os.py:2215: self.rename_src_file = "%s_%d_tmp" %
On 2013/03/11 00:45:35, vstepanov wrote:
> 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 ...)
Good point....this is a side effect from me moving over to a new class
before it was in the FileTests directory.  Now that we have a
RenameTests class we can remove the rename_ in front of all the
variables this will help readability.  Thanks!

Lib/test/test_os.py:2216: os.getpid())
On 2013/03/11 00:45:35, vstepanov wrote:
> should not call this method everywhere, use a variable
Yes that will be more efficient, thanks!

Lib/test/test_os.py:2217: if os.path.exists(self.rename_src_file):
On 2013/03/11 00:45:35, vstepanov wrote:
> There is remove_file_or_directory method, use it for remove exist
Good point, I don't know how I missed the remove_file_or_directory in
setUp but I did.  Thanks.

Lib/test/test_os.py:2220: os.write(fd, b"beans\n")
On 2013/03/11 00:45:35, vstepanov wrote:
> Really need to write many lines in a file and then split the text on
the line in
> tests? Perhaps enough only one.
I got the original code from test_os.py line 111 in the TestWrite
method.  But it does not have to be that way I think the TestWrite
method was testing writes with binary data, byte array data, and memory
view data.  This is actually easier to read and doing the split lines
doesn't add any value.  Thanks.

Lib/test/test_os.py:2277: if (os.path.isfile(file)):
On 2013/03/11 00:45:35, vstepanov wrote:
> outer parentheses are unnecessary
Yes too much scheme sorry I missed those, they are gone now.

Lib/test/test_os.py:2289: self.assertRaises(OSError, os.rename, src,
On 2013/03/11 00:45:35, vstepanov wrote:
> I do not see where `src` and `dst` variables are declared
Oops I didn't catch that because I have not tested on Windows yet.  I do
plan to test on Windows soon maybe even tonight.  My Windows partition
is not setup correctly yet since I got my new computer back in October.

On 2013/03/11 00:45:35, vstepanov wrote:
> I think it would be better to create the files/directories in separate
> 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).
I have been wondering if there is a better approach to all of this. 
According to some of the CPython Committers we have to make the test
case as WET as possible and not repeat ourselves.  Before I was actually
creating the files/directories in the test cases but was not using a
method it took a lot of code.  I could make a version that does that but
use methods to create the files instead then measure the line count to
see which solution is WETTEST.  Let me think about this....

self.assertEqual(os.path.exists(self.rename_dst_directory), True)
On 2013/03/11 00:45:35, vstepanov wrote:
> There are unittest.TestCase.assertTrue and
> methods.
Yes I fixed some of them with V2 but V3 now has all of them fixed. 

Lib/test/test_os.py:2402: src = self.rename_src_file + "does_not_exist"
On 2013/03/11 00:45:35, vstepanov wrote:
> I don't see where `src` and `dst` are used.
You are correct I don't need those thanks!

I will post a new version soon....


More information about the docs mailing list