You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by ko...@apache.org on 2016/03/24 12:59:22 UTC
svn commit: r1736432 -
/subversion/trunk/subversion/tests/cmdline/update_tests.py
Author: kotkov
Date: Thu Mar 24 11:59:22 2016
New Revision: 1736432
URL: http://svn.apache.org/viewvc?rev=1736432&view=rev
Log:
Fix a false negative in update_tests.py#76 with SVNPathAuthz short_circuit.
This test used to assert that having a filename with backslash returns an
error during `svn update' over file:// or svn://, and doesn't do that over
http://. However, the last bit is specific to httpd instances configured
with SVNPathAuthz on, where a subrequest-based check gives 404 and results
in the path being excluded by the server. With SVNPathAuthz short_circuit,
the behavior is the same as when the command is run over file:// or svn://
and results in an error:
svn: E155000: 'completely\unusable\dir' is not valid as filename in [...]
Since the original issue (SVN-3288) is about a crash, and given that we're
currently fine with either of the results, tweak the test to accept both of
them. There is an alternative of teaching the tests to distinguish between
different types of httpd authz and making the checks even more conditional,
but that seems a bit over the top.
* subversion/tests/cmdline/update_tests.py
(windows_update_backslash): Tweak the expectations. Rework and extend
the comment about what we expect in this test.
Modified:
subversion/trunk/subversion/tests/cmdline/update_tests.py
Modified: subversion/trunk/subversion/tests/cmdline/update_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/update_tests.py?rev=1736432&r1=1736431&r2=1736432&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/update_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/update_tests.py Thu Mar 24 11:59:22 2016
@@ -6343,28 +6343,37 @@ def windows_update_backslash(sbox):
'mkdir', 'A/completely\\unusable\\dir')
# No error and a proper skip + recording in the working copy would also
- # be a good result. This just verifies current behavior.
-
- if sbox.repo_url.startswith('http'):
- # Apache Httpd doesn't allow paths with '\\' in them on Windows, so the
- # test if a user is allowed to read them returns a failure. This makes
- # mod_dav_svn report the path as server excluded (aka absent), which
- # doesn't produce output when updating.
- expected_output = [
- "Updating '%s':\n" % wc_dir,
- "At revision 2.\n"
- ]
- expected_err = []
- else:
- expected_output = None
- expected_err = 'svn: E155000: .* is not valid.*'
-
- svntest.actions.run_and_verify_svn(expected_output, expected_err,
- 'up', wc_dir)
-
- if sbox.repo_url.startswith('http'):
+ # be a good result. This just verifies current behavior:
+ #
+ # - Error via file://, svn:// or http:// with SVNPathAuthz short_circuit
+ #
+ # - No error via http:// with SVNPathAuthz on
+ # (The reason is that Apache Httpd doesn't allow paths with '\\' in
+ # them on Windows, and a subrequest-based access check returns 404.
+ # This makes mod_dav_svn report the path as server excluded (aka
+ # absent), which doesn't produce output when updating.)
+ #
+ # Since https://issues.apache.org/jira/browse/SVN-3288 is about a crash,
+ # we're fine with either result -- that is, if `svn update' finished
+ # without an error, we expect specific stdout and proper wc state.
+ # If it failed, we expect to get the following error:
+ #
+ # svn: E155000: 'completely\unusable\dir' is not valid as filename
+ # in directory [...]
+ #
+ exit_code, output, errput = svntest.main.run_svn(1, 'up', wc_dir)
+ if exit_code == 0:
+ verify.verify_outputs("Unexpected output", output, errput, [
+ "Updating '%s':\n" % wc_dir,
+ "At revision 2.\n"
+ ], [])
expected_status = svntest.actions.get_virginal_state(wc_dir, 2)
svntest.actions.run_and_verify_status(wc_dir, expected_status)
+ elif exit_code == 1:
+ verify.verify_outputs("Unexpected output", output, errput,
+ None, 'svn: E155000: .* is not valid.*')
+ else:
+ raise verify.SVNUnexpectedExitCode(exit_code)
def update_moved_away(sbox):
"update subtree of moved away"
Re: svn commit: r1736432 -
/subversion/trunk/subversion/tests/cmdline/update_tests.py
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Bert Huijben wrote on Tue, Mar 29, 2016 at 15:05:21 +0200:
>
>
> > -----Original Message-----
> > From: kotkov@apache.org [mailto:kotkov@apache.org]
> > Sent: donderdag 24 maart 2016 12:59
> > To: commits@subversion.apache.org
> > Subject: svn commit: r1736432 -
> > /subversion/trunk/subversion/tests/cmdline/update_tests.py
> >
> > Author: kotkov
> > Date: Thu Mar 24 11:59:22 2016
> > New Revision: 1736432
> >
> > URL: http://svn.apache.org/viewvc?rev=1736432&view=rev
> > Log:
> > Fix a false negative in update_tests.py#76 with SVNPathAuthz short_circuit.
> >
> > This test used to assert that having a filename with backslash returns an
> > error during `svn update' over file:// or svn://, and doesn't do that over
> > http://. However, the last bit is specific to httpd instances configured
> > with SVNPathAuthz on, where a subrequest-based check gives 404 and
> > results
> > in the path being excluded by the server. With SVNPathAuthz short_circuit,
> > the behavior is the same as when the command is run over file:// or svn://
> > and results in an error:
> >
> > svn: E155000: 'completely\unusable\dir' is not valid as filename in [...]
> >
> > Since the original issue (SVN-3288) is about a crash, and given that we're
> > currently fine with either of the results, tweak the test to accept both of
> > them. There is an alternative of teaching the tests to distinguish between
> > different types of httpd authz and making the checks even more conditional,
> > but that seems a bit over the top.
>
> +1 on this change. Good fix.
>
> Perhaps we can extend it later on to verify in which mode we currently
> run the test suite (assuming the suite sets up the apache server)
We don't need detection to be complete, only to be sound.
davautocheck.sh sets up short-circuit mode if env['SVN_PATH_AUTHZ'] ==
'short_circuit', so we can have the test expect only the short_circuit
behaviour if that envvar is set to that value, and expect either
behaviour otherwise. (It seems that build/run_tests.py doesn't
currently support setting SVNPathAuthz.)
No opinion on whether we _should_ do this; just pointing out it's
possible.
Cheers,
Daniel
RE: svn commit: r1736432 - /subversion/trunk/subversion/tests/cmdline/update_tests.py
Posted by Bert Huijben <be...@qqmail.nl>.
> -----Original Message-----
> From: kotkov@apache.org [mailto:kotkov@apache.org]
> Sent: donderdag 24 maart 2016 12:59
> To: commits@subversion.apache.org
> Subject: svn commit: r1736432 -
> /subversion/trunk/subversion/tests/cmdline/update_tests.py
>
> Author: kotkov
> Date: Thu Mar 24 11:59:22 2016
> New Revision: 1736432
>
> URL: http://svn.apache.org/viewvc?rev=1736432&view=rev
> Log:
> Fix a false negative in update_tests.py#76 with SVNPathAuthz short_circuit.
>
> This test used to assert that having a filename with backslash returns an
> error during `svn update' over file:// or svn://, and doesn't do that over
> http://. However, the last bit is specific to httpd instances configured
> with SVNPathAuthz on, where a subrequest-based check gives 404 and
> results
> in the path being excluded by the server. With SVNPathAuthz short_circuit,
> the behavior is the same as when the command is run over file:// or svn://
> and results in an error:
>
> svn: E155000: 'completely\unusable\dir' is not valid as filename in [...]
>
> Since the original issue (SVN-3288) is about a crash, and given that we're
> currently fine with either of the results, tweak the test to accept both of
> them. There is an alternative of teaching the tests to distinguish between
> different types of httpd authz and making the checks even more conditional,
> but that seems a bit over the top.
+1 on this change. Good fix.
Perhaps we can extend it later on to verify in which mode we currently run the test suite (assuming the suite sets up the apache server)
Bert