Improve tests a little (#1324)

* Avoid redirecting error messages in RGBLINK tests

We check that RGBLINK prints nothing to stdout, so all that
would be captured on stdout is a putative failure message.

* Require each RGBLINK test to check error output

Otherwise, you can have a test that just... checks nothing!

* Document how to add tests

Fixes #1300's last remaining item.
Also add `checkdiff` rules to remind us to update that doc if
any of the test driver scripts are updated.

CONTRIBUTING.md

@@ -69,3 +69,88 @@ header.
 6. Create a pull request against the branch `master`.
 7. Be prepared to get some comments about your code and to modify it. Tip: Use
    `git rebase -i origin/master` to modify chains of commits.
+
+## Adding a test
+
+The test suite is a little ad-hoc, so the way tests work is different for each program being tested.
+
+Feel free to modify how the test scripts work, if the thing you want to test doesn't fit the existing scheme(s).
+
+### RGBASM
+
+Each `.asm` file corresponds to one test.
+RGBASM will be invoked on the `.asm` file with all warnings enabled.
+
+If a `.out` file exists, RGBASM's output (`print`, `println`, etc.) must match its contents.
+If a `.err` file exists, RGBASM's error output (`warn`, errors, etc.) must match its contents.
+
+If a `.out.bin` file exists, the object file will be linked, and the generated ROM truncated to the length of the `.out.bin` file.
+After that, the ROM must match the `.out.bin` file.
+
+### RGBLINK
+
+Each `.asm` file corresponds to one test, or one *set* of tests.
+
+All tests begin by assembling the `.asm` file into an object file, which will be linked in various ways depending on the test.
+
+#### Simple tests
+
+These simply check that RGBLINK's output matches some expected output.
+
+A `.out` file **must** exist, and RGBLINK's output must match that file's contents.
+
+Additionally, if a `.out.bin` file exists, the `.gb` file generated by RGBLINK must match it.
+
+#### Linker script tests
+
+These allow applying various linker scripts to the same object file.
+If one or more `.link` files exist, whose names start the same as the `.asm` file, then each of those files correspond to one test.
+
+Each `.link` linker script **must** be accompanied by a `.out` file, and RGBLINK's output must match that file's contents when passed the corresponding linker script.
+
+#### Variant tests
+
+These allow testing RGBLINK's `-d`, `-t`, and `-w` flags.
+If one or more <code>-<var>&lt;flag&gt;</var>.out</code> or <code>-no-<var>&lt;flag&gt;</var>.out</code> files exist, then each of them corresponds to one test.
+
+The object file will be linked with and without said flag, respectively; and in each case, RGBLINK's output must match the `.out` file's contents.
+
+### RGBFIX
+
+Each `.bin` file corresponds to one test, and **must** be accompanied by a `.flags` file and a `.err` file.
+
+The `.flags` file is a text file whose first line contains flags to pass to RGBFIX.
+(There may be more lines, which will be ignored; they can serve as comments to explain what the test is about.)
+
+RGBFIX will be invoked on the `.bin` file, and its error output must match the contents of the `.err` file.
+(If no errors ought to be printed, then the `.err` file should just be empty.)
+
+Additionally, if a `.gb` file exists, the output of RGBFIX must match the `.gb`.
+
+### RGBGFX
+
+Each `.png` file corresponds to one test.
+RGBGFX will be invoked on the file.
+If a `.flags` file exists, it will be used as part of the RGBGFX invocation (<code>@<var>&lt;file&gt;</var>.flags</code>).
+
+If no `.err` file exists, RGBGFX is simply expected to be able to process the file normally.
+If one *does* exist, RGBGFX's return status is ignored, but its output **must** match the `.err` file's contents.
+
+### Downstream projects
+
+1. Make sure the downstream project supports <code>make <var>&lt;target&gt;</var> RGBDS=<var>&lt;path/to/RGBDS/&gt;</var></code>.
+   While the test suite supports any Make target name, only [Make](//gnu.org/software/make) is currently supported, and the Makefile must support a `RGBDS` variable to use a non-system RGBDS directory.
+
+   Also, only projects hosted on GitHub are currently supported.
+2. Add the project to `test/fetch-test-deps.sh`: add a new `action` line at the bottom, following the existing pattern:
+   
+   ```sh
+   action  <owner>/<repo>  <date of last commit>  <hash of last commit>
+   ```
+
+   (The date is used to avoid fetching too much history when cloning the repositories.)
+3. Add the project to `test/run-tests.sh`: add a new `test_downstream` line at the bottom, following the existing pattern:
+
+   ```sh
+   test_downstream  <repo>  <makefile target>
+   ```

contrib/checkdiff.bash

@@ -66,3 +66,16 @@ dependency src/gfx/main.cpp        contrib/zsh_compl/_rgbgfx \
            "Did the rgbgfx CLI change?"
 dependency src/gfx/main.cpp        contrib/bash_compl/_rgbgfx.bash \
            "Did the rgbgfx CLI change?"
+
+dependency test/fetch-test-deps.sh CONTRIBUTING.md \
+           "Did the test protocol change?"
+dependency test/run-tests.sh       CONTRIBUTING.md \
+           "Did the test protocol change?"
+dependency test/asm/test.sh        CONTRIBUTING.md \
+           "Did the RGBASM test protocol change?"
+dependency test/link/test.sh       CONTRIBUTING.md \
+           "Did the RGBLINK test protocol change?"
+dependency test/fix/test.sh        CONTRIBUTING.md \
+           "Did the RGBFIX test protocol change?"
+dependency test/gfx/test.sh        CONTRIBUTING.md \
+           "Did the RGBGFX test protocol change?"

test/link/test.sh

@@ -76,14 +76,14 @@ for i in *.asm; do
 	for flag in '-d' '-t' '-w'; do
 		if [ -f "${i%.asm}-no${flag}.out" ]; then
 			continueTest "-no${flag}"
-			rgblinkQuiet -o "$gbtemp" "$otemp" >"$outtemp" 2>&1
+			rgblinkQuiet -o "$gbtemp" "$otemp" 2>"$outtemp"
 			tryDiff "${i%.asm}-no${flag}.out" "$outtemp"
 			(( rc = rc || $? ))
 			ran_flag=true
 		fi
 		if [ -f "${i%.asm}${flag}.out" ]; then
 			continueTest "$flag"
-			rgblinkQuiet ${flag} -o "$gbtemp" "$otemp" >"$outtemp" 2>&1
+			rgblinkQuiet ${flag} -o "$gbtemp" "$otemp" 2>"$outtemp"
 			tryDiff "${i%.asm}${flag}.out" "$outtemp"
 			(( rc = rc || $? ))
 			ran_flag=true
@@ -98,7 +98,7 @@ for i in *.asm; do
 		[[ -e "$script" ]] || break # If the glob doesn't match, it just... doesn't expand!
 
 		continueTest "${script#${i%.asm}}"
-		rgblinkQuiet -l "$script" -o "$gbtemp" "$otemp" >"$outtemp" 2>&1
+		rgblinkQuiet -l "$script" -o "$gbtemp" "$otemp" 2>"$outtemp"
 		tryDiff "${script%.link}.out" "$outtemp"
 		(( rc = rc || $? ))
 		ran_flag=true
@@ -109,11 +109,9 @@ for i in *.asm; do
 
 	# The rest of the tests just links a file, and maybe checks the binary
 	continueTest
-	rgblinkQuiet -o "$gbtemp" "$otemp" >"$outtemp" 2>&1
+	rgblinkQuiet -o "$gbtemp" "$otemp" 2>"$outtemp"
-	if [ -f "${i%.asm}.out" ]; then
+	tryDiff "${i%.asm}.out" "$outtemp"
-		tryDiff "${i%.asm}.out" "$outtemp"
+	(( rc = rc || $? ))
-		(( rc = rc || $? ))
-	fi
 
 	bin=${i%.asm}.out.bin
 	if [ -f "$bin" ]; then
@@ -129,7 +127,7 @@ startTest
 "$RGBASM" -o "$otemp" bank-const/a.asm
 "$RGBASM" -o "$gbtemp2" bank-const/b.asm
 continueTest
-rgblinkQuiet -o "$gbtemp" "$gbtemp2" "$otemp" >"$outtemp" 2>&1
+rgblinkQuiet -o "$gbtemp" "$gbtemp2" "$otemp" 2>"$outtemp"
 tryDiff bank-const/out.err "$outtemp"
 (( rc = rc || $? ))
 
@@ -161,7 +159,7 @@ i="overlay.asm"
 startTest
 "$RGBASM" -o "$otemp" overlay/a.asm
 continueTest
-rgblinkQuiet -o "$gbtemp" -t -O overlay/overlay.gb "$otemp" >"$outtemp" 2>&1
+rgblinkQuiet -o "$gbtemp" -t -O overlay/overlay.gb "$otemp" 2>"$outtemp"
 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
@@ -172,7 +170,7 @@ i="scramble-romx.asm"
 startTest
 "$RGBASM" -o "$otemp" scramble-romx/a.asm
 continueTest
-rgblinkQuiet -o "$gbtemp" -S romx=3 "$otemp" >"$outtemp" 2>&1
+rgblinkQuiet -o "$gbtemp" -S romx=3 "$otemp" 2>"$outtemp"
 tryDiff scramble-romx/out.err "$outtemp"
 (( rc = rc || $? ))
 # This test does not compare its exact output with 'tryCmpRom' because no scrambling order is guaranteed