From fad413366ae29271bc65cd7cc985bf1b852cefcc Mon Sep 17 00:00:00 2001
From: Stephen Morris <stephen@isc.org>
Date: Fri, 15 Dec 2017 15:56:33 +0000
Subject: [PATCH] [rt46602] Ensure test output in systests.output is not mixed
 up

When running all the system tests, output from a test is sent to a
test.output file in the test directory.  These are combined in to
systests.output when the run finishes.

(cherry picked from commit 055e5be9fd7555e24449157bc78b8c0852f9b7d1)
(cherry picked from commit 50769a4a9e699b74b2a6fa2d5dc91ca54c99d9f3)
(cherry picked from commit 818ac05cfcd5a22c9f945ca63ee1ace13f6d8a72)
---
 bin/tests/system/.gitignore           |   1 +
 bin/tests/system/Makefile.in          |   6 +-
 bin/tests/system/README               | 119 ++++++++++++++++++--------
 bin/tests/system/allow_query/clean.sh |   1 -
 bin/tests/system/clean.sh             |  20 ++++-
 bin/tests/system/cleanall.sh          |   1 +
 bin/tests/system/run.sh               |  40 ++++-----
 bin/tests/system/runall.sh            |  19 ++--
 bin/tests/system/runsequential.sh     |   8 +-
 bin/tests/system/testsummary.sh       |  31 ++++++-
 10 files changed, 171 insertions(+), 75 deletions(-)

diff --git a/bin/tests/system/.gitignore b/bin/tests/system/.gitignore
index 83336dffe4..5e086e79c9 100644
--- a/bin/tests/system/.gitignore
+++ b/bin/tests/system/.gitignore
@@ -10,5 +10,6 @@ named.pid
 named.run
 named.port
 /feature-test
+**/test.output
 /systests.output
 /random.data
diff --git a/bin/tests/system/Makefile.in b/bin/tests/system/Makefile.in
index 992d0ebe1a..5a05b2d865 100644
--- a/bin/tests/system/Makefile.in
+++ b/bin/tests/system/Makefile.in
@@ -68,15 +68,15 @@ parallel.mk:
 	for directory in $(PARALLEL) ; do \
 		echo "" >> $@ ; \
 		echo "$$directory:" >> $@ ; \
-		echo "	@$(SHELL) ./run.sh -p $$port $$directory" >> $@ ; \
+		echo "	@$(SHELL) ./run.sh -r -p $$port $$directory | tee $$directory/test.output" >> $@ ; \
 		port=`expr $$port + 100` ; \
 	done
 
 # Targets to run the tests.
 
 test: parallel.mk
-	@$(MAKE) -f parallel.mk check 2>&1 | tee systests.output
-	@$(SHELL) ./runsequential.sh 2>&1 | tee -a systests.output
+	@$(MAKE) -f parallel.mk check
+	@$(SHELL) ./runsequential.sh -r
 	@$(SHELL) ./testsummary.sh
 
 check: test
diff --git a/bin/tests/system/README b/bin/tests/system/README
index cdc41fcf40..2f9c638147 100644
--- a/bin/tests/system/README
+++ b/bin/tests/system/README
@@ -105,6 +105,9 @@ Optional flags are:
                     the date looking like "2017-11-23:16:06:32" instead of the
                     default "Thu, 23 Nov 2017 16:06:32 +0000".
 
+    -r              The "runall" flag.  This is related to cleaning up after
+                    all the tests. (See "Mainenance Notes" below.)
+
 Arguments are:
 
     test-name       Mandatory. The name of the test, which is the name of the
@@ -134,9 +137,9 @@ It is also possible to run the test suites from two installations of BIND on
 the same machine at the same time.  To do this:
 
 1. Each installation must have its own directory tree. The system tests create
-temporary configuration files in the test directories, so separate directory
-trees are required to avoid interference between the same test running in the
-different installations.
+files in the test directories, so separate directory trees are required to
+avoid interference between the same test running in the different
+installations.
 
 2. For one of the test suites, the starting port number must be specified by
 setting the environment variable STARTPORT before starting the test suite.
@@ -156,12 +159,12 @@ abouts).  The use of "STARTPORT=20000" to prefix the run of the test suite for
 installation-2 will mean the test suite uses ports 20,000 through 30,000 or so.
 
 Parallel running will reduce the total time taken to run the BIND system tests,
