stdcxx-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Anton Pevtsov <ant...@moscow.vdiweb.com>
Subject Re: Re: test for 21.string::replace
Date Fri, 17 Mar 2006 16:34:00 GMT
The test is comitted here:
http://svn.apache.org/viewcvs?rev=386577&view=rev

Oops, I forgot to set properties. Here is the corrected version:
http://svn.apache.org/viewcvs?rev=386584&view=rev
Sorry about that.

I modified it according to your notes below. The patch is attached. Here
is the ChangeLog:

2006-03-17  Anton Pevtsov  <antonp@moscow.vdiweb.com>

	* 21.string.replace.cpp (ReplaceData): Removed as obsolete
	(TestCase): New structure to store a test case
	(test_cases): Static array of the test cases
	(test_replace): Functions updated to use TestCase and
	test_cases[] instead of ReplaceData.


Martin Sebor wrote:

>> I agree that trying to allocate huge amounts of memory is not a good 
>> way to test the class.
>  
>
[...]

>> A more reliable way to do almost the same thing is to exercise the 
>> template on a user-defined traits or allocator type. The allocator
>  
>
approach is better since basic_string::max_size() uses its
allocator_type::max_size(). The (theoretical) downside to this is that
if basic_string were explicitly specialized on char and wchar_t in the
library (as opposed to being explicitly instantiated as it is
now) the technique won't exercise the same same code in the char and
wchar_t case.

I think it would be useful to exercise the string class using our custom
allocator in any case.  


Thanks,
Anton Pevtsov


-----Original Message-----
From: Martin Sebor [mailto:sebor@roguewave.com] 
Sent: Friday, March 17, 2006 05:42
To: stdcxx-dev@incubator.apache.org
Subject: Re: test for 21.string::replace


Anton Pevtsov wrote:

>> The attached file contains new version of the test for
>> string::replace.
>  
>

Excellent!


>> You might want to add tests cases to be sure that the test covers all
>> possible cases.
>  
>

I'll look into it. I will definitely want to enhance the tests with long
strings, maybe even generate a few. This is the most important function
in all of basic_string.


>> 
>> And there are two issues with this test:
>> 
>> 1. It contains workaround for empty strings with UserChars formatting
>> output. The cause is in the implementation of the %{#*S} directive: it
>  
>

>> tries to cast basic_string<UserChar, ... > to string<char, ...> then

>> gets incorrect string size and finally fails. As a stub it is possible
>  
>

>> to ignore "unknown" strings, but some callback for this case will be 
>> more useful. What do you think about it?
>  
>

I'm hoping to come up with a generic formatting directive that can be
used with all three types of strings: char*, wchar_t*, and UserChar*.
Something along the lines of the _rw_fmtxarrayv() function in
alg_test.cpp that does the formatting of arrays of X, but a little bit
more generic (i.e., with an additional argument telling the function if
it's dealing with char*, wchar_t*, or UserChar*).


>> 
>> 2. There is no test case which results in the length_error exception
>> thrown by replace. The possible way to test this case requires the 
>> huge string with length about SIZE_MAX / 2 and I think this not good 
>> idea. I think we can try to use our own "string" class with modified 
>> size() method to emulate this case, but will this be the appropriate 
>> test?
>  
>

I agree that trying to allocate huge amounts of memory is not a good way
to test the class. We can (and should), however, emulate low memory
conditions by setting a lower memory limit by calling setrlimit():
http://www.opengroup.org/onlinepubs/009695399/functions/getrlimit.html
That's something different than what you're talking about, though, as
that will induce a bad_alloc exception and not length_error.

It might be possible to explicitly specialize a member function of
basic_string (such as max_size) in the test to try to induce an
exception sooner. I believe some of our tests actually do that (I have
written them). Unfortunately, this technique is not safe since the
template is already instantiated in the library (both explicitly as well
as implicitly by virtue of being used in locale). Doing so violates the
One Definition Rule (one definition of the function is in the library
and another in the test) and may be detected by some compilers (e.g.,
EDG eccp detects it in "export" mode).

A more reliable way to do almost the same thing is to exercise the
template on a user-defined traits or allocator type. The allocator
approach is better since basic_string::max_size() uses its
allocator_type::max_size(). The (theoretical) downside to this is that
if basic_string were explicitly specialized on char and wchar_t in the
library (as opposed to being explicitly instantiated as it is
now) the technique won't exercise the same same code in the char and
wchar_t case.

Please go ahead and commit it as it is and let's make improvements
(including those below) as the second pass.

One minor thing I'd like to adjust is the format of some of the
diagnostics. Instead of

# TEXT: line 354. std::basic_string<char, char_traits<char>, 
allocator<char>>("abc").replace (0, 2, "cc") == "ccdefgc", "ccdefgc"
expected

I would prefer to see

# TEXT: line 354. std::basic_string<char, char_traits<char>, 
allocator<char>>("abc").replace (0, 2, "cc") == "ccdefgc", got "ccdefgc"

I.e., the string on the right hand side of == should be the expected
result, with the actual result to follow after ", got"

I also think that it might be more space efficient (in terms of a
significantly reduced number of function calls) in this case to create a
data structure for each test case and build an array of them to store
them all and then simply iterate over the array one element at a time.
It should be easy to make this change and see how much of a space
difference it made.

Here's what I have in mind:

     static const struct {
         int line;
         const char *str;
         int         pos1;
         int         num1;
         const char* src;
         int         pos2;
         int         num2;
         int         cnt;
         int         ch;
         const char* res;
         int         it_res;
         int         bthrow;
     } test_cases[] = {
#define TEST(str, pos1, num1, src, pos2, num2, cnt, \
             ch, res, it_res, bthrow) \
      { __LINE__, str, pos1, num1, src, pos2, num2, cnt,  \
        ch, res, it_res, bthrow }

         TEST ("ab",  0, 1, "c", 0, 1, 2, 'c', "cb", "cb", 0);
         ...
     };

     static void test_replace () {
         //...
         const std::size_t ncases =
             sizeof test_cases / sizeof *test_cases;

         for (std::size_t i = 0; i != ncases; ++i)
             test_replace (pfid, tets_case [i].line, which, ...);
     }

Thanks
Martin


Mime
View raw message