[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