From 64df488e1ef7fc25eb6c46b5c38cfabc8ad0e0f8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= <ondrej@isc.org>
Date: Fri, 6 Dec 2019 11:03:54 +0100
Subject: [PATCH 1/4] Add the standard $n to each test

---
 bin/tests/system/forward/tests.sh | 49 +++++++++++++++++++++----------
 1 file changed, 33 insertions(+), 16 deletions(-)

diff --git a/bin/tests/system/forward/tests.sh b/bin/tests/system/forward/tests.sh
index 1c3096cb79..17f8631ead 100644
--- a/bin/tests/system/forward/tests.sh
+++ b/bin/tests/system/forward/tests.sh
@@ -19,8 +19,10 @@ f1=10.53.0.3
 f2=10.53.0.4
 
 status=0
+n=0
 
-echo_i "checking that a forward zone overrides global forwarders"
+n=$((n+1))
+echo_i "checking that a forward zone overrides global forwarders ($n)"
 ret=0
 $DIG $DIGOPTS +noadd +noauth txt.example1. txt @$hidden > dig.out.hidden || ret=1
 $DIG $DIGOPTS +noadd +noauth txt.example1. txt @$f1 > dig.out.f1 || ret=1
@@ -28,7 +30,8 @@ digcomp dig.out.hidden dig.out.f1 || ret=1
 if [ $ret != 0 ]; then echo_i "failed"; fi
 status=`expr $status + $ret`
 
-echo_i "checking that a forward first zone no forwarders recurses"
+n=$((n+1))
+echo_i "checking that a forward first zone no forwarders recurses ($n)"
 ret=0
 $DIG $DIGOPTS +noadd +noauth txt.example2. txt @$root > dig.out.root || ret=1
 $DIG $DIGOPTS +noadd +noauth txt.example2. txt @$f1 > dig.out.f1 || ret=1
@@ -36,7 +39,8 @@ digcomp dig.out.root dig.out.f1 || ret=1
 if [ $ret != 0 ]; then echo_i "failed"; fi
 status=`expr $status + $ret`
 
-echo_i "checking that a forward only zone no forwarders fails"
+n=$((n+1))
+echo_i "checking that a forward only zone no forwarders fails ($n)"
 ret=0
 $DIG $DIGOPTS +noadd +noauth txt.example2. txt @$root > dig.out.root || ret=1
 $DIG $DIGOPTS +noadd +noauth txt.example2. txt @$f1 > dig.out.f1 || ret=1
@@ -44,7 +48,8 @@ digcomp dig.out.root dig.out.f1 || ret=1
 if [ $ret != 0 ]; then echo_i "failed"; fi
 status=`expr $status + $ret`
 
-echo_i "checking that global forwarders work"
+n=$((n+1))
+echo_i "checking that global forwarders work ($n)"
 ret=0
 $DIG $DIGOPTS +noadd +noauth txt.example4. txt @$hidden > dig.out.hidden || ret=1
 $DIG $DIGOPTS +noadd +noauth txt.example4. txt @$f1 > dig.out.f1 || ret=1
@@ -52,7 +57,8 @@ digcomp dig.out.hidden dig.out.f1 || ret=1
 if [ $ret != 0 ]; then echo_i "failed"; fi
 status=`expr $status + $ret`
 
-echo_i "checking that a forward zone works"
+n=$((n+1))
+echo_i "checking that a forward zone works ($n)"
 ret=0
 $DIG $DIGOPTS +noadd +noauth txt.example1. txt @$hidden > dig.out.hidden || ret=1
 $DIG $DIGOPTS +noadd +noauth txt.example1. txt @$f2 > dig.out.f2 || ret=1
@@ -60,7 +66,8 @@ digcomp dig.out.hidden dig.out.f2 || ret=1
 if [ $ret != 0 ]; then echo_i "failed"; fi
 status=`expr $status + $ret`
 
-echo_i "checking that forwarding doesn't spontaneously happen"
+n=$((n+1))
+echo_i "checking that forwarding doesn't spontaneously happen ($n)"
 ret=0
 $DIG $DIGOPTS +noadd +noauth txt.example2. txt @$root > dig.out.root || ret=1
 $DIG $DIGOPTS +noadd +noauth txt.example2. txt @$f2 > dig.out.f2 || ret=1
@@ -68,7 +75,8 @@ digcomp dig.out.root dig.out.f2 || ret=1
 if [ $ret != 0 ]; then echo_i "failed"; fi
 status=`expr $status + $ret`
 
-echo_i "checking that a forward zone with no specified policy works"
+n=$((n+1))
+echo_i "checking that a forward zone with no specified policy works ($n)"
 ret=0
 $DIG $DIGOPTS +noadd +noauth txt.example3. txt @$hidden > dig.out.hidden || ret=1
 $DIG $DIGOPTS +noadd +noauth txt.example3. txt @$f2 > dig.out.f2 || ret=1
@@ -76,14 +84,16 @@ digcomp dig.out.hidden dig.out.f2 || ret=1
 if [ $ret != 0 ]; then echo_i "failed"; fi
 status=`expr $status + $ret`
 
-echo_i "checking that a forward only doesn't recurse"
+n=$((n+1))
+echo_i "checking that a forward only doesn't recurse ($n)"
 ret=0
 $DIG $DIGOPTS txt.example5. txt @$f2 > dig.out.f2 || ret=1
 grep "SERVFAIL" dig.out.f2 > /dev/null || ret=1
 if [ $ret != 0 ]; then echo_i "failed"; fi
 status=`expr $status + $ret`
 
