Hi,
Daniel Shahaf wrote:
> Jon Foster wrote on Mon, Sep 20, 2010 at 10:48:44 +0100:
> > Hi,
> >
> > This patch changes the tests for the atomic-revprop feature, so that
> > they test the error number rather than parsing the error text.
> > This doesn't work with Serf or Neon yet - they're still TODO. This
> > gives you a way to test Serf & Neon patches. (You might like to
> > hold off on committing this until Serf and Neon are done).
> >
> > The patch is against the atomic-revprop branch, and requires
> > Patch 1 to be applied first. It doesn't apply to trunk (I don't
> > think the tests have been merged to trunk?)
> >
>
> On trunk there are only the libsvn_fs and libsvn_repos parts. The
> libsvn_ra parts, and the associated Python tests, are only on the
> branch.
>
> > [[[
> > Change atomic-revprop tests to look at the error number rather
> > than parsing the error text.
> >
> > * subversion/tests/cmdline/atomic-ra-revprop-change.c:
> > (main): When printing an error, check if it's
SVN_ERR_BAD_OLD_VALUE
> > and print a special message if it is.
> >
> > * subversion/tests/cmdline/prop_tests.py:
> > (FAILS_WITH_BPV): Look for the special message.
> >
> > Patch by: Jon Foster <jon.foster_at_cabot.co.uk>
> > ]]]
> >
>
> So, this is trying to capture the current ra_dav situation, where the
> err->message is correct but err->apr_err isn't?
Correct.
> (and will make the test fail over ra_neon/ra_serf)
Correct.
> In other news, I've been thinking about moving the entire validation
> logic to the C helper; that is, to have it get an extra argv argument
> saying whether the revpropchange should pass or fail, and test by
itself
> that it passed/failed as expected; and the Python tests would always
> expect it to exit(0). What do you think?
Sounds like a good idea.
> > Index: subversion/tests/cmdline/atomic-ra-revprop-change.c
> > ===================================================================
> > --- subversion/tests/cmdline/atomic-ra-revprop-change.c
(revision 998620)
> > +++ subversion/tests/cmdline/atomic-ra-revprop-change.c (working
copy)
> > @@ -226,6 +226,8 @@
> > http_library, pool);
> > if (err)
> > {
> > + if (svn_error_has_cause(err, SVN_ERR_BAD_OLD_VALUE))
> > + fprintf(stderr, "atomic-ra-revprop-change failed due to
incorrect
>
> Or even more directly:
>
> - if (err)
> + if (err && err->apr_err == SVN_ERR_BAD_OLD_VALUE)
>
Wouldn't that leak an error if the error isn't SVN_ERR_BAD_OLD_VALUE?
Also, the current* design requires us to use svn_error_has_cause()
because
the error is wrapped inside an error with a different code. (FWIW,
I think the RA layer shouldn't do that - I think we should change
svn_ra_change_rev_prop() so that the simple test of err->apr_err works.
But that's not urgent, and could even be postponed to 1.8).
Kind regards,
Jon
(* Unless I missed something and the design's been changed?)
**********************************************************************
This email and its attachments may be confidential and are intended solely for the use of the individual to whom it is addressed. Any views or opinions expressed are solely those of the author and do not necessarily represent those of Cabot Communications Ltd.
If you are not the intended recipient of this email and its attachments, you must take no action based upon them, nor must you copy or show them to anyone.
Cabot Communications Limited
Verona House, Filwood Road, Bristol BS16 3RY, UK
+44 (0) 1179584232
Co. Registered in England number 02817269
Please contact the sender if you believe you have received this email in error.
**********************************************************************
______________________________________________________________________
This email has been scanned by the MessageLabs Email Security System.
For more information please visit http://www.messagelabs.com/email
______________________________________________________________________
Received on 2010-09-20 13:40:13 CEST