From 0da216897a461982106ba1bd713088088321a52e Mon Sep 17 00:00:00 2001 From: Eldred Habert Date: Mon, 4 Mar 2024 01:12:29 +0100 Subject: [PATCH] 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 | 85 ++++++++++++++++++++++++++++++++++++ contrib/checkdiff.bash | 13 ++++++ test/link/sizeof-startof.out | 0 test/link/test.sh | 20 ++++----- 4 files changed, 107 insertions(+), 11 deletions(-) create mode 100644 test/link/sizeof-startof.out diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index eb4b6819..c03d834f 100644 --- a/CONTRIBUTING.md +++ b/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 -<flag>.out or -no-<flag>.out 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 (@<file>.flags). + +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 make <target> RGBDS=<path/to/RGBDS/>. + 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 / + ``` + + (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 + ``` diff --git a/contrib/checkdiff.bash b/contrib/checkdiff.bash index 434314f0..57cf8e99 100755 --- a/contrib/checkdiff.bash +++ b/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?" diff --git a/test/link/sizeof-startof.out b/test/link/sizeof-startof.out new file mode 100644 index 00000000..e69de29b diff --git a/test/link/test.sh b/test/link/test.sh index 6a996c89..8523021d 100755 --- a/test/link/test.sh +++ b/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 - if [ -f "${i%.asm}.out" ]; then - tryDiff "${i%.asm}.out" "$outtemp" - (( rc = rc || $? )) - fi + rgblinkQuiet -o "$gbtemp" "$otemp" 2>"$outtemp" + tryDiff "${i%.asm}.out" "$outtemp" + (( rc = rc || $? )) 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