Re: factor tests: ugly long file names

[Jim Meyering]

Stefano Lattarini wrote:

> Hi Jim.
>
> On 10/27/2012 11:14 AM, Jim Meyering wrote:
>> Stefano Lattarini wrote:
>>>>> You can save a fork by avoiding a call to basename:
>>>>>
>>>>>     t=${testname##*/}
>>>>
>>>> Good point. I wasn't sure if that syntax is portable.
>>>> I know it from old dtksh days.
>>>>
>>> It is portable to any decent POSIX shell.  And I think Coreutils assumes
>>> that one is available on the build system.  But then, to ensure such a
>>> shell is actually used, you'll have to invoke your script explicitly with
>>> the configure-determined $(SHELL) from the make recipe below (thank you
>>> Solaris for keeping an old non-POSIX shell as /bin/sh, sigh).
>>>
>>> Jim, can you confirm it's OK to assume that $(SHELL) points to a POSIX
>>> shell in the coreutils build system?
>>
>> It's even better than that.
>> coreutils (via init.sh) guarantees that the shell
>> used to invoke tests accepts $(...) syntax, so we don't
>> have to use the anachronistic `...`.
>>
> Ah, but it seems to me that 'tests/factor/create-test.sh' doesn't
> use 'tests/init.sh' -- nor I'm sure it can, being 'tests/init.sh'
> designed to be sourced only by test scripts :-(

Hi Stefano,

You're right.
create-test.sh is different from the others.
It is subject to the regular (strict) portability rules.
So we can *not* use that syntax there.

> It would be really nice if Autoconf 2.70 would finally ensure that
> the $SHELL it sets is a true POSIX shell ...  See also:
> <http://lists.gnu.org/archive/html/bug-autoconf/2012-06/msg00009.html>

I think that is the way to go.
Trying to identify and remember so many vagaries of shell portability
is a losing battle.

>> It appears that every shell meeting the above criteria also
>> supports ${var##X}, ${var#X}, ${var%%X}, etc. substitution,
>> since this use has been in a test since 2007:
>>
>>     symlink_loop_msg=${readlink_msg#'readlink: p/1: '}
>>
>> init.sh explicitly requires ${var#X} support and a couple
>> other shell features when $(EXEEXT) is nonempty, but I doubt
>> you're building on mingw.
>>
>> So, yes, it is safe to use that syntax in coreutils tests/ scripts.
>>
> Is this true even in light of my observation above?

It is true for all tests/ scripts except create-test.sh.

Berny,

Here are some changes I'd appreciate:
  - shorten a regexp
  - remove two mentions of tests/factor/run.sh in create-test.sh
  - change create-test.sh to take 2 arguments
  - change $testname to $test_name; more readable
  - remove useless curly braces around ${t1} ${t2}.
      The other uses are required, but not those.
  - avoid the need for backslash-escaped slashes in TEMPLATE
  - simply exec "sed"
  - remove the execute bit on run.sh
  - "never redirect directly to $@" (repeat this mantra ;-)

diff --git a/cfg.mk b/cfg.mk
index 6155ec2..54ffde1 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -111,7 +111,7 @@ sc_tests_list_consistency:
          for t in $(all_tests); do echo $$t; done;                     \
          cd $(top_srcdir);                                             \
          $(SHELL) build-aux/vc-list-files tests                        \
-           | grep -Ev '^tests/(factor/run|factor/create-test|init)\.sh$$' \
+           | grep -Ev '^tests/(factor/(run|create-test)|init)\.sh$$'   \
            | $(EGREP) "$$test_extensions_rx\$$";                       \
        } | sort | uniq -u | grep . && exit 1; :

diff --git a/tests/factor/create-test.sh b/tests/factor/create-test.sh
index 907396f..180e89a 100755
--- a/tests/factor/create-test.sh
+++ b/tests/factor/create-test.sh
@@ -1,13 +1,13 @@
 #!/bin/sh
 # Create the factor test scripts.
-# Used template: tests/factor/run.sh.

 # Copyright (C) 2012 Free Software Foundation, Inc.

-# Extract the test name from $1.
-testname="$1"
-t="${testname##*/}"
-t="${t%.sh}"
+test_name=$1
+template=$2
+
+# Extract the test name: remove .sh suffix from the basename.
+t=`echo "$test_name"|sed 's,.*/,,;s,\.sh$,,'`

 # prefix of 2^64
 p=184467440737
@@ -38,8 +38,8 @@ case $t in
   t06) set     60000000     70000000 689bc70d681791e5d1b8ac1316a05d0c4473d6db 
;;
   t07) set     70000000     80000000 d370808f2ab8c865f64c2ff909c5722db5b7d58d 
;;
   t08) set     80000000     90000000 7978aa66bf2bdb446398336ea6f02605e9a77581 
;;
-  t09) set        ${t1}        ${t1} 4622287c5f040cdb7b3bbe4d19d29a71ab277827 
;;
-  t10) set        ${t2}        ${t2} dea308253708b57afad357e8c0d2a111460ef50e 
;;
+  t09) set          $t1          $t1 4622287c5f040cdb7b3bbe4d19d29a71ab277827 
;;
+  t10) set          $t2          $t2 dea308253708b57afad357e8c0d2a111460ef50e 
;;
   t11) set ${p}08551616 ${p}08651615 66c57cd58f4fb572df7f088d17e4f4c1d4f01bb1 