-but will mean that the output from all the tests will be mixed up with one
-another in the systests.output file.  However, if you need to investigate the
-output from a test, there is a simple way of extracting the information.
-Before discussing this though, the format of the test messages needs to be
-understood.
+but will mean that the output from all the tests pointed to the screen will be
+mixed up with one another.  However, the systests.output file produced at the
+end of the run will contain the output from each test, one after the other.
 
+Format of Test Output
+---
 All output from the system tests is in the form of lines with the following
 structure:
 
@@ -217,18 +220,6 @@ the form:
 e.g. nsupdate.out.test28, dig.out.q3.  This aids diagnosis of problems by
 allowing the output that caused the problem to be identified.
 
-Returning to the problem of extracting information about a single test from
-systests.output, the solution is fairly easy: run the command:
-
-    grep ':<test-name>:' systests.output
-
-e.g.
-
-    grep ':catz:' systests.output
-
-(note the colons before and after the test name).  This will list all the
-messages produced by the test in the order they were output.
-
 
 Re-running the Tests
 ===
@@ -255,6 +246,7 @@ Developer Notes
 ===
 This section is intended for developers writing new tests.
 
+
 Overview
 ---
 As noted above, each test suite is in a separate directory.  To interact with
@@ -326,11 +318,30 @@ General
 ---
 1. Each of the four scripts will be invoked with the command
 
-    sh <script> [<arguments>]
+    (cd <test-directory> ; sh <script> [<arguments>] )
 
-Thhe optional arguments are test-specific.  They are
+... so that working directory when the script starts executing is the test
+directory.
 
-Each script should start with the following lines:
+2. Arguments can be only passed to the script if the test is being run as a
+one-off with "run.sh". In this case, everything on the command line after the
+name of the test is passed to each script.  For example, the command:
+
+    sh run.sh -p 12300 mytest -D xyz
+
+... will run "mytest" with a port range of 12300 to 12399.  Each of the
+framework scripts provided by the test will be invoked using the remaining
+arguments, e.g.
+
+   (cd mytest ; sh prereq.sh -D xyz)
+   (cd mytest ; sh setup.sh -D xyz)
+   (cd mytest ; sh tests.sh -D xyz)
+   (cd mytest ; sh clean.sh -D xyz)
+
+No arguments will be passed to the test scripts if the test is run as part of
+a run of the full test suite (e.g. the tests are started with "runall.sh").
+
+3.  Each script should start with the following lines:
 
     SYSTEMTESTTOP=..
     . $SYSTEMTESTTOP/conf.sh
@@ -347,7 +358,7 @@ configured with the appropriate options required.
 
     * If the software required to run the test is present and the BIND
       configure options are correct, prereq.sh should return with a status code
-       of 0.
+      of 0.
 
     * If the software required to run the test is not available and/or BIND
       has not been configured with the appropriate options, prereq.sh should
@@ -356,6 +367,7 @@ configured with the appropriate options required.
     * If there is some other problem (e.g. prerequistie software is available
       but is not properly configured), a status code of 255 should be returned.
 
+
 setup.sh
 ---
 This is responsible for setting up the configuration files used in the test.
@@ -392,6 +404,7 @@ This replaces the tokens @PORT@, @CONTROLPORT@, @EXTRAPORT1@ through
 @EXTRAPORT8@ with the contents of the shell variables listed above.  setup.sh
 should do this for all configuration files required when the test starts.
 
+
 tests.sh
 ---
 This is the main test file and the contents depend on the test. The contents
@@ -449,21 +462,34 @@ clean.sh
 ---
 The inverse of "setup.sh", this is invoked by the framework to clean up the
 test directory.  It should delete all files that have been created by the test
-during its run.
+during its run.A
 
 
 Adding a Test to the System Test Suite
 ---
 Once a set of tests has been created, the following files should be edited:
 
-conf.sh.in  The name of the test should be added to the PARALLELDIRS variable.
+* conf.sh.in  The name of the test should be added to the PARALLELDIRS
+variable.
 
-Makefile.in The name of the test should be added to the PARALLEL variable.
+* Makefile.in The name of the test should be added to the PARALLEL variable.
 
 (It is likely that a future iteration of the system test suite will remove the
 need to edit two files to add a test.)
 
 
