[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: AT_BANNER handling
From: |
Ralf Wildenhues |
Subject: |
Re: AT_BANNER handling |
Date: |
Fri, 19 Oct 2007 19:17:07 +0200 |
User-agent: |
Mutt/1.5.13 (2006-08-11) |
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