-echo_i "checking for negative caching of forwarder response"
+n=$((n+1))
+echo_i "checking for negative caching of forwarder response ($n)"
 # prime the cache, shutdown the forwarder then check that we can
 # get the answer from the cache.  restart forwarder.
 ret=0
@@ -96,7 +106,8 @@ $PERL ../start.pl --restart --noclean --port ${PORT} forward ns4 || ret=1
 if [ $ret != 0 ]; then echo_i "failed"; fi
 status=`expr $status + $ret`
 
-echo_i "checking that forward only zone overrides empty zone"
+n=$((n+1))
+echo_i "checking that forward only zone overrides empty zone ($n)"
 ret=0
 # retry loop in case the server restart above causes transient failure
 for try in 0 1 2 3 4 5 6 7 8 9; do
@@ -110,7 +121,8 @@ done
 if [ $ret != 0 ]; then echo_i "failed"; fi
 status=`expr $status + $ret`
 
-echo_i "checking that DS lookups for grafting forward zones are isolated"
+n=$((n+1))
+echo_i "checking that DS lookups for grafting forward zones are isolated ($n)"
 ret=0
 $DIG $DIGOPTS grafted A @10.53.0.4 > dig.out.q1
 $DIG $DIGOPTS grafted DS @10.53.0.4 > dig.out.q2
@@ -123,21 +135,24 @@ grep "status: NOERROR" dig.out.q4 > /dev/null || ret=1
 if [ $ret != 0 ]; then echo_i "failed"; fi
 status=`expr $status + $ret`
 
-echo_i "checking that rfc1918 inherited 'forward first;' zones are warned about"
+n=$((n+1))
+echo_i "checking that rfc1918 inherited 'forward first;' zones are warned about ($n)"
 ret=0
 $CHECKCONF rfc1918-inherited.conf | grep "forward first;" >/dev/null || ret=1
 $CHECKCONF rfc1918-notinherited.conf | grep "forward first;" >/dev/null && ret=1
 if [ $ret != 0 ]; then echo_i "failed"; fi
 status=`expr $status + $ret`
 
-echo_i "checking that ULA inherited 'forward first;' zones are warned about"
+n=$((n+1))
+echo_i "checking that ULA inherited 'forward first;' zones are warned about ($n)"
 ret=0
 $CHECKCONF ula-inherited.conf | grep "forward first;" >/dev/null || ret=1
 $CHECKCONF ula-notinherited.conf | grep "forward first;" >/dev/null && ret=1
 if [ $ret != 0 ]; then echo_i "failed"; fi
 status=`expr $status + $ret`
 
-echo_i "checking that a forwarder timeout prevents it from being reused in the same fetch context"
+n=$((n+1))
+echo_i "checking that a forwarder timeout prevents it from being reused in the same fetch context ($n)"
 ret=0
 # Make ans6 receive queries without responding to them.
 echo "//" | $SENDCMD
@@ -152,7 +167,8 @@ if [ $sent -ne 1 ]; then ret=1; fi
 if [ $ret != 0 ]; then echo_i "failed"; fi
 status=`expr $status + $ret`
 
-echo_i "checking that priming queries are not forwarded"
+n=$((n+1))
+echo_i "checking that priming queries are not forwarded ($n)"
 ret=0
 $DIG $DIGOPTS +noadd +noauth txt.example1. txt @10.53.0.7 > dig.out.f7 || ret=1
 sent=`tr -d '\r' < ns7/named.run | sed -n '/sending packet to 10.53.0.1/,/^$/p' | grep ";.*IN.*NS" | wc -l`
@@ -164,7 +180,8 @@ sent=`grep "10.53.0.7#.* (.): query '\./NS/IN' approved" ns1/named.run | wc -l`
 if [ $ret != 0 ]; then echo_i "failed"; fi
 status=`expr $status + $ret`
 
-echo_i "checking recovery from forwarding to a non-recursive server"
+n=$((n+1))
+echo_i "checking recovery from forwarding to a non-recursive server ($n)"
 ret=0
 $DIG $DIGOPTS xxx.sld.tld txt @10.53.0.8  > dig.out.f8
 grep "status: NOERROR" dig.out.f8 > /dev/null || ret=1

From 10f4cd066fbc3b84ae917f21ef608ed5759159a8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= <ondrej@isc.org>
Date: Fri, 6 Dec 2019 11:07:28 +0100
Subject: [PATCH 2/4] Use $n to keep diagnostic output of every individual test
 separate

---
 bin/tests/system/forward/tests.sh | 86 +++++++++++++++----------------
 1 file changed, 43 insertions(+), 43 deletions(-)

diff --git a/bin/tests/system/forward/tests.sh b/bin/tests/system/forward/tests.sh
index 17f8631ead..2cd09e0628 100644
--- a/bin/tests/system/forward/tests.sh
+++ b/bin/tests/system/forward/tests.sh
@@ -24,71 +24,71 @@ n=0
 n=$((n+1))
 echo_i "checking that a forward zone overrides global forwarders ($n)"
 ret=0
-$DIG $DIGOPTS +noadd +noauth txt.example1. txt @$hidden > dig.out.hidden || ret=1
-$DIG $DIGOPTS +noadd +noauth txt.example1. txt @$f1 > dig.out.f1 || ret=1
-digcomp dig.out.hidden dig.out.f1 || ret=1
+$DIG $DIGOPTS +noadd +noauth txt.example1. txt @$hidden > dig.out.$n.hidden || ret=1
+$DIG $DIGOPTS +noadd +noauth txt.example1. txt @$f1 > dig.out.$n.f1 || ret=1
+digcomp dig.out.$n.hidden dig.out.$n.f1 || ret=1
 if [ $ret != 0 ]; then echo_i "failed"; fi
 status=`expr $status + $ret`
 
 n=$((n+1))
 echo_i "checking that a forward first zone no forwarders recurses ($n)"
 ret=0
