[MERGE] win32-specific fixes for selftest (all blackbox tests pass!)
Andrew Bennetts
andrew at canonical.com
Tue Mar 6 01:16:09 GMT 2007
Alexander Belchenko wrote:
> New version of my patch.
>
> Andrew Bennetts пишет:
[...]
> >
> >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.
There's a comment in the code just below your change that says "# stderr isn't
buffered" — you need to update/delete that.
[...]
> >>- 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.
Ah, ok. In that case it's probably better to leave it as you have it.
> 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).
(Right, I wasn't suggesting to change the existing error handler for rmtree, but
that you could write a new one that does what you do above.)
> >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.
Fair enough.
-Andrew.
More information about the bazaar
mailing list