Senthil Kumaran S wrote:
> [[[
> Fix lock_test failure in dav.
>
> * subversion/tests/cmdline/lock_tests.py
> (unlock_wrong_token, unlocked_lock_of_other_user): fix test failure due
> to wrong exit code expectation for http errors.
>
> Patch by: stylesen
> ]]]
I am sorry that I committed a patch that fails regression testing. (I only ran
the basic "make check".)
> ------------------------------------------------------------------------
>
> Index: subversion/tests/cmdline/lock_tests.py
> ===================================================================
> --- subversion/tests/cmdline/lock_tests.py (revision 29765)
> +++ subversion/tests/cmdline/lock_tests.py (working copy)
> @@ -1344,10 +1344,17 @@
> # Then, unlocking the WC path should fail.
> # ### The error message returned is actually this, but let's worry about that
> # ### another day...
> - svntest.actions.run_and_verify_svn2(
> - None, None, ".*((No lock on path)|(400 Bad Request))", 0,
> - 'unlock', file_path)
> + if sbox.repo_url.startswith("http"):
> + expected_err = ".*(400 Bad Request).*"
> + exit_code = 1
> + else:
> + expected_err = ".*(No lock on path).*"
> + exit_code = 0
Why should this command return a failure code on one RA layer and a success
code on another? It seems to me that Subversion is wrong, not the test.
We could say, "This is only a regression test, so it should test for the
current behaviour." I don't much like that: I would prefer either for it to
ignore the exit code, or preferably for us to fix Subversion right now to be
consistent.
If we do what your patch does, then please insert a comment saying "### This is
merely testing for the current behaviour of exit codes, not the desired behaviour."
The same applies to the error messages, of course: they should be consistent too.
> #----------------------------------------------------------------------
> # Verify that info shows lock info for locked files with URI-unsafe names
> # when run in recursive mode.
> @@ -1401,10 +1408,12 @@
> # now try to unlock with user jconstant, should fail but exit 0.
That comment about "exit 0" doesn't match the code.
> if sbox.repo_url.startswith("http"):
> expected_err = ".*403 Forbidden.*"
> + exit_code = 1
> else:
> expected_err = "svn: warning: User '%s' is trying to use a lock owned by "\
> "'%s'.*" % (svntest.main.wc_author2, svntest.main.wc_author)
> - svntest.actions.run_and_verify_svn2(None, [], expected_err, 0,
> + exit_code = 0
> + svntest.actions.run_and_verify_svn2(None, [], expected_err, exit_code,
> 'unlock',
> '--username', svntest.main.wc_author2,
> pi_path)
Same questions as above.
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-03-07 15:18:31 CET