Masaru Tsuchiyama wrote on Sat, Aug 03, 2013 at 16:06:06 +0900:
> > 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.
>
> My mail client can handle text/plain MIME type, but it always encodes an
> file with uuencode.
The Content-Transfer-Encoding is not as important as the Content-Type, IMO.
> I'll switch my client to Thunderbird. Thunderbird seems to be able to
> handle such situation.
>
That's one way of solving the problem :-)
> > 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
>
> It seems to use subprocess.CalledProcessError instead of CalledProcessError.
Yes, I wasn't using the fully-qualified name.
> I got NameError when I used the code.
>
> NameError: global name 'CalledProcessError' is not defined
>
> > > >
> > > > 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'll try to find more appropriate method.
>
Okay. Feel free to ask for opinions if needed.
> > > > 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"?
>
> I was running these scripts on the root of the working directory.
>
> When using prop_tests.py on the root directory, I got a file-not-found
> error.
> I implemented the patch to find which file is not found.
>
> Then I found it that atomic-ra-revprop-change.exe couldn't be found.
> --bin option can't handle the location of atomic-ra-revprop-change.exe.
> main.py expects that atomic-ra-revprop-change.exe is in the current
> directory,
> and can't change the location by command line option.
>
> If I run the these scripts at Release\subversion\tests\cmdline, I don't
> get a file-not-found error.
So perhaps the right fix is not your patch, but to look for
atomic-ra-revprop-change up front and bail out if it's not found?
IIRC, on unix, if you run the tests as
subversion/tests/cmdline/basic_tests.py
it will create svn-test-work in cwd, which may be argued to be
undesirable.
Received on 2013-08-03 14:53:30 CEST