+Valgrind
+---
+When running system tests, named can be run under Valgrind. The output from
+Valgrind are sent to per-process files that can be reviewed after the test has
+completed. To enable this, set the USE_VALGRIND environment variable to
+"helgrind" to run the Helgrind tool, or any other value to run the Memcheck
+tool. To use "helgrind" effectively, build BIND with --disable-atomic.
+
+
+Maintenance Notes
+===
+This section is aimed at developers maintaining BIND's system test framework.
 
 Notes on Parallel Execution
 ---
@@ -481,16 +507,41 @@ is created when "make check" is run, and contains a target for each test of
 the form:
 
     <test-name>:
-        @$(SHELL) run.sh -p <baseport> <test-name>
+        @$(SHELL) run.sh -r -p <baseport> <test-name>
 
 The <baseport> is unique and the values of <baseport> for each test are
 separated by at least 100 ports.
 
 
-Valgrind
+Cleaning up from Tests
 ---
-When running system tests, named can be run under Valgrind. The output from
-Valgrind are sent to per-process files that can be reviewed after the test has
-completed. To enable this, set the USE_VALGRIND environment variable to
-"helgrind" to run the Helgrind tool, or any other value to run the Memcheck
-tool. To use "helgrind" effectively, build BIND with --disable-atomic.
+When a test is run, files are produced in the test directory by both the test
+itself and the test framework.
+
+The former comprises the the logs from the running of nameservers, the output
+from "dig" and "rndc" commands used in the test,  and anything else that the
+test developer feels is relevant. In the event of a test failure, they can be
+useful in tracking down the problem.
+
+At the moment, there is only one file in the latter category, the file
+test.output.  This is created when the test is run as part of a run of the
+test suite (e.g. via "runall.sh") and comprises a copy of the output sent to
+stdout.  (If the test is running in parallel with other tests, the output sent
+to stdout will be interleaved with their output.  The output in test.output
+will comprise solely the output from this test.)
+
+If the test is run by itself with a direct invocation of "run.sh", the disposal
+of the output depends on the test.  If it fails, all output is retained.  If it
+succeeds, "run.sh" calls the framework's "clean.sh". This deletes the test's
+test.output file then calls the test's own "clean.sh" to clean up the
+test-specific files.
+
+The clean-up path is different if the test is run as part of the test suite.
+In this case, although the test is ultimately run by a call to "run.sh",
+"run.sh" is invoked with the "-r" switch (the "runall" flag).  Should the test
+fail, all files are retained as before.  If the test passes, the framework's
+"clean.sh" with the "-r" switch.  This causes it to only call the test's
+"clean.sh", leaving the test.output file untouched.  When all tests in the
+testsuite have been run, the test suite code invokes "testsummary.sh".  This
+iterates through all test directory, concatenating the "test.output" file to a
+single "systests.output", deleting "test.output" as it does so.
diff --git a/bin/tests/system/allow_query/clean.sh b/bin/tests/system/allow_query/clean.sh
index 359c6087b8..0947e47591 100644
--- a/bin/tests/system/allow_query/clean.sh
+++ b/bin/tests/system/allow_query/clean.sh
@@ -18,7 +18,6 @@
 # Clean up after allow query tests.
 #
 
-rm -f test.output
 rm -f dig.out.*
 rm -f ns2/named.conf ns2/named.port ns2/controls.conf
 rm -f */named.memstats
diff --git a/bin/tests/system/clean.sh b/bin/tests/system/clean.sh
index f2cbe5fe0f..be3ae61547 100644
--- a/bin/tests/system/clean.sh
+++ b/bin/tests/system/clean.sh
@@ -21,11 +21,29 @@
 SYSTEMTESTTOP=.
 . $SYSTEMTESTTOP/conf.sh
 
-test $# -gt 0 || { echo "usage: $0 test-directory" >&2; exit 1; }
+# See if the "-r" flag is present.  This will usually be set when all the tests
+# are run (e.g. from "runall.sh") and tells the script not to delete the
+# test.output file created by run.sh.  This is because the script running all
+# the tests will call "testsummary.sh", which will concatenate all test output
+# files into a single systests.output.
+
+while getopts "r" flag; do
+    case $flag in
+	r) runall=1 ;;
+	*) exit 1;;
+    esac
+done
+shift `expr $OPTIND - 1`
+
+test $# -gt 0 || { echo "usage: $0 [-r] test-directory" >&2; exit 1; }
 
 test=$1
 shift
 
