[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

RE: [PATCH 2/3] atomic-revprop: Change tests

From: Jon Foster <Jon.Foster_at_cabot.co.uk>
Date: Mon, 20 Sep 2010 23:15:38 +0100

Hi,

Daniel Shahaf wrote:
> Jon Foster wrote on Mon, Sep 20, 2010 at 12:39:31 +0100:
> > Daniel Shahaf wrote:
> > > Jon Foster wrote on Mon, Sep 20, 2010 at 10:48:44 +0100:
> > > > [[[
> > > > Change atomic-revprop tests to look at the error number rather
> > > > than parsing the error text.
> > > So, this is trying to capture the current ra_dav situation, where
the
> > > err->message is correct but err->apr_err isn't?
> > Correct.
> Okay. Perhaps this could have been called out more explicitly in
> the log message --- i.e., have it say not only *what* the patch
> does, but also *why* it does that.

Argh, I made one of the classic log-message-writing blunders!

> > > 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.
> Okay. I started a patch in that way at a time; I've touched it up now

> a tiny bit and I'm attaching it. Let me know what you think...

As discussed on IRC...

> Index: subversion/tests/cmdline/atomic-ra-revprop-change.c
> ===================================================================
> --- subversion/tests/cmdline/atomic-ra-revprop-change.c
(revision 998887)
> +++ subversion/tests/cmdline/atomic-ra-revprop-change.c (working
copy)
[...]
> @@ -117,6 +122,8 @@
> SVN_ERR(svn_config_create(&servers, pool));
> svn_config_set(servers, SVN_CONFIG_SECTION_GLOBAL,
> SVN_CONFIG_OPTION_HTTP_LIBRARY, http_library);
> + svn_config_set(servers, SVN_CONFIG_SECTION_GLOBAL,
> + SVN_CONFIG_OPTION_NEON_DEBUG_MASK,
getenv("SVN_NEON_DEBUG_MASK") /* "131" */);
>
> /* Populate CONFIG. */
> config = apr_hash_make(pool);

If you drop that hunk (which looks totally unrelated) then:

+1 (non-binding)

Tested (both "should pass" and "should fail") and it works.

Kind regards,

Jon

**********************************************************************
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-21 00:16:20 CEST

This is an archived mail posted to the Subversion Dev mailing list.