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

Re: svn commit: r958630 - in /subversion/trunk/subversion: libsvn_ra_local/split_url.c tests/libsvn_ra_local/ra-local-test.c

From: Greg Stein <gstein_at_gmail.com>
Date: Mon, 28 Jun 2010 14:47:56 -0400

This is just compounding the problem.

The test suite should NEVER cause an assertion. You simply are not
allowed to pass those arguments into the functions. That is why the
assertion is there. Neither the test suite, nor third parties should
pass those bad arguments. Period.

If you want to convert the assertions into checked failures, and
return an error on those failures... fine. But that is different from
an assertion. The assertion means "you should NOT have done this."

That isn't a testable case. That is flatly a wrong programming construction.

-g

On Mon, Jun 28, 2010 at 12:40, <julianfoad_at_apache.org> wrote:
> Author: julianfoad
> Date: Mon Jun 28 16:40:52 2010
> New Revision: 958630
>
> URL: http://svn.apache.org/viewvc?rev=958630&view=rev
> Log:
> Remove some code that was just to help the test suite, now that the test
> suite can catch SVN_ERR_ASSERT failures (since r958583).
>
> * subversion/libsvn_ra_local/split_url.c
>  (svn_ra_local__split_URL): Don't bother checking the URL before calling
>    svn_uri_get_dirent_from_file_url().
>
> * subversion/tests/libsvn_ra_local/ra-local-test.c
>  (split_url_syntax): Accept an assertion failure as an alternative to an
>    "illegal URL" error when testing a malformed URL.
>
> Modified:
>    subversion/trunk/subversion/libsvn_ra_local/split_url.c
>    subversion/trunk/subversion/tests/libsvn_ra_local/ra-local-test.c
>
> Modified: subversion/trunk/subversion/libsvn_ra_local/split_url.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_local/split_url.c?rev=958630&r1=958629&r2=958630&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_ra_local/split_url.c (original)
> +++ subversion/trunk/subversion/libsvn_ra_local/split_url.c Mon Jun 28 16:40:52 2010
> @@ -40,13 +40,6 @@ svn_ra_local__split_URL(svn_repos_t **re
>   const char *repos_root_dirent;
>   svn_stringbuf_t *urlbuf;
>
> -  /* Verify that the URL is well-formed (loosely) */
> -  /* First, check for the "file://" prefix, to avoid assertion in tests */
> -  if (strncmp(URL, "file://", 7) != 0)
> -    return svn_error_createf(SVN_ERR_RA_ILLEGAL_URL, NULL,
> -                             _("Local URL '%s' does not contain 'file://' "
> -                               "prefix"), URL);
> -
>   SVN_ERR(svn_uri_get_dirent_from_file_url(&repos_dirent, URL, pool));
>
>   /* Search for a repository in the full path. */
>
> Modified: subversion/trunk/subversion/tests/libsvn_ra_local/ra-local-test.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_ra_local/ra-local-test.c?rev=958630&r1=958629&r2=958630&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/tests/libsvn_ra_local/ra-local-test.c (original)
> +++ subversion/trunk/subversion/tests/libsvn_ra_local/ra-local-test.c Mon Jun 28 16:40:52 2010
> @@ -148,7 +148,7 @@ split_url_syntax(apr_pool_t *pool)
>
>   /* Use only single slash after scheme */
>   apr_err = try_split_url("file:/path/to/repos", pool);
> -  if (apr_err != SVN_ERR_RA_ILLEGAL_URL)
> +  if (apr_err != SVN_ERR_RA_ILLEGAL_URL && apr_err != SVN_ERR_ASSERTION_FAIL)
>     return svn_error_create
>       (SVN_ERR_TEST_FAILED, NULL,
>        "svn_ra_local__split_URL failed to catch bad URL (slashes)");
>
>
>
Received on 2010-06-28 20:48:35 CEST

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