You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@apr.apache.org by jo...@apache.org on 2011/05/11 16:51:34 UTC
svn commit: r1101905 - /apr/apr/trunk/test/testfnmatch.c
Author: jorton
Date: Wed May 11 14:51:33 2011
New Revision: 1101905
URL: http://svn.apache.org/viewvc?rev=1101905&view=rev
Log:
* test/testfnmatch.c: Add a few more apr_fnmatch() tests to
improve (already good!) coverage a little.
(test_fnmatch_test): New test.
Modified:
apr/apr/trunk/test/testfnmatch.c
Modified: apr/apr/trunk/test/testfnmatch.c
URL: http://svn.apache.org/viewvc/apr/apr/trunk/test/testfnmatch.c?rev=1101905&r1=1101904&r2=1101905&view=diff
==============================================================================
--- apr/apr/trunk/test/testfnmatch.c (original)
+++ apr/apr/trunk/test/testfnmatch.c Wed May 11 14:51:33 2011
@@ -75,6 +75,24 @@ static struct pattern_s {
{"tes*", "test", SUCCEEDS},
{"test*", "test", SUCCEEDS},
+ {"test*?*[a-z]*", "testgoop", SUCCEEDS},
+ {"te[^x]t", "test", SUCCEEDS},
+ {"te[^abc]t", "test", SUCCEEDS},
+ {"te[^x]t", "test", SUCCEEDS},
+ {"te[!x]t", "test", SUCCEEDS},
+ {"te[^x]t", "text", FAILS},
+ {"te[^\\x]t", "text", FAILS},
+ {"te[^x\\", "text", FAILS},
+ {"te[/]t", "text", FAILS},
+ {"te[S]t", "test", SUCCEEDS_IF(APR_FNM_CASE_BLIND)},
+ {"te[r-t]t", "test", SUCCEEDS},
+ {"te[r-t]t", "teSt", SUCCEEDS_IF(APR_FNM_CASE_BLIND)},
+ {"te[r-T]t", "test", SUCCEEDS_IF(APR_FNM_CASE_BLIND)},
+ {"te[R-T]t", "test", SUCCEEDS_IF(APR_FNM_CASE_BLIND)},
+ {"te[r-Tz]t", "tezt", SUCCEEDS},
+ {"te[R-T]t", "tent", FAILS},
+ {"\\/test", "/test", FAILS_IF(APR_FNM_NOESCAPE)},
+
{"test/this", "test/", FAILS},
{"test/", "test/this", FAILS},
{"test*/this", "test/this", SUCCEEDS},
@@ -133,6 +151,35 @@ static void test_fnmatch(abts_case *tc,
}
}
+static void test_fnmatch_test(abts_case *tc, void *data)
+{
+ static const struct test {
+ const char *pattern;
+ int result;
+ } ft_tests[] = {
+ { "a*b", 1 },
+ { "a?", 1 },
+ { "a\\b?", 1 },
+ { "a[b-c]", 1 },
+ { "a", 0 },
+ { "a\\", 0 },
+ { NULL, 0 }
+ };
+ const struct test *t;
+
+ for (t = ft_tests; t->pattern != NULL; t++) {
+ int res = apr_fnmatch_test(t->pattern);
+
+ if (res != t->result) {
+ char buf[128];
+
+ sprintf(buf, "apr_fnmatch_test(\"%s\") = %d, expected %d\n",
+ t->pattern, res, t->result);
+ abts_fail(tc, buf, __LINE__);
+ }
+ }
+}
+
static void test_glob(abts_case *tc, void *data)
{
int i;
@@ -177,6 +224,7 @@ abts_suite *testfnmatch(abts_suite *suit
suite = ADD_SUITE(suite)
abts_run_test(suite, test_fnmatch, NULL);
+ abts_run_test(suite, test_fnmatch_test, NULL);
abts_run_test(suite, test_glob, NULL);
abts_run_test(suite, test_glob_currdir, NULL);
Re: svn commit: r1101905 - /apr/apr/trunk/test/testfnmatch.c
Posted by "William A. Rowe Jr." <wr...@rowe-clan.net>.
On 5/12/2011 11:26 AM, William A. Rowe Jr. wrote:
>
> I believe what is meant is mismatch + 1; []] should be valid to find ']', only
> as the leading character, and a quick test confirms it fails. But the exception
> is more optimal if handled ahead of while (**pattern)... fixing shortly.
... which wouldn't handle []-}] properly, for example. Not that it is absolutely
required, but is accepted on both linux and bsd. Will fix accordingly, this is a
target ripe for goto optimization.
Re: svn commit: r1101905 - /apr/apr/trunk/test/testfnmatch.c
Posted by "William A. Rowe Jr." <wr...@rowe-clan.net>.
On 5/12/2011 3:40 AM, Joe Orton wrote:
>
> File 'strings/apr_fnmatch.c'
> Lines executed:91.48% of 176
> Branches executed:95.40% of 261
> Taken at least once:81.61% of 261
> Calls executed:100.00% of 17
Can you email me the coverage detail report? It should identify further
optimizations.
> I had one question actually, if you have a spare moment ever, there was
> a branch I could not work out how to trigger in fnmatch_ch():
>
> if (**pattern == '[')
> {
> ++*pattern;
> ...
> while (**pattern)
> {
> /* ']' is an ordinary character at the start of the range pattern */
> if ((**pattern == ']') && (*pattern > mismatch)) {
>
> That last expression (*pattern > mismatch) looks like it should always
> evaluate to true; I could not work out why that test should be necessary.
I believe what is meant is mismatch + 1; []] should be valid to find ']', only
as the leading character, and a quick test confirms it fails. But the exception
is more optimal if handled ahead of while (**pattern)... fixing shortly.
Re: svn commit: r1101905 - /apr/apr/trunk/test/testfnmatch.c
Posted by Joe Orton <jo...@redhat.com>.
On Wed, May 11, 2011 at 10:46:54AM -0500, William Rowe wrote:
> On 5/11/2011 9:51 AM, jorton@apache.org wrote:
> > Author: jorton
> > Date: Wed May 11 14:51:33 2011
> > New Revision: 1101905
> >
> > URL: http://svn.apache.org/viewvc?rev=1101905&view=rev
> > Log:
> > * test/testfnmatch.c: Add a few more apr_fnmatch() tests to
> > improve (already good!) coverage a little.
> > (test_fnmatch_test): New test.
>
> Thanks for your work on this, Joe!
'tas but a trifle - thanks to *you* for slogging through this one, it
looked like a tough nut to crack.
> Note that I've been careful in prior tests to strike both match and
> notmatch at the start, middle, end and beyond end of the string tested
> (Jeff had tripped over one such edge case), and it's probably worth
> repeating that exercise with these regex patterns. Unfortunately I've
> been exhausted of cycles to take this any further, so it's great to
> see others add on to the cases.
OK, I will revisit that if I get a chance. I just wanted to make sure
some of the more obscure branches were getting tested; gcov says this
with the current code which is pretty good:
File 'strings/apr_fnmatch.c'
Lines executed:91.48% of 176
Branches executed:95.40% of 261
Taken at least once:81.61% of 261
Calls executed:100.00% of 17
I had one question actually, if you have a spare moment ever, there was
a branch I could not work out how to trigger in fnmatch_ch():
if (**pattern == '[')
{
++*pattern;
...
while (**pattern)
{
/* ']' is an ordinary character at the start of the range pattern */
if ((**pattern == ']') && (*pattern > mismatch)) {
That last expression (*pattern > mismatch) looks like it should always
evaluate to true; I could not work out why that test should be necessary.
When evaluating that line:
a) mismatch always points to the first char in the *pattern on entry to the fn
b) *pattern must have been incremented at least once since entry
c) therefore, *pattern > mismatch must always be true
Regards, Joe
Re: svn commit: r1101905 - /apr/apr/trunk/test/testfnmatch.c
Posted by "William A. Rowe Jr." <wr...@rowe-clan.net>.
On 5/11/2011 9:51 AM, jorton@apache.org wrote:
> Author: jorton
> Date: Wed May 11 14:51:33 2011
> New Revision: 1101905
>
> URL: http://svn.apache.org/viewvc?rev=1101905&view=rev
> Log:
> * test/testfnmatch.c: Add a few more apr_fnmatch() tests to
> improve (already good!) coverage a little.
> (test_fnmatch_test): New test.
Thanks for your work on this, Joe! Note that I've been careful in
prior tests to strike both match and notmatch at the start, middle,
end and beyond end of the string tested (Jeff had tripped over one
such edge case), and it's probably worth repeating that exercise
with these regex patterns. Unfortunately I've been exhausted of
cycles to take this any further, so it's great to see others add on
to the cases.