[PATCH] tests: improve readability

[dana <dana@xxxxxxx>]

mikael and i were talking about how the test output is not as readable
as we'd like. this is my attempt at improving it:

- add blank line between each test script execution
- colourise diffs
- use fake file names to clarify what diffs mean
- colourise test-script failure messages
- colourise summary
- add list of failing test scripts to summary

colours only show when appropriate

imo the blank lines help a lot when there's a failing test, but i can
see how they might feel superfluous when they all pass

open to feedback

dana


diff --git a/Test/runtests.zsh b/Test/runtests.zsh
index 538663f50..9fe694a60 100644
--- a/Test/runtests.zsh
+++ b/Test/runtests.zsh
@@ -8,21 +8,55 @@ emulate zsh
 # We could probably do that with subshells instead.
 
 integer success=0 failure=0 skipped=0 retval
+typeset resln rule col
+typeset -a ffiles res
+
+# colour to use for ztst failure messages. also used as a signal that colours
+# are available generally
+[[ -t 1 ]] &&
+(( ! $+ZTST_failcolour )) &&
+[[ -z $NO_COLOR ]] &&
+[[ $TERM == *color* || "$( tput colors 2> /dev/null )" == <8-> ]] &&
+typeset -x ZTST_failcolour=red
+
 for file in "${(f)ZTST_testlist}"; do