;;
   t12) set ${p}08651616 ${p}08751615 729228e693b1a568ecc85b199927424c7d16d410 
;;
   t13) set ${p}08751616 ${p}08851615 5a0c985017c2d285e4698f836f5a059e0b684563 
;;
@@ -67,20 +67,17 @@ case $t in
   t35) set   ${q}958336   ${q}960335 2374919a89196e1fce93adfe779cb4664556d4b6 
;;
   t36) set   ${q}960336   ${q}962335 569e4363e8d9e8830a187d9ab27365eef08abde1 
;;
   *)
-    echo "$0: error: unknown test: '$testname' -> '$t'" >&2
+    echo "$0: error: unknown test: '$test_name' -> '$t'" >&2
     exit 1
     ;;
 esac

-TEMPLATE="TEST SCRIPT DERIVED FROM THE TEMPLATE tests\/factor\/run.sh"
+TEMPLATE="TEST SCRIPT DERIVED FROM THE TEMPLATE $template"

 # Create the test script from the template for this test
 # by substituting the START, the END and the CKSUM.
-sed \
+exec sed \
   -e "s/__START__/$1/" \
   -e "s/__END__/$2/" \
   -e "s/__CKSUM__/$3/" \
-  -e "s/__TEMPLATE__/$TEMPLATE/" \
-  || exit 1
-
-exit 0
+  -e "s!__TEMPLATE__!$TEMPLATE!" "$template"
diff --git a/tests/factor/run.sh b/tests/factor/run.sh
old mode 100755
new mode 100644
diff --git a/tests/local.mk b/tests/local.mk
index 9ee696a..8fc4030 100644
--- a/tests/local.mk
+++ b/tests/local.mk
@@ -640,10 +640,10 @@ factor_tests = \

 $(factor_tests): $(tf)/run.sh $(tf)/create-test.sh
        $(AM_V_GEN)$(MKDIR_P) $(tf)
-       $(AM_V_at)$(srcdir)/$(tf)/create-test.sh $@ \
-         < $(srcdir)/$(tf)/run.sh \
-         > $@
-       @chmod a+x $@
+       $(AM_V_at)$(SHELL) $(srcdir)/$(tf)/create-test.sh $@ \
+         $(srcdir)/$(tf)/run.sh > $@-t
+       $(AM_V_at)chmod a+x $@-t
+       $(AM_V_at)mv $@-t $@

 CLEANFILES += $(factor_tests)