[MERGE] win32-specific fixes for selftest (all blackbox tests pass!)
Alexander Belchenko
bialix at ukr.net
Mon Mar 5 15:52:45 GMT 2007
New version of my patch.
Andrew Bennetts пишет:
> A few comments, but I'm basically happy for this to be merged. Thanks so much
> for doing this.
>
> Alexander Belchenko wrote:
>> === modified file bzr // last-changed:bialix at ukr.net-20070304001602-mhzsxzstjkx
>> ... t50ao
>> --- bzr
>> +++ bzr
>> @@ -106,6 +106,7 @@
>> # but our policy is to always close files from a finally block. -- mbp 20070215
>> try:
>> sys.stdout.flush()
>> + sys.stderr.flush()
>> except IOError, e:
>> import errno
>> if sys.platform != 'win32' or e.errno != errno.EINVAL:
>
> You should update the comment saying "# stderr isn't buffered". Probably just
> delete that comment I guess; it seemed like a reasonable assumption at the time,
> but apparently it isn't.
Done.
>
>> === modified file bzrlib/tests/__init__.py
>> --- bzrlib/tests/__init__.py
>> +++ bzrlib/tests/__init__.py
>> @@ -475,7 +475,14 @@
>> else:
>> test_root = test_root.encode(
>> sys.getfilesystemencoding())
>> - osutils.rmtree(test_root)
>> + try:
>> + osutils.rmtree(test_root)
>> + except OSError, e:
>> + if sys.platform == 'win32' and e.errno == errno.EACCES:
>> + print >>sys.stderr, ('Permission denied: '
>> + 'unable to remove testing dir')
>> + else:
>> + raise
>
> You do this several times.
Actually only 2 times and this cases is not strictly the same.
In first case I catch permission denied error for osutils.rmtree,
in second case I catch error for os.remove.
Code for osutils.rmtree I wrote myself. That code helps to remove
tree with read-only entries. But in my case there is opened
file not closed yet (because when error occurs in some testing
server running in thread, this server hangs and keep all opened
files handlers).
> I think it'd be better to add this behaviour to
> osutils.rmtree. In fact it almost has this feature already; see the default
> value for the 'onerror' parameter. Perhaps this should be done in an
> alternative error handler you can pass to onerror. Or perhaps just do
> "ignore_errors=True"?
No, this is bad idea. I prefer to keep it my way.
> Also, the error message printed should include the value of 'test_root'.
Done.
> Finally, I wonder if it would be better to use warnings.warn rather than
> printing directly to stderr?
warnings.warn print too much extra info about code context. I think this
info should not be exposed, because it's not related to the root of
problem.
>
>> branch = Branch.open(
>> - 'bzr+ssh://fred:secret@localhost:%d%s' % (port, path_to_branch))
>> -
>> + 'bzr+ssh://fred:secret@localhost:%d%s' % (port,path_to_branch))
>> +
>
> And thanks for fixing my code formatting :)
I uncommit this mistake.
--
Alexander
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: selftest.fixes.3.patch
Url: https://lists.ubuntu.com/archives/bazaar/attachments/20070305/c5afee89/attachment-0001.diff
More information about the bazaar
mailing list