+if [ "$runall" = "" ]; then
+    rm -f $test/test.output
+fi
+
 if test -f $test/clean.sh; then
     ( cd $test && $SHELL clean.sh "$@" )
 fi
diff --git a/bin/tests/system/cleanall.sh b/bin/tests/system/cleanall.sh
index 6088d1b5bb..a76c23da32 100644
--- a/bin/tests/system/cleanall.sh
+++ b/bin/tests/system/cleanall.sh
@@ -34,5 +34,6 @@ status=0
 for d in $SUBDIRS
 do
    test ! -f $d/clean.sh || ( cd $d && $SHELL clean.sh )
+   test -f $d/test.output && rm $d/test.output
    test -d $d && find $d -type d -exec rmdir '{}' \; 2> /dev/null
 done
diff --git a/bin/tests/system/run.sh b/bin/tests/system/run.sh
index 80af115563..476fdc53ce 100644
--- a/bin/tests/system/run.sh
+++ b/bin/tests/system/run.sh
@@ -27,12 +27,13 @@ clean=true
 baseport=5300
 dateargs="-R"
 
-while getopts "knp:d:" flag; do
+while getopts "rnp:d:" flag; do
     case "$flag" in
 	k) stopservers=false ;;
 	n) clean=false ;;
 	p) baseport=$OPTARG ;;
 	d) dateargs=$OPTARG ;;
+	r) runall="-r" ;;
 	*) exit 1 ;;
     esac
 done
@@ -63,7 +64,7 @@ if [ $? -ne 0 ]; then
     echofail "Must specify a numeric value for the port"
     exit 1
 elif [ $baseport -lt $minvalid -o $baseport -gt $maxvalid  ]; then
-    echofail "Tte specified port must be in the range $minvalid to $maxvalid" >&2
+    echofail "The specified port must be in the range $minvalid to $maxvalid" >&2
     exit 1
 fi
 
@@ -87,16 +88,16 @@ export LOWPORT=$baseport
 export HIGHPORT=`expr $baseport + $numport - 1`
 
 
-echoinfo "S:$test:`date $dateargs`" >&2
-echoinfo "T:$test:1:A" >&2
-echoinfo "A:$test:System test $test" >&2
+echoinfo "S:$test:`date $dateargs`"
+echoinfo "T:$test:1:A"
+echoinfo "A:$test:System test $test"
 echoinfo "I:$test:PORTRANGE:${LOWPORT} - ${HIGHPORT}"
 
 if [ x${PERL:+set} = x ]
 then
-    echowarn "I:$test:Perl not available.  Skipping test." >&2
-    echowarn "R:$test:UNTESTED" >&2
-    echoinfo "E:$test:`date $dateargs`" >&2
+    echowarn "I:$test:Perl not available.  Skipping test."
+    echowarn "R:$test:UNTESTED"
+    echoinfo "E:$test:`date $dateargs`"
     exit 0;
 fi
 
@@ -107,7 +108,7 @@ result=$?
 if [ $result -eq 0 ]; then
     : prereqs ok
 else
-    echowarn "I:$test:Prerequisites missing, skipping test." >&2
+    echowarn "I:$test:Prerequisites missing, skipping test."
     [ $result -eq 255 ] && echowarn "R:$test:SKIPPED" || echowarn "R:$test:UNTESTED"
     echoinfo "E:$test:`date $dateargs`" >&2
     exit 0
@@ -115,9 +116,9 @@ fi
 
 # Test sockets after the prerequisites has been setup
 $PERL testsock.pl -p $PORT  || {
-    echowarn "I:$test:Network interface aliases not set up.  Skipping test." >&2;
-    echowarn "R:$test:UNTESTED" >&2;
-    echoinfo "E:$test:`date $dateargs`" >&2;
+    echowarn "I:$test:Network interface aliases not set up.  Skipping test."
+    echowarn "R:$test:UNTESTED"
+    echoinfo "E:$test:`date $dateargs`"
     exit 0;
 }
 
@@ -127,9 +128,9 @@ if
 then
     : pkcs11 ok
 else
