Fix shellcheck warnings in the test scripts

Making them more robust to changes.
We ought to automate this some day.

My version of ShellCheck (v0.9.0) errors on test/gfx/test.sh, though...

test/asm/test.sh

@@ -12,7 +12,9 @@ output="$(mktemp)"
 errput="$(mktemp)"
 rc=0
 
-trap "rm -f '$o' '$gb' '$input' '$output' '$errput'" EXIT
+# Immediate expansion is the desired behaviour.
+# shellcheck disable=SC2064
+trap "rm -f ${o@Q} ${gb@Q} ${input@Q} ${output@Q} ${errput@Q}" EXIT
 
 bold="$(tput bold)"
 resbold="$(tput sgr0)"
@@ -57,9 +59,9 @@ fi
 
 # Check whether to use '.simple.err' files if they exist
 # (rgbasm with pre-3.0 Bison just reports "syntax error")
-$RGBASM -Weverything -o $o syntax-error.asm > $output 2> $errput
+$RGBASM -Weverything -o "$o" syntax-error.asm >"$output" 2>"$errput"
 simple_error=0
-if ! diff --strip-trailing-cr syntax-error.err $errput; then
+if ! diff --strip-trailing-cr syntax-error.err "$errput"; then
 	echo "${bold}${orange}Warning: using .simple.err files when available.${rescolors}${resbold}"
 	simple_error=1
 fi
@@ -67,19 +69,19 @@ fi
 for i in *.asm; do
 	flags=${i%.asm}.flags
 	RGBASMFLAGS=-Weverything
-	if [ -f $flags ]; then
+	if [ -f "$flags" ]; then
 		RGBASMFLAGS="$(head -n 1 "$flags")" # Allow other lines to serve as comments
 	fi
 	for variant in '' '.pipe'; do
 		echo "${bold}${green}${i%.asm}${variant}...${rescolors}${resbold}"
 		desired_errname=${i%.asm}.err
-		if [ "$simple_error" -eq 1 ] && [ -e ${i%.asm}.simple.err ]; then
-			desired_errname=${i%.asm}.simple.err
+		if [ "$simple_error" -eq 1 ] && [ -e "${i%.asm}.simple.err" ]; then
+			desired_errname="${i%.asm}.simple.err"
 		fi
 		if [ -z "$variant" ]; then
-			$RGBASM $RGBASMFLAGS -o $o $i > $output 2> $errput
-			desired_output=${i%.asm}.out
-			desired_errput=$desired_errname
+			$RGBASM $RGBASMFLAGS -o "$o" "$i" >"$output" 2>"$errput"
+			desired_output="${i%.asm}.out"
+			desired_errput="$desired_errname"
 		else
 			# `include-recursion.asm` refers to its own name inside the test code.
 			# Skip testing with stdin input for that file.
@@ -91,32 +93,35 @@ for i in *.asm; do
 			# stdin redirection makes the input an unseekable pipe - a scenario
 			# that's harder to deal with and was broken when the feature was
 			# first implemented.
-			cat $i | $RGBASM $RGBASMFLAGS -o $o - > $output 2> $errput
+			# shellcheck disable=SC2002
+			cat "$i" | "$RGBASM" $RGBASMFLAGS -o "$o" - >"$output" 2>"$errput"
 
 			# Use two otherwise unused files for temp storage
-			desired_output=$input
-			desired_errput=$gb
+			desired_output="$input"
+			desired_errput="$gb"
 			# Escape regex metacharacters
 			subst="$(printf '%s\n' "$i" | sed 's:[][\/.^$*]:\\&:g')"
 			# Replace the file name with a dash to match changed output
-			sed "s/$subst/<stdin>/g" ${i%.asm}.out > $desired_output
-			sed "s/$subst/<stdin>/g" $desired_errname > $desired_errput
+			sed "s/$subst/<stdin>/g" "${i%.asm}.out" >"$desired_output"
+			sed "s/$subst/<stdin>/g" "$desired_errname" >"$desired_errput"
 		fi
 
