[PATCH] tests: improve readability

dana <[email protected]>
Newsgroups gmane.comp.shells.zsh.devel
Message-ID <[email protected]>
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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.