Re: AT_BANNER handling

[Ralf Wildenhues]

* Eric Blake wrote on Sat, Oct 20, 2007 at 06:00:20AM CEST:
> According to Ralf Wildenhues on 10/19/2007 11:17 AM:
> > 
> > Your patch breaks
> >   ./testsuite -10 --list
> 
> I'll look into it;[...]

Thanks for fixing it!

> >  at_format='m4_bpatsubst(m4_defn([AT_ordinal]), [.], [?])'
> >  # Description of all the test groups.
> > -at_help_all="AS_ESCAPE(m4_dquote(m4_defn([AT_help_all])))"])])dnl
> > +at_help_all="AS_ESCAPE(m4_dquote(m4_defn([AT_help_all])))"
> > +
> > +# at_func_validate_ranges [N...]
> > +# ------------------------------
> > +# validate test group ranges
> > +at_func_validate_ranges ()
> 
> Does this entire shell function have to live in the m4_wrap, or should we
> use the m4_wrap to capture the final value of AT_ordinal into a shell
> variable, and then the function can just reference the shell variable?  I
> don't have a strong opinion either way, so I guess just leave your patch
> the way it is.

FWIW I don't have a strong opinion either.

> >      # Ranges
> >      [[0-9]- | [0-9][0-9]- | [0-9][0-9][0-9]- | [0-9][0-9][0-9][0-9]-])
> >     at_range_start=`echo $at_option |tr -d X-`
> > +   at_func_validate_ranges $at_range_start
> >     at_range=`AS_ECHO([" $at_groups_all "]) | \
> >       sed -e 's/^.* \('$at_range_start' \)/\1/'`
> 
> Not your fault, but should we simplify and remove the -e from the sed
> invocation?

Saving three bytes vs. having similar scripts in the three range cases?
I don't see a huge win there.

FWIW, I had to fix one of my tests for the --list breakage, and remove
one of the Banners tests that failed now.  This is what I pushed.

Cheers,
Ralf

2007-10-20  Ralf Wildenhues  <address@hidden> 

        * lib/autotest/general.m4 (Defaults): Validate input ranges ...
        <at_func_validate_ranges>: ... using this new function.
        * tests/autotest.at (Keywords and ranges): Test invalid ranges.
        Test --list with ranges and keywords.
        (Banners): Remove one now-failing test.

diff --git a/lib/autotest/general.m4 b/lib/autotest/general.m4
index aa29202..29834f1 100644
--- a/lib/autotest/general.m4
+++ b/lib/autotest/general.m4
@@ -430,7 +430,21 @@ at_groups_all='AT_groups_all'
 # numerical order.
 at_format='m4_bpatsubst(m4_defn([AT_ordinal]), [.], [?])'
 # Description of all the test groups.
-at_help_all="AS_ESCAPE(m4_dquote(m4_defn([AT_help_all])))"])])dnl
+at_help_all="AS_ESCAPE(m4_dquote(m4_defn([AT_help_all])))"
+
+# at_func_validate_ranges [N...]
+# ------------------------------
+# validate test group ranges
+at_func_validate_ranges ()
+{
+  for at_grp
+  do
+    if test $at_grp -lt 1 || test $at_grp -gt AT_ordinal; then
+      AS_ECHO(["invalid test group: $at_grp"]) >&2
+      exit 1
+    fi
+  done
+}])])dnl
 m4_divert_push([PARSE_ARGS])dnl
 
 at_prev=
@@ -487,12 +501,14 @@ do
        ;;
 
     [[0-9] | [0-9][0-9] | [0-9][0-9][0-9] | [0-9][0-9][0-9][0-9]])
+       at_func_validate_ranges $at_option
        at_groups="$at_groups$at_option "
        ;;
 
     # Ranges
     [[0-9]- | [0-9][0-9]- | [0-9][0-9][0-9]- | [0-9][0-9][0-9][0-9]-])
        at_range_start=`echo $at_option |tr -d X-`
+       at_func_validate_ranges $at_range_start
        at_range=`AS_ECHO([" $at_groups_all "]) | \
          sed -e 's/^.* \('$at_range_start' \)/\1/'`
        at_groups="$at_groups$at_range "
@@ -500,6 +516,7 @@ do
 
     [-[0-9] | -[0-9][0-9] | -[0-9][0-9][0-9] | -[0-9][0-9][0-9][0-9]])
        at_range_end=`echo $at_option |tr -d X-`
+       at_func_validate_ranges $at_range_end
        at_range=`AS_ECHO([" $at_groups_all "]) | \
          sed -e 's/\( '$at_range_end'\) .*$/\1/'`
        at_groups="$at_groups$at_range "
@@ -518,6 +535,7 @@ do
          at_range_end=$at_range_start
          at_range_start=$at_tmp
        fi
+       at_func_validate_ranges $at_range_start $at_range_end
        at_range=`AS_ECHO([" $at_groups_all "]) | \
          sed -e 's/^.*\( '$at_range_start' \)/\1/' \
              -e 's/\( '$at_range_end'\) .*$/\1/'`
diff --git a/tests/autotest.at b/tests/autotest.at
index 75a6015..ea84c18 100644
--- a/tests/autotest.at
+++ b/tests/autotest.at
@@ -439,7 +439,6 @@ AT_CHECK_BANNERS([-k a],    [first], [1], [second], [1])
 AT_CHECK_BANNERS([4],          [first], [0], [second], [0])
 AT_CHECK_BANNERS([4-],         [first], [0], [second], [1])
 AT_CHECK_BANNERS([-k two],     [first], [0], [second], [0])
-AT_CHECK_BANNERS([0 3],                [first], [0], [second], [0])
 AT_CHECK_BANNERS([-k three],   [first], [0], [second], [1])
 AT_CHECK_BANNERS([5],          [first], [0], [second], [0])
 AT_CHECK_BANNERS([5-],         [first], [0], [second], [1])
@@ -515,6 +514,18 @@ AT_CHECK_KEYS([1-3 2-1], [none|first|second], [3], [both], 
[0])
 AT_CHECK_KEYS([-3], [none|first|second], [3], [both], [0])
 AT_CHECK_KEYS([4-], [both], [1], [none|first|second], [0])
 AT_CHECK_KEYS([-k second 4-], [second|both], [2], [none|first], [0])
+
+AT_CHECK([./k 0], [1], [ignore], [ignore])
+AT_CHECK([./k 0-], [1], [ignore], [ignore])
+AT_CHECK([./k -0], [1], [ignore], [ignore])
+AT_CHECK([./k 5], [1], [ignore], [ignore])
+AT_CHECK([./k 5-], [1], [ignore], [ignore])
+AT_CHECK([./k 1-5], [1], [ignore], [ignore])
+AT_CHECK([./k -k nonexistent], [0], [ignore])
+
+AT_CHECK_KEYS([--list -k nonexistent], [KEYWORDS], [1], [first|second|both], 
[0])
+AT_CHECK_KEYS([--list 1], [none], [1], [first|second|both], [0])
+AT_CHECK_KEYS([--list -k none -k first], [none|first], [2], [second|both], [0])
 AT_CLEANUP