-		tryDiff $desired_output $output out
+		tryDiff "$desired_output" "$output" out
 		our_rc=$?
-		tryDiff $desired_errput $errput err
-		our_rc=$(($? || $our_rc))
+		tryDiff "$desired_errput" "$errput" err
+		(( our_rc = our_rc || $? ))
 
 		bin=${i%.asm}.out.bin
-		if [ -f $bin ]; then
-			$RGBLINK -o $gb $o
-			dd if=$gb count=1 bs=$(printf %s $(wc -c < $bin)) > $output 2>/dev/null
-			tryCmp $bin $output
-			our_rc=$(($? || $our_rc))
+		if [ -f "$bin" ]; then
+			"$RGBLINK" -o "$gb" "$o"
+			# `printf` ensures we only capture the first word.
+			bin_size=$(printf %s $(wc -c <"$bin"))
+			dd "if=$gb" count=1 "bs=$bin_size" >"$output" 2>/dev/null
+			tryCmp "$bin" "$output"
+			(( our_rc = our_rc || $? ))
 		fi
 
-		rc=$(($rc || $our_rc))
+		(( rc = rc || our_rc ))
 		if [ $our_rc -ne 0 ]; then break; fi
 	done
 done

test/fix/test.sh

@@ -7,8 +7,10 @@ src="$PWD"
 rc=0
 
 cp ../../{rgbfix,contrib/gbdiff.bash} "$tmpdir"
-cd "$tmpdir"
-trap "cd; rm -rf '$tmpdir'" EXIT
+cd "$tmpdir" || exit
+# Immediate expansion is the desired behaviour.
+# shellcheck disable=SC2064
+trap "cd; rm -rf ${tmpdir@Q}" EXIT
 
 bold="$(tput bold)"
 resbold="$(tput sgr0)"
