Re: [PATCH] {maint} maintcheck: look for problematic names of testcases

[Ralf Wildenhues]

* Stefano Lattarini wrote on Mon, Mar 14, 2011 at 12:51:29PM CET:
> > Or maybe we could define a "maintainer-check" that looks at the
> > testcases' names and complains about suspicious ones...  I'll
> > give it a shot unless someone beats me.
> >
> Done in the attached patch.

OK with nits addressed.

Thanks,
Ralf

> Subject: [PATCH] maintcheck: look for problematic names of testcases
> 
> The configure.in stub created by default by `tests/defs' obtains
> the first argument of AC_INIT from the test name, and this can
> cause some supported autoconf versions to fail with a spurious
> error if that test name contains the name of an m4 builtin (e.g.,
> `defn' or `undefine').

I'd say "name of a macro", because I don't think it's only builtins that
are affected.  For example, AC_DEFUN wouldn't work either.  So, it would
be good to also test that we don't use prefixes _?A[CUMHS]_ or _?m4_
either (can be done in followup patch).

> See for example the bug fixed by commit v1.11-287-g1325a8a.
> 
> This change add a maintainer check that warns about test names
> which are possibly problematic in this regard.
> 
> * Makefile.am (sc_test_names): New maintainer-check target.
> (syntax_check_rules): Add it.
> (m4_builtins): New helper variable.
> (TESTS): Updated according to the following renamings.
> * tests/include.test: Renamed ...
> * tests/hdr-vars-defined-once.test: ... to this.
> * tests/sinclude.test: Renamed ...
> * tests/m4-inclusion.test: ... to this, and simplified
> accordingly.
> * tests/include2.test: Renamed ...
> * tests/dist-included-parent-dir.test: ... to this, for
> consistency.


> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -111,6 +111,7 @@ dist-hook:
>  # Some simple checks, and then ordinary check.  These are only really
>  # guaranteed to work on my machine.
>  syntax_check_rules = \
> +sc_test_names \
>  sc_diff_automake_in_automake \
>  sc_perl_syntax \
>  sc_no_brace_variable_expansions \
> @@ -151,6 +152,61 @@ sc_at_in_texi
>  $(syntax_check_rules): automake aclocal
>  maintainer-check: $(syntax_check_rules)
>  
> +## Look for test whose names can cause spurious failures when used as
> +## first argument to AC_INIT (chiefly because they might contain an
> +## m4/m4sugar builtins).

either "builtin" or s/ an//

> +m4_builtins = \
> +  bpatsubst \
> +  bregexp \
> +  builtin \
> +  changecom \
> +  changequote \
> +  changeword \
> +  debugfile \
> +  debugmode \
> +  decr \
> +  define \
> +  defn \
> +  divert \
> +  divnum \
> +  dumpdef \
> +  errprint \
> +  esyscmd \
> +  eval \
> +  format \
> +  ifdef \
> +  ifelse \
> +  include \
> +  incr \
> +  index \
> +  indir \
> +  len \
> +  maketemp \
> +  mkstemp \
> +  patsubst \
> +  popdef \
> +  pushdef \
> +  regexp \
> +  shift \
> +  sinclude \
> +  substr \
> +  symbols \
> +  syscmd \
> +  sysval \
> +  traceoff \
> +  traceon \
> +  translit \
> +  undefine \
> +  undivert
> +sc_test_names:
> +     @m4_builtins_rx=`echo $(m4_builtins) | sed 's/ /|/g'`; \
> +      m4_builtins_rx="\\<($$m4_builtins_rx)\\>|\\<_?m4[a-zA-Z0-9_]"; \
                                                      ^

This underscore plus question doesn't make sense to me.  Why not this?
  \\<m4_[a-zA-Z0-9]


> +      if ls tests/*.test | LC_ALL=C grep -E "$$m4_builtins_rx"; then \
> +        echo "the names of the tests above can be problematic" 1>&2; \
> +        echo "Avoid test names that contain names of m4 builtins" 1>&2; \
> +        exit 1; \
> +      fi
> +
>  ## This check avoids accidental configure substitutions in the source.
>  ## There are exactly 8 lines that should be modified.  This works out
>  ## to 28 lines of diffs.


> diff --git a/tests/include2.test b/tests/dist-included-parent-dir.test
> diff --git a/tests/include.test b/tests/hdr-vars-defined-once.test
> diff --git a/tests/sinclude.test b/tests/m4-inclusion.test