"B. W. Fitzpatrick" <fitz@red-bean.com> writes:
> > > Last week I submitted a little change to the svnversion test. Before
> > > the test was found acceptable, I had to rewrite it so that it was
> > > written as a 'new style' test: one which uses the Python exception
> > > handling instead of the return 0 / return 1 structure.
> >
> > I'm reviewing this now.
>
> I have a few comments about the patch:
>
> 1. Yay!
>
> 2. I'd like to see the whitespace changes in a separate patch, but
> that's not really A Big Deal.
>
> 3. I found a couple of places where you're doing this:
>
> stat_output, err_output = svntest.main.run_svn(None, 'stat', '-vN')
> if err_output:
> raise svntest.Failure
>
> I'd like to see you use
>
> run_and_verify_svn(message, expected_stdout, expected_stderr, *varargs):
>
> in these cases. It integrates the checking for different outputs on
> stderr and stdout. I've already converted a bunch of the tests to use
> it, so there are plenty of examples.
These fixes, plus addressing the "raise 1" oopsy, plus my sudden lack
of immediate time to review and apply the patch are good reasons to
suggest that the patch be resubmitted with the proper fixes in place.
At that later date, perhaps I'll have more review/application time to
offer.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Mon Aug 25 19:26:08 2003