@@ -43,7 +45,7 @@ runTest () {
 		fi
 		if [[ -z "$variant" ]]; then
 			cp "$2/$1.bin" out.gb
-			if [[ -n "$(eval $RGBFIX $flags out.gb '2>out.err')" ]]; then
+			if [[ -n "$(eval "$RGBFIX" $flags out.gb '2>out.err')" ]]; then
 				echo "${bold}${red}Fixing $1 in-place shouldn't output anything on stdout!${rescolors}${resbold}"
 				our_rc=1
 			fi
@@ -52,18 +54,19 @@ runTest () {
 			# Stop! This is not a Useless Use Of Cat. Using cat instead of
 			# stdin redirection makes the input an unseekable pipe - a scenario
 			# that's harder to deal with.
+			# shellcheck disable=SC2002
 			cat "$2/$1.bin" | eval $RGBFIX "$flags" - '>out.gb' '2>out.err'
 			subst='<stdin>'
 		fi
 
 		sed "s/$subst/<filename>/g" "out.err" | tryDiff "$2/$1.err" - "$1.err${variant}"
-		our_rc=$(($? || $our_rc))
+		(( our_rc = our_rc || $? ))
 		if [[ -r "$2/$1.gb" ]]; then
 			tryCmp "$2/$1.gb" "out.gb" "$1.gb${variant}"
-			our_rc=$(($? || $our_rc))
+			(( our_rc = our_rc || $? ))
 		fi
 
-		rc=$(($rc || $our_rc))
+		(( rc = rc || our_rc ))
 		if [[ $our_rc -ne 0 ]]; then break; fi
 	done
 }

test/link/test.sh

@@ -9,7 +9,9 @@ gbtemp2="$(mktemp)"
 outtemp="$(mktemp)"
 rc=0
 
-trap "rm -f '$otemp' '$gbtemp' '$gbtemp2' '$outtemp'" EXIT
+# Immediate expansion is the desired behaviour.
+# shellcheck disable=SC2064
+trap "rm -f ${otemp@Q} ${gbtemp@Q} ${gbtemp2@Q} ${outtemp@Q}" EXIT
 
 bold="$(tput bold)"
 resbold="$(tput sgr0)"
@@ -38,6 +40,12 @@ tryCmp () {
 		false
 	fi
 }
+tryCmpRom () {
+	# `printf` lets us keep only the first returned word.
+	rom_size=$(printf %s $(wc -c <"$1"))
+	dd if="$gbtemp" count=1 bs="$rom_size" >"$otemp" 2>/dev/null
+	tryCmp "$1" "$otemp"
+}
 
 rgblinkQuiet () {
 	out="$(env $RGBLINK "$@")" || return $?
@@ -49,51 +57,50 @@ rgblinkQuiet () {
 
 for i in *.asm; do
 	startTest
-	$RGBASM -o $otemp $i
+	"$RGBASM" -o "$otemp" "$i"
 
 	# Some tests have variants depending on flags
-	ran_flag=
+	ran_flag=false
 	for flag in '-d' '-t' '-w'; do
-		if [ -f ${i%.asm}-no${flag}.out ]; then
-			rgblinkQuiet -o $gbtemp $otemp > $outtemp 2>&1
-			tryDiff ${i%.asm}-no${flag}.out $outtemp
-			rc=$(($? || $rc))
-			ran_flag=1
+		if [ -f "${i%.asm}-no${flag}.out" ]; then
+			rgblinkQuiet -o "$gbtemp" "$otemp" >"$outtemp" 2>&1
+			tryDiff "${i%.asm}-no${flag}.out" "$outtemp"
+			(( rc = rc || $? ))
+			ran_flag=true
 		fi
-		if [ -f ${i%.asm}${flag}.out ]; then
-			rgblinkQuiet ${flag} -o $gbtemp $otemp > $outtemp 2>&1
-			tryDiff ${i%.asm}${flag}.out $outtemp
-			rc=$(($? || $rc))
-			ran_flag=1
+		if [ -f "${i%.asm}${flag}.out" ]; then
+			rgblinkQuiet ${flag} -o "$gbtemp" "$otemp" >"$outtemp" 2>&1
+			tryDiff "${i%.asm}${flag}.out" "$outtemp"
+			(( rc = rc || $? ))
+			ran_flag=true
 		fi
 	done
-	if [ -n "$ran_flag" ]; then
+	if "$ran_flag"; then
 		continue
 	fi
 
 	# Other tests have several linker scripts
-	find . -name "${i%.asm}*.link" | while read script; do
-		rgblinkQuiet -l $script -o $gbtemp $otemp > $outtemp 2>&1
-		tryDiff ${script%.link}.out $outtemp
-		rc=$(($? || $rc))
+	while read -rd '' script; do
+		rgblinkQuiet -l "$script" -o "$gbtemp" "$otemp" >"$outtemp" 2>&1
+		tryDiff "${script%.link}.out" "$outtemp"
+		(( rc = rc || $? ))
 		ran_flag=1
-	done
+	done < <(find . -name "${i%.asm}*.link" -print0)
 	if [ -n "$ran_flag" ]; then
 		continue
 	fi
 
 	# The rest of the tests just links a file, and maybe checks the binary
-	rgblinkQuiet -o $gbtemp $otemp > $outtemp 2>&1
-	if [ -f ${i%.asm}.out ]; then
-		tryDiff ${i%.asm}.out $outtemp
-		rc=$(($? || $rc))
+	rgblinkQuiet -o "$gbtemp" "$otemp" >"$outtemp" 2>&1
+	if [ -f "${i%.asm}.out" ]; then
+		tryDiff "${i%.asm}.out" "$outtemp"
+		(( rc = rc || $? ))
 	fi
 
 	bin=${i%.asm}.out.bin
-	if [ -f $bin ]; then
-		dd if=$gbtemp count=1 bs=$(printf %s $(wc -c < $bin)) > $otemp 2>/dev/null
-		tryCmp $bin $otemp
-		rc=$(($? || $rc))
+	if [ -f "$bin" ]; then
+		tryCmpRom "$bin"
+		(( rc = rc || $? ))
 	fi
 done
 
@@ -101,85 +108,81 @@ done
 
 i="bank-const.asm"
 startTest
-$RGBASM -o $otemp bank-const/a.asm
-$RGBASM -o $gbtemp2 bank-const/b.asm
-rgblinkQuiet -o $gbtemp $gbtemp2 $otemp > $outtemp 2>&1
-tryDiff bank-const/out.err $outtemp
-rc=$(($? || $rc))
+"$RGBASM" -o "$otemp" bank-const/a.asm
+"$RGBASM" -o "$gbtemp2" bank-const/b.asm
+rgblinkQuiet -o "$gbtemp" "$gbtemp2" "$otemp" >"$outtemp" 2>&1
+tryDiff bank-const/out.err "$outtemp"
+(( rc = rc || $? ))
 
 for i in fragment-align/*; do
 	startTest
-	$RGBASM -o $otemp $i/a.asm
-	$RGBASM -o $gbtemp2 $i/b.asm
-	rgblinkQuiet -o $gbtemp $otemp $gbtemp2 2>$outtemp
-	tryDiff $i/out.err $outtemp
-	rc=$(($? || $rc))
-	if [[ -f $i/out.gb ]]; then
-		dd if=$gbtemp count=1 bs=$(printf %s $(wc -c < $i/out.gb)) > $otemp 2>/dev/null
-		tryCmp $i/out.gb $otemp
-		rc=$(($? || $rc))
+	"$RGBASM" -o "$otemp" "$i"/a.asm
+	"$RGBASM" -o "$gbtemp2" "$i"/b.asm
+	rgblinkQuiet -o "$gbtemp" "$otemp" "$gbtemp2" 2>"$outtemp"
+	tryDiff "$i"/out.err "$outtemp"
+	(( rc = rc || $? ))
+	if [[ -f "$i"/out.gb ]]; then
+		tryCmpRom "$i"/out.gb
+		(( rc = rc || $? ))
 	fi
 done
 
 i="high-low.asm"
 startTest
-$RGBASM -o $otemp high-low/a.asm
-rgblinkQuiet -o $gbtemp $otemp
-$RGBASM -o $otemp high-low/b.asm
-rgblinkQuiet -o $gbtemp2 $otemp
-tryCmp $gbtemp $gbtemp2
-rc=$(($? || $rc))
+"$RGBASM" -o "$otemp" high-low/a.asm
+rgblinkQuiet -o "$gbtemp" "$otemp"
+"$RGBASM" -o "$otemp" high-low/b.asm
+rgblinkQuiet -o "$gbtemp2" "$otemp"
+tryCmp "$gbtemp" "$gbtemp2"
+(( rc = rc || $? ))
 
 i="overlay.asm"
 startTest
-$RGBASM -o $otemp overlay/a.asm
-rgblinkQuiet -o $gbtemp -t -O overlay/overlay.gb $otemp > $outtemp 2>&1
-tryDiff overlay/out.err $outtemp
-rc=$(($? || $rc))
+"$RGBASM" -o "$otemp" overlay/a.asm
+rgblinkQuiet -o "$gbtemp" -t -O overlay/overlay.gb "$otemp" >"$outtemp" 2>&1
+tryDiff overlay/out.err "$outtemp"
+(( rc = rc || $? ))
 # This test does not trim its output with 'dd' because it needs to verify the correct output size
-tryCmp overlay/out.gb $gbtemp
-rc=$(($? || $rc))
+tryCmp overlay/out.gb "$gbtemp"
+(( rc = rc || $? ))
 
 i="section-fragment/jr-offset.asm"
 startTest
-$RGBASM -o $otemp section-fragment/jr-offset/a.asm
-$RGBASM -o $gbtemp2 section-fragment/jr-offset/b.asm
-rgblinkQuiet -o $gbtemp $otemp $gbtemp2
-dd if=$gbtemp count=1 bs=$(printf %s $(wc -c < section-fragment/jr-offset/ref.out.bin)) > $otemp 2>/dev/null
-tryCmp section-fragment/jr-offset/ref.out.bin $otemp
-rc=$(($? || $rc))
+"$RGBASM" -o "$otemp" section-fragment/jr-offset/a.asm
+"$RGBASM" -o "$gbtemp2" section-fragment/jr-offset/b.asm
+rgblinkQuiet -o "$gbtemp" "$otemp" "$gbtemp2"
+tryCmpRom section-fragment/jr-offset/ref.out.bin
+(( rc = rc || $? ))
 
 i="section-union/good.asm"
 startTest
-$RGBASM -o $otemp section-union/good/a.asm
-$RGBASM -o $gbtemp2 section-union/good/b.asm
-rgblinkQuiet -o $gbtemp -l section-union/good/script.link $otemp $gbtemp2
-dd if=$gbtemp count=1 bs=$(printf %s $(wc -c < section-union/good/ref.out.bin)) > $otemp 2>/dev/null
-tryCmp section-union/good/ref.out.bin $otemp
-rc=$(($? || $rc))
+"$RGBASM" -o "$otemp" section-union/good/a.asm
+"$RGBASM" -o "$gbtemp2" section-union/good/b.asm
+rgblinkQuiet -o "$gbtemp" -l section-union/good/script.link "$otemp" "$gbtemp2"
+tryCmpRom section-union/good/ref.out.bin
+(( rc = rc || $? ))
 
 i="section-union/fragments.asm"
 startTest
-$RGBASM -o $otemp section-union/fragments/a.asm
-$RGBASM -o $gbtemp2 section-union/fragments/b.asm
-rgblinkQuiet -o $gbtemp $otemp $gbtemp2
-dd if=$gbtemp count=1 bs=$(printf %s $(wc -c < section-union/fragments/ref.out.bin)) > $otemp 2>/dev/null
-tryCmp section-union/fragments/ref.out.bin $otemp
-rc=$(($? || $rc))
+"$RGBASM" -o "$otemp" section-union/fragments/a.asm
+"$RGBASM" -o "$gbtemp2" section-union/fragments/b.asm
+rgblinkQuiet -o "$gbtemp" "$otemp" "$gbtemp2"
+tryCmpRom section-union/fragments/ref.out.bin
+(( rc = rc || $? ))
 
 for i in section-union/*.asm; do
 	startTest
-	$RGBASM -o $otemp $i
-	$RGBASM -o $gbtemp2 $i -DSECOND
-	if rgblinkQuiet $otemp $gbtemp2 2>$outtemp; then
+	"$RGBASM" -o "$otemp" "$i"
+	"$RGBASM" -o "$gbtemp2" "$i" -DSECOND
+	if rgblinkQuiet "$otemp" "$gbtemp2" 2>"$outtemp"; then
 		echo -e "${bold}${red}$i didn't fail to link!${rescolors}${resbold}"
 		rc=1
 	fi
-	echo --- >> $outtemp
+	echo --- >>"$outtemp"
 	# Ensure RGBASM also errors out
-	cat $i - $i <<<'def SECOND equs "1"' | $RGBASM - 2>> $outtemp
-	tryDiff ${i%.asm}.out $outtemp
-	rc=$(($? || $rc))
+	cat "$i" - "$i" <<<'def SECOND equs "1"' | "$RGBASM" - 2>>"$outtemp"
+	tryDiff "${i%.asm}.out" "$outtemp"
+	(( rc = rc || $? ))
 done
 
 exit $rc