From 6f0defbfe588881dcc1637de6dfbc8f4e7dbff10 Mon Sep 17 00:00:00 2001 From: ISSOtm Date: Fri, 24 Nov 2023 20:52:46 +0100 Subject: [PATCH] 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 | 57 +++++++++-------- test/fix/test.sh | 15 +++-- test/link/test.sh | 159 +++++++++++++++++++++++----------------------- 3 files changed, 121 insertions(+), 110 deletions(-) diff --git a/test/asm/test.sh b/test/asm/test.sh index 8b1db2b8..9d7b53d4 100755 --- a/test/asm/test.sh +++ b/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)" @@ -40,9 +42,9 @@ tryCmp () { } # Add the version constants test, outputting the closest tag to the HEAD -if git describe --tags --abbrev=0 > version.out; then - $RGBASM --version >> version.out - cat > version.asm <version.out; then + $RGBASM --version >>version.out + cat >version.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//g" ${i%.asm}.out > $desired_output - sed "s/$subst//g" $desired_errname > $desired_errput + sed "s/$subst//g" "${i%.asm}.out" >"$desired_output" + sed "s/$subst//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 diff --git a/test/fix/test.sh b/test/fix/test.sh index 145c2421..0e376d2d 100755 --- a/test/fix/test.sh +++ b/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='' fi sed "s/$subst//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 } diff --git a/test/link/test.sh b/test/link/test.sh index 71ddab89..7c51c66d 100755 --- a/test/link/test.sh +++ b/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