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

Re: [PATCH] add a test of svn_subst_translate_string2() to the subst_translate-test test suite (v. 3)

From: Daniel Shahaf <d.s_at_daniel.shahaf.name>
Date: Tue, 22 Feb 2011 18:09:22 +0200

Thanks, r1073377. See below for some comments.

Danny Trebbien wrote on Sun, Feb 20, 2011 at 09:58:07 -0800:
> Attached is version 3 of the patch and log message.
>
>
> >> >> +        (! setlocale(LC_ALL, "en_US.ISO-8859-1")) &&
> >> >> +        (! setlocale(LC_ALL, "en_GB.ISO-8859-1")) &&
> >> >> +        (! setlocale(LC_ALL, "de_DE.ISO-8859-1")))
> >> >> +      return svn_error_createf(SVN_ERR_TEST_SKIPPED, NULL, "None of the locales English.1252, German.1252, French.1252, en_US.ISO-8859-1, en_GB.ISO-8859-1, and de_DE.ISO-8859-1 are installed.");
...
> > So, sorry, but I'm not yet convinced that listing all the locales in the
> > log message is a good idea.
>
> Okay. I changed it to "Tried %d locales, but none are installed."
>

Thanks.

> >> +test_svn_subst_translate_string2_null_encoding_helper(apr_pool_t *pool)
> >> +{
> >> +  {
> >> +    svn_string_t *new_value = NULL;
> >> +    svn_boolean_t translated_to_utf8 = FALSE;
> >> +    svn_boolean_t translated_line_endings = TRUE;
> >
> > Why did you initialize these two variables?  The API will always set
> > them for you, so you shouldn't initialize them.
>
> It's part of the test to check that the API sets them.
>

Fair enough.

> [[[
> Add a test of svn_subst_translate_string2() to the subst_translate test suite.
>
> As discussed at:
> http://thread.gmane.org/gmane.comp.version-control.subversion.devel/125782
> Message-ID: <AANLkTimo=NpF_FqB+PBPkeifXjbnsORGYkZ5npCAxyVz_at_mail.gmail.com>
>
> * subversion/tests/libsvn_subr/subst_translate-test.c
> (test_svn_subst_translate_string2_null_encoding_helper): New function. It is
> the core of the new svn_subst_translate_string2_null_encoding test.
> (test_svn_subst_translate_string2_null_encoding): New test that tests
> svn_subst_translate_string2() with ENCODING set to NULL.
> (test_funcs): Add test_svn_subst_translate_string2_null_encoding().
> ]]]

I tweaked this a bit (e.g., adding ARRAY_LEN() which you neglected to).

> +++ subversion/tests/libsvn_subr/subst_translate-test.c (working copy)
> +/* The body of the svn_subst_translate_string2_null_encoding test. It should
> + only be called by test_svn_subst_translate_string2_null_encoding(), as this
> + code assumes that the process locale has been changed to a locale that uses
> + either CP-1252 or ISO-8859-1 for the default narrow string encoding. */
> static svn_error_t *
> +test_svn_subst_translate_string2_null_encoding_helper(apr_pool_t *pool)

"narrow" string? As opposed to a "wide-character" string?

> +test_svn_subst_translate_string2_null_encoding(apr_pool_t *pool)
> +{
> + char orig_lc_all[256] = { '\0' };
> + svn_error_t *test_result;
...
> + strncpy(orig_lc_all, setlocale(LC_ALL, NULL), sizeof (orig_lc_all));
> +

Typically we use pools (apr_pstrdup()). I haven't changed this.
Received on 2011-02-22 17:14:45 CET

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