-$DIG $DIGOPTS +noadd +noauth txt.example2. txt @$root > dig.out.root || ret=1
-$DIG $DIGOPTS +noadd +noauth txt.example2. txt @$f1 > dig.out.f1 || ret=1
-digcomp dig.out.root dig.out.f1 || ret=1
+$DIG $DIGOPTS +noadd +noauth txt.example2. txt @$root > dig.out.$n.root || ret=1
+$DIG $DIGOPTS +noadd +noauth txt.example2. txt @$f1 > dig.out.$n.f1 || ret=1
+digcomp dig.out.$n.root dig.out.$n.f1 || ret=1
 if [ $ret != 0 ]; then echo_i "failed"; fi
 status=`expr $status + $ret`
 
 n=$((n+1))
 echo_i "checking that a forward only zone no forwarders fails ($n)"
 ret=0
-$DIG $DIGOPTS +noadd +noauth txt.example2. txt @$root > dig.out.root || ret=1
-$DIG $DIGOPTS +noadd +noauth txt.example2. txt @$f1 > dig.out.f1 || ret=1
-digcomp dig.out.root dig.out.f1 || ret=1
+$DIG $DIGOPTS +noadd +noauth txt.example2. txt @$root > dig.out.$n.root || ret=1
+$DIG $DIGOPTS +noadd +noauth txt.example2. txt @$f1 > dig.out.$n.f1 || ret=1
+digcomp dig.out.$n.root dig.out.$n.f1 || ret=1
 if [ $ret != 0 ]; then echo_i "failed"; fi
 status=`expr $status + $ret`
 
 n=$((n+1))
 echo_i "checking that global forwarders work ($n)"
 ret=0
-$DIG $DIGOPTS +noadd +noauth txt.example4. txt @$hidden > dig.out.hidden || ret=1
-$DIG $DIGOPTS +noadd +noauth txt.example4. txt @$f1 > dig.out.f1 || ret=1
-digcomp dig.out.hidden dig.out.f1 || ret=1
+$DIG $DIGOPTS +noadd +noauth txt.example4. txt @$hidden > dig.out.$n.hidden || ret=1
+$DIG $DIGOPTS +noadd +noauth txt.example4. txt @$f1 > dig.out.$n.f1 || ret=1
+digcomp dig.out.$n.hidden dig.out.$n.f1 || ret=1
 if [ $ret != 0 ]; then echo_i "failed"; fi
 status=`expr $status + $ret`
 
 n=$((n+1))
 echo_i "checking that a forward zone works ($n)"
 ret=0
-$DIG $DIGOPTS +noadd +noauth txt.example1. txt @$hidden > dig.out.hidden || ret=1
-$DIG $DIGOPTS +noadd +noauth txt.example1. txt @$f2 > dig.out.f2 || ret=1
-digcomp dig.out.hidden dig.out.f2 || ret=1
+$DIG $DIGOPTS +noadd +noauth txt.example1. txt @$hidden > dig.out.$n.hidden || ret=1
+$DIG $DIGOPTS +noadd +noauth txt.example1. txt @$f2 > dig.out.$n.f2 || ret=1
+digcomp dig.out.$n.hidden dig.out.$n.f2 || ret=1
 if [ $ret != 0 ]; then echo_i "failed"; fi
 status=`expr $status + $ret`
 
 n=$((n+1))
 echo_i "checking that forwarding doesn't spontaneously happen ($n)"
 ret=0
-$DIG $DIGOPTS +noadd +noauth txt.example2. txt @$root > dig.out.root || ret=1
-$DIG $DIGOPTS +noadd +noauth txt.example2. txt @$f2 > dig.out.f2 || ret=1
-digcomp dig.out.root dig.out.f2 || ret=1
+$DIG $DIGOPTS +noadd +noauth txt.example2. txt @$root > dig.out.$n.root || ret=1
+$DIG $DIGOPTS +noadd +noauth txt.example2. txt @$f2 > dig.out.$n.f2 || ret=1
+digcomp dig.out.$n.root dig.out.$n.f2 || ret=1
 if [ $ret != 0 ]; then echo_i "failed"; fi
 status=`expr $status + $ret`
 
 n=$((n+1))
 echo_i "checking that a forward zone with no specified policy works ($n)"
 ret=0
-$DIG $DIGOPTS +noadd +noauth txt.example3. txt @$hidden > dig.out.hidden || ret=1
-$DIG $DIGOPTS +noadd +noauth txt.example3. txt @$f2 > dig.out.f2 || ret=1
-digcomp dig.out.hidden dig.out.f2 || ret=1
+$DIG $DIGOPTS +noadd +noauth txt.example3. txt @$hidden > dig.out.$n.hidden || ret=1
+$DIG $DIGOPTS +noadd +noauth txt.example3. txt @$f2 > dig.out.$n.f2 || ret=1
+digcomp dig.out.$n.hidden dig.out.$n.f2 || ret=1
 if [ $ret != 0 ]; then echo_i "failed"; fi
 status=`expr $status + $ret`
 
 n=$((n+1))
 echo_i "checking that a forward only doesn't recurse ($n)"
 ret=0
-$DIG $DIGOPTS txt.example5. txt @$f2 > dig.out.f2 || ret=1
-grep "SERVFAIL" dig.out.f2 > /dev/null || ret=1
+$DIG $DIGOPTS txt.example5. txt @$f2 > dig.out.$n.f2 || ret=1
+grep "SERVFAIL" dig.out.$n.f2 > /dev/null || ret=1
 if [ $ret != 0 ]; then echo_i "failed"; fi
 status=`expr $status + $ret`
 
@@ -97,11 +97,11 @@ echo_i "checking for negative caching of forwarder response ($n)"
 # prime the cache, shutdown the forwarder then check that we can
 # get the answer from the cache.  restart forwarder.
 ret=0
-$DIG $DIGOPTS nonexist. txt @10.53.0.5 > dig.out.f2 || ret=1
-grep "status: NXDOMAIN" dig.out.f2 > /dev/null || ret=1
+$DIG $DIGOPTS nonexist. txt @10.53.0.5 > dig.out.$n.f2 || ret=1
+grep "status: NXDOMAIN" dig.out.$n.f2 > /dev/null || ret=1
 $PERL ../stop.pl forward ns4 || ret=1
-$DIG $DIGOPTS nonexist. txt @10.53.0.5 > dig.out.f2 || ret=1
-grep "status: NXDOMAIN" dig.out.f2 > /dev/null || ret=1
+$DIG $DIGOPTS nonexist. txt @10.53.0.5 > dig.out.$n.f2 || ret=1
+grep "status: NXDOMAIN" dig.out.$n.f2 > /dev/null || ret=1
 $PERL ../start.pl --restart --noclean --port ${PORT} forward ns4 || ret=1
 if [ $ret != 0 ]; then echo_i "failed"; fi
 status=`expr $status + $ret`
@@ -111,10 +111,10 @@ echo_i "checking that forward only zone overrides empty zone ($n)"
 ret=0
 # retry loop in case the server restart above causes transient failure
 for try in 0 1 2 3 4 5 6 7 8 9; do
-    $DIG $DIGOPTS 1.0.10.in-addr.arpa TXT @10.53.0.4 > dig.out.f2
-    grep "status: NOERROR" dig.out.f2 > /dev/null || ret=1
-    $DIG $DIGOPTS 2.0.10.in-addr.arpa TXT @10.53.0.4 > dig.out.f2
-    grep "status: NXDOMAIN" dig.out.f2 > /dev/null || ret=1
+    $DIG $DIGOPTS 1.0.10.in-addr.arpa TXT @10.53.0.4 > dig.out.$n.f2
+    grep "status: NOERROR" dig.out.$n.f2 > /dev/null || ret=1
+    $DIG $DIGOPTS 2.0.10.in-addr.arpa TXT @10.53.0.4 > dig.out.$n.f2
+    grep "status: NXDOMAIN" dig.out.$n.f2 > /dev/null || ret=1
     [ "$ret" -eq 0 ] && break
     sleep 1
 done
@@ -124,14 +124,14 @@ status=`expr $status + $ret`
 n=$((n+1))
 echo_i "checking that DS lookups for grafting forward zones are isolated ($n)"
 ret=0
-$DIG $DIGOPTS grafted A @10.53.0.4 > dig.out.q1
-$DIG $DIGOPTS grafted DS @10.53.0.4 > dig.out.q2
-$DIG $DIGOPTS grafted A @10.53.0.4 > dig.out.q3
-$DIG $DIGOPTS grafted AAAA @10.53.0.4 > dig.out.q4
-grep "status: NOERROR" dig.out.q1 > /dev/null || ret=1
-grep "status: NXDOMAIN" dig.out.q2 > /dev/null || ret=1
-grep "status: NOERROR" dig.out.q3 > /dev/null || ret=1
-grep "status: NOERROR" dig.out.q4 > /dev/null || ret=1
+$DIG $DIGOPTS grafted A @10.53.0.4 > dig.out.$n.q1
+$DIG $DIGOPTS grafted DS @10.53.0.4 > dig.out.$n.q2
+$DIG $DIGOPTS grafted A @10.53.0.4 > dig.out.$n.q3
+$DIG $DIGOPTS grafted AAAA @10.53.0.4 > dig.out.$n.q4
+grep "status: NOERROR" dig.out.$n.q1 > /dev/null || ret=1
+grep "status: NXDOMAIN" dig.out.$n.q2 > /dev/null || ret=1
+grep "status: NOERROR" dig.out.$n.q3 > /dev/null || ret=1
+grep "status: NOERROR" dig.out.$n.q4 > /dev/null || ret=1
 if [ $ret != 0 ]; then echo_i "failed"; fi
 status=`expr $status + $ret`
 
@@ -160,7 +160,7 @@ echo "//" | $SENDCMD
 # and is delegated from the root to check whether the forwarder will be retried
 # when a delegation is encountered after falling back to full recursive
 # resolution.
-$DIG $DIGOPTS txt.example7. txt @$f1 > dig.out.f1 || ret=1
+$DIG $DIGOPTS txt.example7. txt @$f1 > dig.out.$n.f1 || ret=1
 # The forwarder for the "example7" zone should only be queried once.
 sent=`tr -d '\r' < ns3/named.run | sed -n '/sending packet to 10.53.0.6/,/^$/p' | grep ";txt.example7.*IN.*TXT" | wc -l`
 if [ $sent -ne 1 ]; then ret=1; fi
@@ -170,7 +170,7 @@ status=`expr $status + $ret`
 n=$((n+1))
 echo_i "checking that priming queries are not forwarded ($n)"
 ret=0
-$DIG $DIGOPTS +noadd +noauth txt.example1. txt @10.53.0.7 > dig.out.f7 || ret=1
+$DIG $DIGOPTS +noadd +noauth txt.example1. txt @10.53.0.7 > dig.out.$n.f7 || ret=1
 sent=`tr -d '\r' < ns7/named.run | sed -n '/sending packet to 10.53.0.1/,/^$/p' | grep ";.*IN.*NS" | wc -l`
 [ $sent -eq 1 ] || ret=1
 sent=`grep "10.53.0.7#.* (.): query '\./NS/IN' approved" ns4/named.run | wc -l`
@@ -183,8 +183,8 @@ status=`expr $status + $ret`
 n=$((n+1))
 echo_i "checking recovery from forwarding to a non-recursive server ($n)"
 ret=0
-$DIG $DIGOPTS xxx.sld.tld txt @10.53.0.8  > dig.out.f8
-grep "status: NOERROR" dig.out.f8 > /dev/null || ret=1
+$DIG $DIGOPTS xxx.sld.tld txt @10.53.0.8  > dig.out.$n.f8
+grep "status: NOERROR" dig.out.$n.f8 > /dev/null || ret=1
 if [ $ret != 0 ]; then echo_i "failed"; fi
 status=`expr $status + $ret`
 

From 0e15cbb09254ed9a8b6d9092f70b41503f91e9d5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= <ondrej@isc.org>
Date: Fri, 6 Dec 2019 11:17:33 +0100
Subject: [PATCH 3/4] Make forward system test shellcheck clean

---
 bin/tests/system/forward/tests.sh | 128 ++++++++++++++++--------------
 1 file changed, 67 insertions(+), 61 deletions(-)

diff --git a/bin/tests/system/forward/tests.sh b/bin/tests/system/forward/tests.sh
index 2cd09e0628..bb27d31094 100644
--- a/bin/tests/system/forward/tests.sh
+++ b/bin/tests/system/forward/tests.sh
@@ -7,11 +7,17 @@
 # See the COPYRIGHT file distributed with this work for additional
 # information regarding copyright ownership.
 
+#shellcheck source=conf.sh
 SYSTEMTESTTOP=..
-. $SYSTEMTESTTOP/conf.sh
+. "$SYSTEMTESTTOP/conf.sh"
 
-DIGOPTS="-p ${PORT}"
-SENDCMD="$PERL ../send.pl 10.53.0.6 $EXTRAPORT1"
+dig_with_opts() (
+	"$DIG" -p "$PORT" "$@"
+)
+
+sendcmd() (
+	"$PERL" ../send.pl 10.53.0.6 "$EXTRAPORT1"
+)
 
 root=10.53.0.1
 hidden=10.53.0.2
@@ -24,116 +30,116 @@ n=0
 n=$((n+1))
 echo_i "checking that a forward zone overrides global forwarders ($n)"
 ret=0
-$DIG $DIGOPTS +noadd +noauth txt.example1. txt @$hidden > dig.out.$n.hidden || ret=1
-$DIG $DIGOPTS +noadd +noauth txt.example1. txt @$f1 > dig.out.$n.f1 || ret=1
+dig_with_opts +noadd +noauth txt.example1. txt @$hidden > dig.out.$n.hidden || ret=1
+dig_with_opts +noadd +noauth txt.example1. txt @$f1 > dig.out.$n.f1 || ret=1
 digcomp dig.out.$n.hidden dig.out.$n.f1 || ret=1
 if [ $ret != 0 ]; then echo_i "failed"; fi
-status=`expr $status + $ret`
+status=$((status+ret))
 
 n=$((n+1))
 echo_i "checking that a forward first zone no forwarders recurses ($n)"
 ret=0
-$DIG $DIGOPTS +noadd +noauth txt.example2. txt @$root > dig.out.$n.root || ret=1
-$DIG $DIGOPTS +noadd +noauth txt.example2. txt @$f1 > dig.out.$n.f1 || ret=1
+dig_with_opts +noadd +noauth txt.example2. txt @$root > dig.out.$n.root || ret=1
+dig_with_opts +noadd +noauth txt.example2. txt @$f1 > dig.out.$n.f1 || ret=1
 digcomp dig.out.$n.root dig.out.$n.f1 || ret=1
 if [ $ret != 0 ]; then echo_i "failed"; fi
-status=`expr $status + $ret`
+status=$((status+ret))
 
 n=$((n+1))
 echo_i "checking that a forward only zone no forwarders fails ($n)"
 ret=0
-$DIG $DIGOPTS +noadd +noauth txt.example2. txt @$root > dig.out.$n.root || ret=1
-$DIG $DIGOPTS +noadd +noauth txt.example2. txt @$f1 > dig.out.$n.f1 || ret=1
+dig_with_opts +noadd +noauth txt.example2. txt @$root > dig.out.$n.root || ret=1
+dig_with_opts +noadd +noauth txt.example2. txt @$f1 > dig.out.$n.f1 || ret=1
 digcomp dig.out.$n.root dig.out.$n.f1 || ret=1
 if [ $ret != 0 ]; then echo_i "failed"; fi
-status=`expr $status + $ret`
+status=$((status+ret))
 
 n=$((n+1))
 echo_i "checking that global forwarders work ($n)"
 ret=0
-$DIG $DIGOPTS +noadd +noauth txt.example4. txt @$hidden > dig.out.$n.hidden || ret=1
-$DIG $DIGOPTS +noadd +noauth txt.example4. txt @$f1 > dig.out.$n.f1 || ret=1
+dig_with_opts +noadd +noauth txt.example4. txt @$hidden > dig.out.$n.hidden || ret=1
+dig_with_opts +noadd +noauth txt.example4. txt @$f1 > dig.out.$n.f1 || ret=1
 digcomp dig.out.$n.hidden dig.out.$n.f1 || ret=1
 if [ $ret != 0 ]; then echo_i "failed"; fi
-status=`expr $status + $ret`
+status=$((status+ret))
 
 n=$((n+1))
 echo_i "checking that a forward zone works ($n)"
 ret=0
-$DIG $DIGOPTS +noadd +noauth txt.example1. txt @$hidden > dig.out.$n.hidden || ret=1
-$DIG $DIGOPTS +noadd +noauth txt.example1. txt @$f2 > dig.out.$n.f2 || ret=1
+dig_with_opts +noadd +noauth txt.example1. txt @$hidden > dig.out.$n.hidden || ret=1
+dig_with_opts +noadd +noauth txt.example1. txt @$f2 > dig.out.$n.f2 || ret=1
 digcomp dig.out.$n.hidden dig.out.$n.f2 || ret=1
 if [ $ret != 0 ]; then echo_i "failed"; fi
-status=`expr $status + $ret`
+status=$((status+ret))
 
 n=$((n+1))
 echo_i "checking that forwarding doesn't spontaneously happen ($n)"
 ret=0
-$DIG $DIGOPTS +noadd +noauth txt.example2. txt @$root > dig.out.$n.root || ret=1
-$DIG $DIGOPTS +noadd +noauth txt.example2. txt @$f2 > dig.out.$n.f2 || ret=1
+dig_with_opts +noadd +noauth txt.example2. txt @$root > dig.out.$n.root || ret=1
+dig_with_opts +noadd +noauth txt.example2. txt @$f2 > dig.out.$n.f2 || ret=1
 digcomp dig.out.$n.root dig.out.$n.f2 || ret=1
 if [ $ret != 0 ]; then echo_i "failed"; fi
-status=`expr $status + $ret`
+status=$((status+ret))
 
 n=$((n+1))
 echo_i "checking that a forward zone with no specified policy works ($n)"
 ret=0
-$DIG $DIGOPTS +noadd +noauth txt.example3. txt @$hidden > dig.out.$n.hidden || ret=1
-$DIG $DIGOPTS +noadd +noauth txt.example3. txt @$f2 > dig.out.$n.f2 || ret=1
+dig_with_opts +noadd +noauth txt.example3. txt @$hidden > dig.out.$n.hidden || ret=1
+dig_with_opts +noadd +noauth txt.example3. txt @$f2 > dig.out.$n.f2 || ret=1
 digcomp dig.out.$n.hidden dig.out.$n.f2 || ret=1
 if [ $ret != 0 ]; then echo_i "failed"; fi
-status=`expr $status + $ret`
+status=$((status+ret))
 
 n=$((n+1))
 echo_i "checking that a forward only doesn't recurse ($n)"
 ret=0
-$DIG $DIGOPTS txt.example5. txt @$f2 > dig.out.$n.f2 || ret=1
+dig_with_opts txt.example5. txt @$f2 > dig.out.$n.f2 || ret=1
 grep "SERVFAIL" dig.out.$n.f2 > /dev/null || ret=1
 if [ $ret != 0 ]; then echo_i "failed"; fi
-status=`expr $status + $ret`
+status=$((status+ret))
 
 n=$((n+1))
 echo_i "checking for negative caching of forwarder response ($n)"
 # prime the cache, shutdown the forwarder then check that we can
 # get the answer from the cache.  restart forwarder.
 ret=0
-$DIG $DIGOPTS nonexist. txt @10.53.0.5 > dig.out.$n.f2 || ret=1
+dig_with_opts nonexist. txt @10.53.0.5 > dig.out.$n.f2 || ret=1
 grep "status: NXDOMAIN" dig.out.$n.f2 > /dev/null || ret=1
 $PERL ../stop.pl forward ns4 || ret=1
-$DIG $DIGOPTS nonexist. txt @10.53.0.5 > dig.out.$n.f2 || ret=1
+dig_with_opts nonexist. txt @10.53.0.5 > dig.out.$n.f2 || ret=1
 grep "status: NXDOMAIN" dig.out.$n.f2 > /dev/null || ret=1
-$PERL ../start.pl --restart --noclean --port ${PORT} forward ns4 || ret=1
+$PERL ../start.pl --restart --noclean --port "${PORT}" forward ns4 || ret=1
 if [ $ret != 0 ]; then echo_i "failed"; fi
-status=`expr $status + $ret`
+status=$((status+ret))
+
+check_override() (
+    dig_with_opts 1.0.10.in-addr.arpa TXT @10.53.0.4 > dig.out.$n.f2 &&
+    grep "status: NOERROR" dig.out.$n.f2 > /dev/null &&
+    dig_with_opts 2.0.10.in-addr.arpa TXT @10.53.0.4 > dig.out.$n.f2 &&
+    grep "status: NXDOMAIN" dig.out.$n.f2 > /dev/null
+)
 
 n=$((n+1))
 echo_i "checking that forward only zone overrides empty zone ($n)"
 ret=0
 # retry loop in case the server restart above causes transient failure
-for try in 0 1 2 3 4 5 6 7 8 9; do
-    $DIG $DIGOPTS 1.0.10.in-addr.arpa TXT @10.53.0.4 > dig.out.$n.f2
-    grep "status: NOERROR" dig.out.$n.f2 > /dev/null || ret=1
-    $DIG $DIGOPTS 2.0.10.in-addr.arpa TXT @10.53.0.4 > dig.out.$n.f2
-    grep "status: NXDOMAIN" dig.out.$n.f2 > /dev/null || ret=1
-    [ "$ret" -eq 0 ] && break
-    sleep 1
-done
+retry_quiet 10 check_override || ret=1
 if [ $ret != 0 ]; then echo_i "failed"; fi
-status=`expr $status + $ret`
+status=$((status+ret))
 
 n=$((n+1))
 echo_i "checking that DS lookups for grafting forward zones are isolated ($n)"
 ret=0
-$DIG $DIGOPTS grafted A @10.53.0.4 > dig.out.$n.q1
-$DIG $DIGOPTS grafted DS @10.53.0.4 > dig.out.$n.q2
-$DIG $DIGOPTS grafted A @10.53.0.4 > dig.out.$n.q3
-$DIG $DIGOPTS grafted AAAA @10.53.0.4 > dig.out.$n.q4
+dig_with_opts grafted A @10.53.0.4 > dig.out.$n.q1 || ret=1
+dig_with_opts grafted DS @10.53.0.4 > dig.out.$n.q2 || ret=1
+dig_with_opts grafted A @10.53.0.4 > dig.out.$n.q3 || ret=1
+dig_with_opts grafted AAAA @10.53.0.4 > dig.out.$n.q4 || ret=1
 grep "status: NOERROR" dig.out.$n.q1 > /dev/null || ret=1
 grep "status: NXDOMAIN" dig.out.$n.q2 > /dev/null || ret=1
 grep "status: NOERROR" dig.out.$n.q3 > /dev/null || ret=1
 grep "status: NOERROR" dig.out.$n.q4 > /dev/null || ret=1
 if [ $ret != 0 ]; then echo_i "failed"; fi
-status=`expr $status + $ret`
+status=$((status+ret))
 
 n=$((n+1))
 echo_i "checking that rfc1918 inherited 'forward first;' zones are warned about ($n)"
@@ -141,7 +147,7 @@ ret=0
 $CHECKCONF rfc1918-inherited.conf | grep "forward first;" >/dev/null || ret=1
 $CHECKCONF rfc1918-notinherited.conf | grep "forward first;" >/dev/null && ret=1
 if [ $ret != 0 ]; then echo_i "failed"; fi
-status=`expr $status + $ret`
+status=$((status+ret))
 
 n=$((n+1))
 echo_i "checking that ULA inherited 'forward first;' zones are warned about ($n)"
@@ -149,44 +155,44 @@ ret=0
 $CHECKCONF ula-inherited.conf | grep "forward first;" >/dev/null || ret=1
 $CHECKCONF ula-notinherited.conf | grep "forward first;" >/dev/null && ret=1
 if [ $ret != 0 ]; then echo_i "failed"; fi
-status=`expr $status + $ret`
+status=$((status+ret))
 
 n=$((n+1))
 echo_i "checking that a forwarder timeout prevents it from being reused in the same fetch context ($n)"
 ret=0
 # Make ans6 receive queries without responding to them.
-echo "//" | $SENDCMD
+echo "//" | sendcmd
 # Query for a record in a zone which is forwarded to a non-responding forwarder
 # and is delegated from the root to check whether the forwarder will be retried
 # when a delegation is encountered after falling back to full recursive
 # resolution.
-$DIG $DIGOPTS txt.example7. txt @$f1 > dig.out.$n.f1 || ret=1
+dig_with_opts txt.example7. txt @$f1 > dig.out.$n.f1 || ret=1
 # The forwarder for the "example7" zone should only be queried once.
-sent=`tr -d '\r' < ns3/named.run | sed -n '/sending packet to 10.53.0.6/,/^$/p' | grep ";txt.example7.*IN.*TXT" | wc -l`
-if [ $sent -ne 1 ]; then ret=1; fi
+sent=$(tr -d '\r' < ns3/named.run | sed -n '/sending packet to 10.53.0.6/,/^$/p' | grep -c ";txt.example7.*IN.*TXT")
+if [ "$sent" -ne 1 ]; then ret=1; fi
 if [ $ret != 0 ]; then echo_i "failed"; fi
-status=`expr $status + $ret`
+status=$((status+ret))
 
 n=$((n+1))
 echo_i "checking that priming queries are not forwarded ($n)"
 ret=0
-$DIG $DIGOPTS +noadd +noauth txt.example1. txt @10.53.0.7 > dig.out.$n.f7 || ret=1
-sent=`tr -d '\r' < ns7/named.run | sed -n '/sending packet to 10.53.0.1/,/^$/p' | grep ";.*IN.*NS" | wc -l`
-[ $sent -eq 1 ] || ret=1
-sent=`grep "10.53.0.7#.* (.): query '\./NS/IN' approved" ns4/named.run | wc -l`
-[ $sent -eq 0 ] || ret=1
-sent=`grep "10.53.0.7#.* (.): query '\./NS/IN' approved" ns1/named.run | wc -l`
-[ $sent -eq 1 ] || ret=1
+dig_with_opts +noadd +noauth txt.example1. txt @10.53.0.7 > dig.out.$n.f7 || ret=1
+sent=$(tr -d '\r' < ns7/named.run | sed -n '/sending packet to 10.53.0.1/,/^$/p' | grep -c ";.*IN.*NS")
+[ "$sent" -eq 1 ] || ret=1
+sent=$(grep -c "10.53.0.7#.* (.): query '\./NS/IN' approved" ns4/named.run)
+[ "$sent" -eq 0 ] || ret=1
+sent=$(grep -c "10.53.0.7#.* (.): query '\./NS/IN' approved" ns1/named.run)
+[ "$sent" -eq 1 ] || ret=1
 if [ $ret != 0 ]; then echo_i "failed"; fi
-status=`expr $status + $ret`
+status=$((status+ret))
 
 n=$((n+1))
 echo_i "checking recovery from forwarding to a non-recursive server ($n)"
 ret=0
-$DIG $DIGOPTS xxx.sld.tld txt @10.53.0.8  > dig.out.$n.f8
+dig_with_opts xxx.sld.tld txt @10.53.0.8  > dig.out.$n.f8 || ret=1
 grep "status: NOERROR" dig.out.$n.f8 > /dev/null || ret=1
 if [ $ret != 0 ]; then echo_i "failed"; fi
-status=`expr $status + $ret`
+status=$((status+ret))
 
 echo_i "exit status: $status"
 [ $status -eq 0 ] || exit 1

From fb03edacd857ce3a9970837beafe6ea445925e18 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= <ondrej@isc.org>
Date: Fri, 6 Dec 2019 11:54:54 +0100
Subject: [PATCH 4/4] Wait for named to forward the question before testing the
 validity

---
 bin/tests/system/forward/clean.sh | 12 ++++++------
 bin/tests/system/forward/tests.sh | 31 +++++++++++++++++++++++++++----
 2 files changed, 33 insertions(+), 10 deletions(-)

diff --git a/bin/tests/system/forward/clean.sh b/bin/tests/system/forward/clean.sh
index 3c0dd682dd..46a8a87cfd 100644
--- a/bin/tests/system/forward/clean.sh
+++ b/bin/tests/system/forward/clean.sh
@@ -10,9 +10,9 @@
 #
 # Clean up after forward tests.
 #
-rm -f dig.out.*
-rm -f */named.conf
-rm -f */named.memstats
-rm -f */named.run
-rm -f ns*/named.lock
-rm -f ns*/managed-keys.bind*
+rm -f ./dig.out.*
+rm -f ./*/named.conf
+rm -f ./*/named.memstats
+rm -f ./*/named.run ./*/named.run.prev
+rm -f ./ns*/named.lock
+rm -f ./ns*/managed-keys.bind*
diff --git a/bin/tests/system/forward/tests.sh b/bin/tests/system/forward/tests.sh
index bb27d31094..abb91c3c86 100644
--- a/bin/tests/system/forward/tests.sh
+++ b/bin/tests/system/forward/tests.sh
@@ -157,6 +157,25 @@ $CHECKCONF ula-notinherited.conf | grep "forward first;" >/dev/null && ret=1
 if [ $ret != 0 ]; then echo_i "failed"; fi
 status=$((status+ret))
 
+count_sent() (
+	logfile="$1"
+	start_pattern="$2"
+	pattern="$3"
+	nextpartpeek "$logfile" | sed -n "/$start_pattern/,/^\$/p" | grep -c "$pattern"
+)
+
+check_sent() (
+	expected="$1"
+	shift
+	count=$(count_sent "$@")
+	[ "$expected" = "$count" ]
+)
+
+wait_for_log() (
+	nextpartpeek "$1" | grep "$2" >/dev/null
+
+)
+
 n=$((n+1))
 echo_i "checking that a forwarder timeout prevents it from being reused in the same fetch context ($n)"
 ret=0
@@ -166,19 +185,23 @@ echo "//" | sendcmd
 # and is delegated from the root to check whether the forwarder will be retried
 # when a delegation is encountered after falling back to full recursive
 # resolution.
+nextpart ns3/named.run >/dev/null
 dig_with_opts txt.example7. txt @$f1 > dig.out.$n.f1 || ret=1
 # The forwarder for the "example7" zone should only be queried once.
-sent=$(tr -d '\r' < ns3/named.run | sed -n '/sending packet to 10.53.0.6/,/^$/p' | grep -c ";txt.example7.*IN.*TXT")
-if [ "$sent" -ne 1 ]; then ret=1; fi
+start_pattern="sending packet to 10\.53\.0\.6"
+retry_quiet 5 wait_for_log ns3/named.run "$start_pattern"
+check_sent 1 ns3/named.run "$start_pattern" ";txt\.example7\.[[:space:]]*IN[[:space:]]*TXT$" || ret=1
 if [ $ret != 0 ]; then echo_i "failed"; fi
 status=$((status+ret))
 
 n=$((n+1))
 echo_i "checking that priming queries are not forwarded ($n)"
 ret=0
+nextpart ns7/named.run >/dev/null
 dig_with_opts +noadd +noauth txt.example1. txt @10.53.0.7 > dig.out.$n.f7 || ret=1
-sent=$(tr -d '\r' < ns7/named.run | sed -n '/sending packet to 10.53.0.1/,/^$/p' | grep -c ";.*IN.*NS")
-[ "$sent" -eq 1 ] || ret=1
+start_pattern="sending packet to 10\.53\.0\.1"
+retry_quiet 5 wait_for_log ns7/named.run "$start_pattern" || ret=1
+check_sent 1 ns7/named.run "$start_pattern" ";\.[[:space:]]*IN[[:space:]]*NS$" || ret=1
 sent=$(grep -c "10.53.0.7#.* (.): query '\./NS/IN' approved" ns4/named.run)
 [ "$sent" -eq 0 ] || ret=1
 sent=$(grep -c "10.53.0.7#.* (.): query '\./NS/IN' approved" ns1/named.run)