You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by "C. Michael Pilato" <cm...@collab.net> on 2010/12/14 17:14:00 UTC

Testing help/advice needed.

I've got a patch readied for issue #3755 ("svnadmin and svnrdump should
error on non-UTF8 log messages, authors, etc."), but it currently fails
three tests:

FAIL:  svnlook_tests.py 4: svnlook info must allow inconsistent newlines
FAIL:  svnrdump_tests.py 32: dump: inconsistent line endings in svn:props
FAIL:  svnsync_tests.py 26: copy with inconsistent lineendings in svn:props

All three fail for the same reason.  Can you guess it?  It's because the
tests are depending on the ability of 'svnadmin load' to introduce bogus
properties into a Subversion repository so that *other* validation code can
be tested.

What to do about this?

Is there a case to be made for allowing 'svnadmin load' to, upon specific
request by the user (via a new --disable-prop-validation flag or something),
*not* error out on invalid properties?  If so, then I can create and make
use of such a facility in these test scenarios.  But if not, what then?  Do
I remove the tests?  Do I mark them as FSFS-only and try to hack around in
the FSFS data format from the test suit?  Do I write a quicky little C
program that bypasses the libsvn_repos propchange validation logic, for use
with these Python tests?

-- 
C. Michael Pilato <cm...@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand


Re: Testing help/advice needed.

Posted by Stefan Sperling <st...@elego.de>.
On Tue, Dec 14, 2010 at 03:57:37PM -0500, C. Michael Pilato wrote:
> On 12/14/2010 01:47 PM, Daniel Shahaf wrote:
> > C. Michael Pilato wrote on Tue, Dec 14, 2010 at 12:14:00 -0500:
> >> Is there a case to be made for allowing 'svnadmin load' to, upon specific
> >> request by the user (via a new --disable-prop-validation flag or something),
> > 
> > Yes.
> > 
> > We've had many reports of svnsync aborting over non-canonical revprop
> > values after we started enforcing that in libsvn_repos during commits.
> > Every one of those reports represents a broken repository.  Adding such
> > a flag will allow the admins of said repositories to dump|load their
> > repositories without first fixing all historical revprops.
> 
> It's not just revprops that are being validated by the 'svnadmin load'
> process now -- it's node properties (svn:ignore, etc.), too.  I can
> certainly see the case for allowing a *technically* sane dump to be loaded
> without error, even if some of the contents thereof aren't up to our
> strictest standards.
> 
> Now I find myself questioning the whole change.  Do I move forward in this
> fashion, with 'svnadmin load' defaulting to prop validation w/ errors but
> offering a --disable-prop-validation flag?  Should the defaults be reversed?
>  What about 'svnrdump' -- does it need the same flexibility (I don't think
> this is actually possible, by the way)?  Do I back off a bit and make
> nothing error, but instead should print warning notifications during the
> load process whenever a bogus property is about to be loaded?

I'd say make it error, provide a by-pass flag for those who need it,
and document the by-pass flag in the error message.
Erroring out makes these kinds problems much more likely to be noticed
and fixed.

Stefan

Re: Testing help/advice needed.

Posted by Peter Samuelson <pe...@p12n.org>.
[C. Michael Pilato]
> It's not just revprops that are being validated by the 'svnadmin load'
> process now -- it's node properties (svn:ignore, etc.), too.  I can
> certainly see the case for allowing a *technically* sane dump to be loaded
> without error, even if some of the contents thereof aren't up to our
> strictest standards.

Well, that, or provide a tool that fixes broken dump files enough to be
acceptable to svnadmin load.  I don't know if we have such a tool
already (can contrib/server-side/svncutter do it?) - and indeed is
there a canonical obviously correct interpretation for all the various
things we check for that we didn't used to?

Hmmm, even if svncutter can do it, it's currently in contrib, so we
won't ship it in tarballs.  The license header looks like it may not
pass ASF muster, but I suppose ESR could be prevailed upon to permit
the standard boilerplate.  If this is something we'd want to move to
tools/.

-- 
Peter Samuelson | org-tld!p12n!peter | http://p12n.org/

Re: Testing help/advice needed.

Posted by "C. Michael Pilato" <cm...@collab.net>.
On 12/14/2010 01:47 PM, Daniel Shahaf wrote:
> C. Michael Pilato wrote on Tue, Dec 14, 2010 at 12:14:00 -0500:
>> Is there a case to be made for allowing 'svnadmin load' to, upon specific
>> request by the user (via a new --disable-prop-validation flag or something),
> 
> Yes.
> 
> We've had many reports of svnsync aborting over non-canonical revprop
> values after we started enforcing that in libsvn_repos during commits.
> Every one of those reports represents a broken repository.  Adding such
> a flag will allow the admins of said repositories to dump|load their
> repositories without first fixing all historical revprops.

It's not just revprops that are being validated by the 'svnadmin load'
process now -- it's node properties (svn:ignore, etc.), too.  I can
certainly see the case for allowing a *technically* sane dump to be loaded
without error, even if some of the contents thereof aren't up to our
strictest standards.

Now I find myself questioning the whole change.  Do I move forward in this
fashion, with 'svnadmin load' defaulting to prop validation w/ errors but
offering a --disable-prop-validation flag?  Should the defaults be reversed?
 What about 'svnrdump' -- does it need the same flexibility (I don't think
this is actually possible, by the way)?  Do I back off a bit and make
nothing error, but instead should print warning notifications during the
load process whenever a bogus property is about to be loaded?

-- 
C. Michael Pilato <cm...@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand


Re: Testing help/advice needed.

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
C. Michael Pilato wrote on Tue, Dec 14, 2010 at 12:14:00 -0500:
> I've got a patch readied for issue #3755 ("svnadmin and svnrdump should
> error on non-UTF8 log messages, authors, etc."), but it currently fails
> three tests:
> 
> FAIL:  svnlook_tests.py 4: svnlook info must allow inconsistent newlines
> FAIL:  svnrdump_tests.py 32: dump: inconsistent line endings in svn:props
> FAIL:  svnsync_tests.py 26: copy with inconsistent lineendings in svn:props
> 
> All three fail for the same reason.  Can you guess it?  It's because the
> tests are depending on the ability of 'svnadmin load' to introduce bogus
> properties into a Subversion repository so that *other* validation code can
> be tested.
> 
> What to do about this?
> 
> Is there a case to be made for allowing 'svnadmin load' to, upon specific
> request by the user (via a new --disable-prop-validation flag or something),

Yes.

We've had many reports of svnsync aborting over non-canonical revprop
values after we started enforcing that in libsvn_repos during commits.
Every one of those reports represents a broken repository.  Adding such
a flag will allow the admins of said repositories to dump|load their
repositories without first fixing all historical revprops.

(Which they may not have done, and which is even trickier to do if they
have to restore a 'bad' repository from a backup in the form of
a dumpfile.)

Daniel

> *not* error out on invalid properties?  If so, then I can create and make
> use of such a facility in these test scenarios.  But if not, what then?  Do
> I remove the tests?  Do I mark them as FSFS-only and try to hack around in
> the FSFS data format from the test suit?  Do I write a quicky little C
> program that bypasses the libsvn_repos propchange validation logic, for use
> with these Python tests?
> 
> -- 
> C. Michael Pilato <cm...@collab.net>
> CollabNet   <>   www.collab.net   <>   Distributed Development On Demand
>