-    echowarn "I:$test:Need PKCS#11, skipping test." >&2
-    echowarn "R:$test:PKCS11ONLY" >&2
-    echoinfo "E:$test:`date $dateargs`" >&2
+    echowarn "I:$test:Need PKCS#11, skipping test."
+    echowarn "R:$test:PKCS11ONLY"
+    echoinfo "E:$test:`date $dateargs`"
     exit 0
 fi
 
@@ -167,17 +168,14 @@ else
     if $clean
     then
         rm -f $SYSTEMTESTTOP/random.data
-        if test -f $test/clean.sh
-        then
-		( cd $test && $SHELL clean.sh "$@" )
-        fi
+	$SHELL clean.sh $runall $test "$@"
         if test -d ../../../.git
         then
-            git status -su --ignored $test |
+            git status -su --ignored $test | \
             sed -n -e 's|^?? \(.*\)|I:file \1 not removed|p' \
             -e 's|^!! \(.*/named.run\)$|I:file \1 not removed|p' \
             -e 's|^!! \(.*/named.memstats\)$|I:file \1 not removed|p'
-		fi
+	fi
     fi
 fi
 
diff --git a/bin/tests/system/runall.sh b/bin/tests/system/runall.sh
index 5668493b70..3909934515 100644
--- a/bin/tests/system/runall.sh
+++ b/bin/tests/system/runall.sh
@@ -26,16 +26,21 @@
 SYSTEMTESTTOP=.
 . $SYSTEMTESTTOP/conf.sh
 
-numproc=
+usage="Usage: ./runall.sh [numprocesses]"
+
 if [ $# -eq 0 ]; then
     numproc=1
-
-elif [ $# -gt 1 ] ||  "$(($1 + 0))" -ne "$1" ]; then
-    echo "Usage: ./runall.sh [numprocesses]"
-
-else
+elif [ $# -eq 1 ]; then
+    test "$1" -eq "$1" > /dev/null 2>& 1
+    if [ $? -ne 0 ]; then
+        # Value passed is not numeric
+        echo "$usage"
+        exit 1
+    fi
     numproc=$1
-
+else
+    echo "$usage"
+    exit 1
 fi
 
 make -j $numproc check
diff --git a/bin/tests/system/runsequential.sh b/bin/tests/system/runsequential.sh
index caa4ed7b93..602fb3048a 100644
--- a/bin/tests/system/runsequential.sh
+++ b/bin/tests/system/runsequential.sh
@@ -16,7 +16,7 @@
 SYSTEMTESTTOP=.
 . $SYSTEMTESTTOP/conf.sh
 
-    for d in $SEQUENTIALDIRS
-    do
-        $SHELL run.sh "${@}" $d
-    done
+for d in $SEQUENTIALDIRS
+do
+    $SHELL run.sh "${@}" $d | tee $d/test.output
+done
diff --git a/bin/tests/system/testsummary.sh b/bin/tests/system/testsummary.sh
index 11cc79f12c..785fb732b3 100644
--- a/bin/tests/system/testsummary.sh
+++ b/bin/tests/system/testsummary.sh
@@ -6,12 +6,16 @@
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
-# Searches the system test output file (systests.output) and prints a summary
-# of tests passed, failed, not run.  It also checks whether the IP addresses
-# 10.53.0.[1-8] were set up and, if not, prints a warning.
+# Creates the system tests output file from the various test.output files. It
+# then searches that file and prints the number of tests passed, failed, not
+# run.  It also checks whether the IP addresses 10.53.0.[1-8] were set up and,
+# if not, prints a warning.
 #
 # Usage:
-#    testsummary.sh
+#    testsummary.sh [-n]
+#
+# -n	Do NOT delete the individual test.output files after concatenating
+#	them into systests.output.
 #
 # Status return:
 # 0 - no tests failed
@@ -20,6 +24,25 @@
 SYSTEMTESTTOP=.
 . $SYSTEMTESTTOP/conf.sh
 
+while getopts "n" flag; do
+    case $flag in
+	n) keepfile=1 ;;
+	*) exit 1 ;;
+    esac
+done
+
+rm -f systests.output
+touch systests.output
+for directory in $SUBDIRS ; do
+    if [ -e $directory/test.output ]; then
+        cat $directory/test.output >> systests.output
+	if [ "$keepfile" = "" ]; then
+	    rm $directory/test.output
+	fi
+    fi
+done
+
+
 $PERL testsock.pl || {
     cat <<EOF >&2
 I: