Re: AT_BANNER handling

[Ralf Wildenhues]

Hello Eric,

Your patch breaks
  ./testsuite -10 --list

see below for tests.  Apologies if that was already broken in the patch
I reviewed, and I failed to see that.

* Eric Blake wrote on Fri, Oct 19, 2007 at 03:04:27PM CEST:
> 
> Slick idea.  Done.  The only minor drawback is that we lost the *) branch
> of the case, which gave us the ability to detect bugs in calling
> at_func_banner with a non-existent banner number.  But we shouldn't have
> those bugs in the first place, besides, the new code just reads a normally
> uninitialized variable; which is harmless unless the user pollutes the at_
> namespace in their environment.

I'm not worrying about that too much.  However, checks for passing
correct testsuite numbers are also weaker now than before.
  ./testsuite 999

now outputs
| 1 test was successful.

It used to print
| testsuite: no such test group: 9999
[...]
| 0 tests were successful.

which was better, but still exited with 0.  However, this:
  ./testsuite 9999

(back then with 2.59, and now) outputs
| testsuite: invalid option: 99999
| Try `./testsuite --help' for more information.

and exits with 1, so there is room for improvement here.  I'm thinking
of something like the following.  AFAICS it requires another place for
shell functions.  The testsuite additions also expose the breakage above
(see the last two tests).

OK?  Feel free to split off the last two tests to the commit that fixes
them.  ;-)

Cheers,
Ralf

2007-10-19  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.

diff --git a/lib/autotest/general.m4 b/lib/autotest/general.m4
index 012633c..9bb3575 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..3da1500 100644
--- a/tests/autotest.at
+++ b/tests/autotest.at
@@ -515,6 +515,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,first], [none|first], [2], [second|both], [0])
 AT_CLEANUP