+  file=${file#./}
   $ZTST_exe +Z -f $ZTST_srcdir/ztst.zsh $file
   retval=$?
   if (( $retval == 2 )); then
     (( skipped++ ))
   elif (( $retval )); then
     (( failure++ ))
-    (( $retval > 128 )) && print "$file: failed: SIG$signals[$retval - 127]."
+    (( $retval > 128 )) && print -r - "$file: failed: SIG$signals[$retval - 127]."
+    ffiles+=( $file )
   else
     (( success++ ))
   fi
+  print
 done
-print "**************************************
-$success successful test script${${success:#1}:+s}, \
-$failure failure${${failure:#1}:+s}, \
-$skipped skipped
-**************************************"
+
+res=(
+  "$success successful test script${${success:#1}:+s}"
+  "$failure failure${${failure:#1}:+s}"
+  "$skipped skipped"
+)
+resln=${(j<, >)res}
+rule=${resln//?/\*}
+
+[[ -n $ZTST_failcolour ]] &&
+if (( failure )); then
+  col=red
+elif (( success )); then
+  col=green
+else
+  col=yellow
+fi
+
+(( $#col )) && print -rnP - "%F{$col}"
+print -rl - $rule $resln
+(( $#ffiles )) &&
+print -rl - '' "failing test script${${#ffiles:#1}:+s}:" ${(@)ffiles/#/  }
+print -rl - $rule
+(( $#col )) && print -rnP - %f
+
 return $(( failure ? 1 : 0 ))
diff --git a/Test/ztst.zsh b/Test/ztst.zsh
index aeb02cd54..ae5133e4d 100755
--- a/Test/ztst.zsh
+++ b/Test/ztst.zsh
@@ -148,7 +148,9 @@ ZTST_testfailed() {
   if [[ -n $ZTST_message ]]; then
     print -r "Was testing: $ZTST_message"
   fi
+  [[ -n $ZTST_failcolour ]] && print -rnP "%F{$ZTST_failcolour}"
   print -r "$ZTST_testname: test failed."
+  [[ -n $ZTST_failcolour ]] && print -rnP %f
   if [[ -n $ZTST_failmsg ]]; then
     print -r "The following may (or may not) help identifying the cause:
 $ZTST_failmsg"
@@ -165,7 +167,9 @@ ZTST_testxpassed() {
   if [[ -n $ZTST_message ]]; then
     print -r "Was testing: $ZTST_message"
   fi
+  [[ -n $ZTST_failcolour ]] && print -rnP "%F{$ZTST_failcolour}"
   print -r "$ZTST_testname: test XPassed."
+  [[ -n $ZTST_failcolour ]] && print -rnP %f
   if [[ -n $ZTST_failmsg ]]; then
     print -r "The following may (or may not) help identifying the cause:
 $ZTST_failmsg"
@@ -326,7 +330,6 @@ ZTST_diff() {
   emulate -L zsh
   setopt extendedglob
 
-  local -a diff_arg
   local diff_out
   integer diff_pat diff_ret
 
@@ -343,8 +346,38 @@ ZTST_diff() {
     ;;
   esac
   shift
-  [[ $OSTYPE != (aix|solaris)* ]] && diff_arg=( -a )
-      
+
+  (( $+functions[ZTST_do_diff] )) ||
+  ZTST_do_diff() {
+    local MATCH MBEGIN MEND ret
+    local -a out cmd
+
+    cmd=( command diff -u )
+    [[ $OSTYPE != (aix|solaris)* ]] && cmd+=( -a )
+
+    out=( ${(f)"$( $cmd "$@" )"} ) || ret=$?
+
+    # massage + colourise diff
+    (( ret )) && {
+      local B=${(%):-%B} b=${(%):-%b} f=${(%):-%f}
+      local Fr=${(%):-'%F{red}'} Fg=${(%):-'%F{green}'} Fc=${(%):-'%F{cyan}'}
+
+      [[ -n $ZTST_failcolour ]] || B= b= Fr= Fg= Fc= f=
+
+      out[1]=${out[1]/#---*/$B--- expected$b}
+      out[2]=${out[2]/#+++*/$B+++ actual$b}
+
+      (( $#B )) && {
+	out=( ${(@)out/#(#m)@@*/$Fc$MATCH$f} )
+	out=( ${(@)out/#(#m)-*/$Fr$MATCH$f} )
+	out=( ${(@)out/#(#m)+*/$Fg$MATCH$f} )
+      }
+    }
+
+    print -rl - $out
+    return ret
+  }
+
   if (( diff_pat )); then
     local -a diff_lines1 diff_lines2
     integer failed i l
@@ -365,30 +398,19 @@ ZTST_diff() {
       done
     fi
     if (( failed )); then
-      for (( l = 1; l <= ${#diff_lines1}; ++l )); do
-	if (( l == i )); then
-	  p="-"
-	else
-	  p=" "
-	fi
-	print -r -- "$p<${diff_lines1[l]}"
-      done
-      for (( l = 1; l <= ${#diff_lines2}; ++l )); do
-	if (( l == i )); then
-	  p="+"
-	else
-	  p=" "
-	fi
-	print -r -- "$p>${diff_lines2[l]}"
-      done
+      diff_out=$( ZTST_do_diff \
+	<( print -rl - "${(@)diff_lines1}" ) \
+	<( print -rl - "${(@)diff_lines2}" )
+      )
       diff_ret=1
     fi
   else
-    diff_out=$(diff $diff_arg "$@")
+    diff_out=$(ZTST_do_diff "$@")
     diff_ret="$?"
-    if [[ "$diff_ret" != "0" ]]; then
-      print -r -- "$diff_out"
-    fi
+  fi
+
+  if [[ "$diff_ret" != "0" ]]; then
+    print -r -- "$diff_out"
   fi
 
   return "$diff_ret"
@@ -526,7 +548,7 @@ $(<$ZTST_terr)"
 	rm -rf $ZTST_out
 	print -r -- "${(e)substlines}" >$ZTST_out
       fi
-      if [[ $ZTST_flags != *d* ]] && ! $ZTST_diff $diff_out -u $ZTST_out $ZTST_tout; then
+      if [[ $ZTST_flags != *d* ]] && ! $ZTST_diff $diff_out $ZTST_out $ZTST_tout; then
         if (( expected_to_fail )); then
           ZTST_verbose 1 "Test failed, as expected."
           continue
@@ -542,7 +564,7 @@ $(<$ZTST_terr)}"
 	rm -rf $ZTST_err
 	print -r -- "${(e)substlines}" >$ZTST_err
       fi
-      if [[ $ZTST_flags != *D* ]] && ! $ZTST_diff $diff_err -u $ZTST_err $ZTST_terr; then
+      if [[ $ZTST_flags != *D* ]] && ! $ZTST_diff $diff_err $ZTST_err $ZTST_terr; then
         if (( expected_to_fail )); then
           ZTST_verbose 1 "Test failed, as expected."
           continue