Masaru Tsuchiyama wrote on Sat, Aug 03, 2013 at 11:04:30 +0900:
> > Please send patches as text/plain (*.txt extension often does this).
>
> I understand.
>
Thanks.
One of your later patches had a *.txt extension but application/octet-stream
MIME type. It would be good if you could get your client to set text/*
MIME type --- this actually changes the way the patch renders in many
devs' clients, makes it easier for them to read/review/reply the patch.
> > Masaru Tsuchiyama wrote on Sun, Jul 28, 2013 at 20:00:27 +0900:
> > > +++ subversion/tests/cmdline/svntest/main.py (working copy)
> > > @@ -412,12 +412,19 @@
> > > if not stderr:
> > > stderr = subprocess.PIPE
> > >
> > > - p = subprocess.Popen(command,
> > > + try:
> > > + p = subprocess.Popen(command,
> > > bufsize,
> > > stdin=stdin,
> > > stdout=stdout,
> > > stderr=stderr,
> > > close_fds=not windows)
> > > + except:
> >
> > Don't catch everything. Here you can/should catch only CalledProcessError.
>
> I encountered WindowsError exception on Windows Python 2.7.
> I'm not sure which expection is raised on Linux/UNIX.
> So I catch all, and reraise it.
>
Capturing all exceptions is wrong, period. How do you know that on
Linux you aren't capturing an "DoNotCatchMeOrIWillRmDashRFSlashError"
exception?
WindowsError is an instance of OSError, and that method is documented as
raising CalledProcessError, so you might do:
except (OSError, CalledProcessError):
> > > + # catch expeption, print information, and reraise
> > > + print "current dir:", os.path.abspath(os.getcwd())
> > > + print command
> >
> > Makes me twitchy. svntest is a library and it shouldn't print, since
> > this assumes no caller catches the exception. If possible I'd rather
> > annotate the exception object and re-raise it.
>
> Could you tell me how to 'annotate an exception'?
>
I'm not completely sure. The 'raise from' syntax is not available. You
could set a new attribute on the CalledProcessError or OSError instance,
and have callers inspect that attribute _if available_ (hasattr/getattr).
Or you could subclass those two and raise an instance of the subclass.
> > I haven't looked at the context, but I assume this is to address the
> > failure mode of running tests before the svn* binaries are built?
>
> I used checkout_tests.py, export_tests.py, prop_tests.py, and
> update_test.py respectively. At the time, I needed to copy the test
> binaries manually to the location which these scripts expected.
So... what is a case in which the new information you added will be
displayed? Is it "run one test on windows"? Is it "run tests with
a typo in the argument of the --bin option"?
Received on 2013-08-03 06:12:13 CEST