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 * 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 to string 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, allocator>("abc").replace (0, 2, "cc") == "ccdefgc", "ccdefgc" expected I would prefer to see # TEXT: line 354. std::basic_string, allocator>("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