On Mon, Dec 17, 2001 at 02:19:14PM -0600, Karl Fogel wrote:
> Garrett Rooney <rooneg@electricjellyfish.net> writes:
> > here's a new version, with a test case.
>
> Applied, with the following tweaks, in revision 656:
>
> * The description string for the new error code should just
> describe what the error means. The advice about using "--force"
> to override is about a client-specific behavior, not about the
> error itself, so it belongs in main.c, where the error is
> handled; I moved it there.
yeah, that does make more sense.
> * Where main.c handles the error, I changed it to print to stderr,
> not stdout. Also noticed some other unrelated places in main.c
> where we handle errors to stdout; will fix those in a separate
> commit. Don't know why we were doing that; both standard
> practice and our test suite clearly prefer stderr.
i had it outputting to stdout because i saw other places doing that...
i agree though, they should be going to stderr.
> * I tried the new python test before patching the rest of the code,
> just to make sure the test fails without the code fix, but it
> passed... Oops. :-)
>
> Turns out a couple of invocations of svntest.main.run_svn()
> weren't passing the boolean error_expected flag as the first
> argument, so instead "ci" (the commit subcommand) became the
> first argument, and got interpreted as setting error_expected of
> course. So when Subversion errored out because it was invoked
> without a subcommand (because "ci" got used as a flag to
> run_svn() instead), run_svn() accepted the error as expected and
> the continued on like nothing was wrong. :-)
>
> In general, if you write a test for a bugfix, make sure the test
> fails when the bugfix is not present. In this particular case,
> that would actually have been the _only_ way to learn that the
> whole regression test was in fact not working (also partly due to
> the stdout/stderr thing mentioned above).
oops ;-)
i knocked out the test case fairly quickly, and had some trouble with
it, so i guess i was so thrilled when i finally figured out enough to
make the damn thing pass that i forgot to go through and make sure it
would fail!
> Above nits notwithstanding, nice work -- especially the thoroughness
> of coverage in the new regression test.
thanks.
> P.S. Enjoyed the deadpan humor of your log message for log_tests.py :-)
when i first started adding the test case, i was trying to figure out
where it should go, and my first instinct was the log tests file, but
then i opened it up and saw that comment... then i was a bit
discouraged, and thought i'd have to take a look at making that test
work... then i noticed that contrary to the comment it did seem to
work, and i was quite amused...
--
garrett rooney Unix was not designed to stop you from
rooneg@electricjellyfish.net doing stupid things, because that would
http://electricjellyfish.net/ stop you from doing clever things.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Oct 21 14:36:53 2006