Refactor the test harness for external repo tests (#1994)

- Use CTest labels to filter tests ("internal"/"external", "free"/"nonfree",
  and individual tool+project names) instead of defining `TESTS_RUN_NONFREE`
- Allow each external test to run independently in CTest
- Remove the unused no-op `fetch-test-deps.sh --only-internal` option

.gitattributes

@@ -1,6 +1,7 @@
 # Shell scripts need Unix line endings (see https://github.com/gbdev/rgbds/issues/841)
 *.sh text eol=lf
 *.bash text eol=lf
+*.cfg text eol=lf
 
 # Flags also need Unix line endings (see https://github.com/gbdev/rgbds/issues/955)
 *.flags text eol=lf

docs/CONTRIBUTING.md

@@ -64,20 +64,28 @@ years). If you are adding new files, you need to use the
 4. Compile your changes with `make develop` instead of just `make`. This
    target checks for additional warnings. Your patches shouldn't introduce any
    new warning (but it may be possible to remove some warning checks if it makes
-   the code much easier). You can also use `cmake --preset develop` if you prefer.
+   the code much easier).
-5. Test your changes by running `./run-tests.sh` in the `test` directory, or using `ctest`.
+   
-   (You must run `./fetch-test-deps.sh` first; if you forget to, the test suite
+   You can also use `cmake --preset develop` if you prefer.
-   will fail and remind you mid-way.)
+5. Test your changes by running `./run-tests.sh` in the `test` directory.
+   `./run-tests.sh --help` will print its available options. (You must run
+   `./fetch-test-deps.sh` first; if you forget to, the test suite will fail and
+   remind you mid-way.)
+   
+   You can also use `ctest --test-dir build` if you prefer.
+   `ctest --test-dir build --print-labels` will print its available
+   [test labels](https://cmake.org/cmake/help/latest/manual/ctest.1.html#label-matching).
 6. Format your changes according to `clang-format`, which will reformat the
-   coding style according to our standards defined in `.clang-format`.
+   coding style according to our standards defined in `.clang-format`. You can
+   use `make format` to format all the C++ files.
 7. Create a pull request against the branch `master`.
 8. Check the results of the GitHub Actions CI jobs for your pull request. The
    "Code format checking" and "Regression testing" jobs should all succeed.
-   The "Diff completeness check" and "Static analysis" jobs should be manually
+   The "Diff completeness check" job should be manually checked, as it may
-   checked, as they may output warnings which do not count as failure errors.
+   output warnings which do not count as failure errors. The "Code coverage
-   The "Code coverage report" provides an
+   report" provides an [LCOV](https://github.com/linux-test-project/lcov)-generated
-   [LCOV](https://github.com/linux-test-project/lcov)-generated report which
+   report which can be downloaded and checked to see if your new code has full
-   can be downloaded and checked to see if your new code has full test coverage.
+   test coverage.
 9. 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.
 
@@ -215,24 +223,34 @@ If a `.flags` file exists, it will be used as part of the RGBGFX invocation
 Each `seed*.bin` file corresponds to one test.
 Each one is a binary RNG file which is passed to the `rgbgfx_test` program.
 
-### Downstream projects
+### External projects
+
+Each `*.cfg` file corresponds to one test.
+Each one defines the parameters for a real external project that builds using RGBDS.
 
 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.
-2. Add the project to `test/fetch-test-deps.sh`: add a new `action` line at the
+2. Add the project to `test/fetch-test-deps.sh`: add a new line at the bottom,
-   bottom, following the existing pattern:
+   following the existing pattern:
    
    ```sh
-   action  <domain>  <owner>  <repo>  <hash of last commit>
+   . external/<name>.cfg && action
    ```
-3. Add the project to `test/run-tests.sh`: add a new `test_downstream` line at
+3. Add the project to `test/run-tests.sh`: add a new line at the bottom,
-   the bottom, following the existing pattern:
+   following the existing pattern:
    
    ```sh
-   test_downstream  <owner>  <repo>  <makefile target>  <build file>  <sha1 hash of build file>
+   ./external/test.sh <name>
+   ```
+4. Add the project to `CMakeLists.txt`: add it to the `foreach(project ...)`
+   list, and add new lines at the bottom, following the existing pattern:
+   
+   ```cmake
+   set_tests_properties(<name> PROPERTIES LABELS "<name>;external;free"
+                               FIXTURES_REQUIRED "free-repos")
    ```
 
 ## Container images

test/CMakeLists.txt

@@ -1,6 +1,5 @@
 # SPDX-License-Identifier: MIT
 
-option(TESTS_RUN_NONFREE "Run tests that build nonfree codebases." ON)
 set(TESTS_OS_NAME "" CACHE STRING "Skip running tests known to fail on this OS.")
 
 add_executable(randtilegen gfx/randtilegen.cpp)
@@ -18,35 +17,52 @@ foreach(prog "randtilegen" "rgbgfx_test")
                      COMMAND_EXPAND_LISTS VERBATIM)
 endforeach()
 
-set(ONLY_FREE)
-if(NOT TESTS_RUN_NONFREE)
-  set(ONLY_FREE "--only-free")
-endif()
-
-set(OS_NAME)
-if(NOT TESTS_OS_NAME STREQUAL "")
-  set(OS_NAME "--os" "${TESTS_OS_NAME}")
-endif()
-
 foreach(component "asm" "link" "fix" "gfx")
   add_test(NAME "rgb${component}"
            COMMAND bash -- test.sh
            WORKING_DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}/${component}")
   set_tests_properties("rgb${component}" PROPERTIES REQUIRED_FILES "$<TARGET_FILE:rgb${component}>"
                                                     PROCESSORS 1
-                                                    LABELS "rgb${component}")
+                                                    LABELS "rgb${component};internal;free")
 endforeach()
 set_tests_properties(rgbgfx PROPERTIES REQUIRED_FILES "$<TARGET_FILE:rgbgfx>;$<TARGET_FILE:randtilegen>;$<TARGET_FILE:rgbgfx_test>")
 
 add_test(NAME fetch-test-deps
-         COMMAND bash -- fetch-test-deps.sh ${ONLY_FREE}
+         COMMAND bash -- fetch-test-deps.sh --only-free
          WORKING_DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}")
-set_tests_properties(fetch-test-deps PROPERTIES FIXTURES_SETUP "external-repos"
+set_tests_properties(fetch-test-deps PROPERTIES FIXTURES_SETUP "free-repos"
                                                 LABELS "external")
-add_test(NAME external
+add_test(NAME fetch-nonfree-deps
-         COMMAND bash -- run-tests.sh --only-external ${ONLY_FREE} ${OS_NAME}
+         COMMAND bash -- fetch-test-deps.sh
          WORKING_DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}")
-set_tests_properties(external PROPERTIES DEPENDS "rgbasm;rgblink;rgbfix;rgbgfx" # Only attempt building whole projects if each tool passes muster on its own.
+set_tests_properties(fetch-nonfree-deps PROPERTIES FIXTURES_SETUP "external-repos"
-                                         PROCESSORS 4
+                                                   FIXTURES_REQUIRED "free-repos"
-                                         FIXTURES_REQUIRED "external-repos"
+                                                   LABELS "external;nonfree")
-                                         LABELS "external") # Allow filtering out external tests.
+
+foreach(project "pokecrystal" "pokered" "ladx" "ucity" "libbet" "sameboy" "gb-starter-kit")
+  add_test(NAME "${project}"
+           COMMAND bash -- external/test.sh ${project}
+           WORKING_DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}")
+  set_tests_properties(${project} PROPERTIES DEPENDS "rgbasm;rgblink;rgbfix;rgbgfx" # Only attempt building whole projects if each tool passes muster on its own.
+                                             PROCESSORS 4)
+endforeach()
+set_tests_properties(pokecrystal    PROPERTIES LABELS "pokecrystal;external;nonfree"
+                                               FIXTURES_REQUIRED "external-repos")
+set_tests_properties(pokered        PROPERTIES LABELS "pokered;external;nonfree"
+                                               FIXTURES_REQUIRED "external-repos")
+set_tests_properties(ladx           PROPERTIES LABELS "ladx;external;nonfree"
+                                               FIXTURES_REQUIRED "external-repos")
+set_tests_properties(ucity          PROPERTIES LABELS "ucity;external;free"
+                                               FIXTURES_REQUIRED "free-repos")
+set_tests_properties(libbet         PROPERTIES LABELS "libbet;external;free"
+                                               FIXTURES_REQUIRED "free-repos")
+set_tests_properties(sameboy        PROPERTIES LABELS "sameboy;external;free"
+                                               FIXTURES_REQUIRED "free-repos")
+set_tests_properties(gb-starter-kit PROPERTIES LABELS "gb-starter-kit;external;free"
+                                               FIXTURES_REQUIRED "free-repos")
+
+# gb-starter kit fails with any `make` on Windows: https://codeberg.org/ISSOtm/gb-starter-kit/issues/1
+# gb-starter-kit fails with macOS/BSD `make`: https://codeberg.org/ISSOtm/gb-starter-kit/issues/29
+if(TESTS_OS_NAME MATCHES "^(windows|macos|.*bsd)(-.*)?")
+  set_tests_properties(gb-starter-kit PROPERTIES DISABLED TRUE)
+endif()

test/external/gb-starter-kit.cfg

@@ -0,0 +1,7 @@
+EXTERNAL_TEST_OWNER=ISSOtm
+EXTERNAL_TEST_REPO=gb-starter-kit
+EXTERNAL_TEST_DOMAIN=codeberg.org
+EXTERNAL_TEST_COMMIT=74b647d62ff74b40d2b52e585cbebe148463212e
+EXTERNAL_TEST_TARGET=all
+EXTERNAL_TEST_FILE=bin/boilerplate.gb
+EXTERNAL_TEST_HASH=b4f130169ba73284e0d0e71b53e7baa4eca2f7fe

test/external/ladx.cfg

@@ -0,0 +1,7 @@
+EXTERNAL_TEST_OWNER=zladx
+EXTERNAL_TEST_REPO=LADX-Disassembly
+EXTERNAL_TEST_DOMAIN=github.com
+EXTERNAL_TEST_COMMIT=c77af4473e7a877c68e1de34a2aaf80e9076dc35
+EXTERNAL_TEST_TARGET=default
+EXTERNAL_TEST_FILE=azle.gbc
+EXTERNAL_TEST_HASH=d90ac17e9bf17b6c61624ad9f05447bdb5efc01a

test/external/libbet.cfg

@@ -0,0 +1,7 @@
+EXTERNAL_TEST_OWNER=pinobatch
+EXTERNAL_TEST_REPO=libbet
+EXTERNAL_TEST_DOMAIN=github.com
+EXTERNAL_TEST_COMMIT=e42c0036b18e6e715987b88b4973389b283974c9
+EXTERNAL_TEST_TARGET=all
+EXTERNAL_TEST_FILE=libbet.gb
+EXTERNAL_TEST_HASH=f117089aa056600e2d404bbcbac96b016fc64611

test/external/pokecrystal.cfg

@@ -0,0 +1,7 @@
+EXTERNAL_TEST_OWNER=pret
+EXTERNAL_TEST_REPO=pokecrystal
+EXTERNAL_TEST_DOMAIN=github.com
+EXTERNAL_TEST_COMMIT=2bbb15675de0d2bbebc8cc9978f5c7fb15bc73b9
+EXTERNAL_TEST_TARGET=compare
+EXTERNAL_TEST_FILE=pokecrystal.gbc
+EXTERNAL_TEST_HASH=f4cd194bdee0d04ca4eac29e09b8e4e9d818c133

test/external/pokered.cfg

@@ -0,0 +1,7 @@
+EXTERNAL_TEST_OWNER=pret
+EXTERNAL_TEST_REPO=pokered
+EXTERNAL_TEST_DOMAIN=github.com
+EXTERNAL_TEST_COMMIT=0555b42dc0ceffaae613e97cc0cf2e8c0b45013c
+EXTERNAL_TEST_TARGET=compare
+EXTERNAL_TEST_FILE=pokered.gbc
+EXTERNAL_TEST_HASH=ea9bcae617fdf159b045185467ae58b2e4a48b9a

test/external/sameboy.cfg

@@ -0,0 +1,7 @@
+EXTERNAL_TEST_OWNER=LIJI32
+EXTERNAL_TEST_REPO=SameBoy
+EXTERNAL_TEST_DOMAIN=github.com
+EXTERNAL_TEST_COMMIT=2f4a6f231ec40ecfc0ab7df0a09eb932e7ccddec
+EXTERNAL_TEST_TARGET=bootroms
+EXTERNAL_TEST_FILE=build/bin/BootROMs/cgb_boot.bin
+EXTERNAL_TEST_HASH=113903775a9d34b798c2f8076672da6626815a91

test/external/test.sh

@@ -0,0 +1,35 @@
+#!/usr/bin/env bash
+set -euo pipefail
+
+export LC_ALL=C
+
+# Game Boy release date, 1989-04-21T12:34:56Z (for reproducible test results)
+export SOURCE_DATE_EPOCH=609165296
+
+cd "$(dirname "$0")/.."
+
+RGBDS_PATH="RGBDS=../../"
+
+if [ ! -f "external/$1.cfg" ]; then
+	echo >&2 'External test file '"$1"'.cfg does not exist'
+	exit 1
+fi
+
+# Sourcing "external/$1.cfg" defines `EXTERNAL_TEST_*` values used below.
+. "external/$1.cfg"
+
+if ! cd "$EXTERNAL_TEST_REPO"; then
+	echo >&2 'Please fetch test deps before running any external test'
+	exit 1
+fi
+make clean $RGBDS_PATH
+make -j4 "$EXTERNAL_TEST_TARGET" $RGBDS_PATH
+hash="$(sha1sum -b "$EXTERNAL_TEST_FILE" | head -c 40)"
+if [ "$hash" != "$EXTERNAL_TEST_HASH" ]; then
+	cat >&2 <<EOM
+error: "$EXTERNAL_TEST_FILE" checksum did not match!
+    Expected $EXTERNAL_TEST_HASH,
+         got $hash
+EOM
+	exit 1
+fi

test/external/ucity.cfg

@@ -0,0 +1,7 @@
+EXTERNAL_TEST_OWNER=AntonioND
+EXTERNAL_TEST_REPO=ucity
+EXTERNAL_TEST_DOMAIN=github.com
+EXTERNAL_TEST_COMMIT=d1880a2a112d7c26f16c0fc06a15b6c32fdc9137
+EXTERNAL_TEST_TARGET=all
+EXTERNAL_TEST_FILE=ucity.gbc
+EXTERNAL_TEST_HASH=5f026649611c9606ce0bf70dc1552e054e7df5bc

test/fetch-test-deps.sh

@@ -5,20 +5,18 @@ cd "$(dirname "$0")"
 
 usage() {
 	cat <<"EOF"
-Downloads source code of Game Boy programs used as RGBDS test cases.
+Downloads source code of Game Boy project repos used as RGBDS test cases.
 Options:
-    -h, --help          show this help message
+    -h, --help      show this help message
-    --only-free         download only freely licensed codebases
+    --only-free     download only freely licensed codebases
-    --only-internal     do not download any codebases
+    --get-hash      print repos' commit hashes instead of downloading them
-    --get-hash          print programs' commit hashes instead of downloading them
+    --get-paths     print repos' clone paths instead of downloading them
-    --get-paths         print programs' GitHub paths instead of downloading them
 EOF
 }
 
 # Parse options in pure Bash because macOS `getopt` is stuck
 # in what util-linux `getopt` calls `GETOPT_COMPATIBLE` mode
 nonfree=true
-external=true
 actionname=
 osname=
 while [[ $# -gt 0 ]]; do
@@ -30,9 +28,6 @@ while [[ $# -gt 0 ]]; do
 		--only-free)
 			nonfree=false
 			;;
-		--only-internal)
-			external=false
-			;;
 		--get-hash|--get-paths)
 			actionname="$1"
 			;;
@@ -49,43 +44,42 @@ done
 
 case "$actionname" in
 	--get-hash)
-		action() { # _ _ repo commit
+		action() {
-			printf "%s@%s-" "$3" "$4"
+			printf "%s@%s-" "$EXTERNAL_TEST_REPO" "$EXTERNAL_TEST_COMMIT"
 		}
 		;;
 
 	--get-paths)
-		action() { # _ _ repo _
+		action() {
-			printf "test/%s," "$3"
+			printf "test/%s," "$EXTERNAL_TEST_REPO"
 		}
 		;;
 
 	*)
 		echo "Fetching test dependency repositories"
 
-		action() { # domain owner repo commit
+		action() {
-			if [ ! -d "$3" ]; then
+			if [ ! -d "$EXTERNAL_TEST_REPO" ]; then
-				git clone "https://$1/$2/$3.git" --revision="$4" --depth=1 --recursive --shallow-submodules --config advice.detachedHead=false
+				git clone "https://$EXTERNAL_TEST_DOMAIN/$EXTERNAL_TEST_OWNER/$EXTERNAL_TEST_REPO.git" \
+					--revision="$EXTERNAL_TEST_COMMIT" --depth=1 --recursive --shallow-submodules \
+					--config advice.detachedHead=false
 			fi
-			pushd "$3"
+			pushd "$EXTERNAL_TEST_REPO"
-			git checkout --force --detach "$4" --
+			git checkout --force --detach "$EXTERNAL_TEST_COMMIT" --
-			if [ -f "../patches/$3.patch" ]; then
+			if [ -f "../patches/$EXTERNAL_TEST_REPO.patch" ]; then
-				git apply --ignore-whitespace "../patches/$3.patch"
+				git apply --ignore-whitespace "../patches/$EXTERNAL_TEST_REPO.patch"
 			fi
 			popd
 		}
 esac
 
-if ! "$external"; then
+# Sourcing each "external/*.cfg" file defines `EXTERNAL_TEST_*` values used by the `action` functions.
-	exit
-fi
-
 if "$nonfree"; then
-	action github.com pret  pokecrystal      2bbb15675de0d2bbebc8cc9978f5c7fb15bc73b9
+	. external/pokecrystal.cfg && action
-	action github.com pret  pokered          0555b42dc0ceffaae613e97cc0cf2e8c0b45013c
+	. external/pokered.cfg     && action
-	action github.com zladx LADX-Disassembly c77af4473e7a877c68e1de34a2aaf80e9076dc35
+	. external/ladx.cfg        && action
 fi
-action github.com   AntonioND ucity          d1880a2a112d7c26f16c0fc06a15b6c32fdc9137
+. external/ucity.cfg          && action
-action github.com   pinobatch libbet         e42c0036b18e6e715987b88b4973389b283974c9
+. external/libbet.cfg         && action
-action github.com   LIJI32    SameBoy        2f4a6f231ec40ecfc0ab7df0a09eb932e7ccddec
+. external/sameboy.cfg        && action
-action codeberg.org ISSOtm    gb-starter-kit 74b647d62ff74b40d2b52e585cbebe148463212e
+. external/gb-starter-kit.cfg && action

test/run-tests.sh

@@ -39,7 +39,6 @@ while [[ $# -gt 0 ]]; do
 			;;
 		--only-internal)
 			external=false
-			FETCH_TEST_DEPS="$FETCH_TEST_DEPS --only-internal"
 			;;
 		--only-external)
 			internal=false
@@ -109,35 +108,18 @@ fi
 
 # Test some significant external projects that use RGBDS
 # When adding new ones, don't forget to add them to the .gitignore!
-# When updating subprojects, change the commit being checked out, and set the `shallow-since`
-# to the day before, to reduce the amount of refs being transferred and thus speed up CI.
-
-test_downstream() { # owner repo make-target build-file build-hash
-	if ! pushd "$2"; then
-		echo >&2 'Please run `'"$FETCH_TEST_DEPS"'` before running the test suite'
-		return 1
-	fi
-	make clean $RGBDS_PATH
-	make -j4 "$3" $RGBDS_PATH
-	hash="$(sha1sum -b "$4" | head -c 40)"
-	if [ "$hash" != "$5" ]; then
-		echo >&2 'SHA-1 hash of '"$4"' did not match: '"$hash"
-		return 1
-	fi
-	popd
-}
 
 if "$nonfree"; then
-	test_downstream pret  pokecrystal      compare pokecrystal.gbc f4cd194bdee0d04ca4eac29e09b8e4e9d818c133
+	./external/test.sh pokecrystal
-	test_downstream pret  pokered          compare pokered.gbc     ea9bcae617fdf159b045185467ae58b2e4a48b9a
+	./external/test.sh pokered
-	test_downstream zladx LADX-Disassembly default azle.gbc        d90ac17e9bf17b6c61624ad9f05447bdb5efc01a
+	./external/test.sh ladx
 fi
-test_downstream AntonioND ucity   all      ucity.gbc 5f026649611c9606ce0bf70dc1552e054e7df5bc
+./external/test.sh ucity
-test_downstream pinobatch libbet  all      libbet.gb f117089aa056600e2d404bbcbac96b016fc64611
+./external/test.sh libbet
-test_downstream LIJI32    SameBoy bootroms build/bin/BootROMs/cgb_boot.bin 113903775a9d34b798c2f8076672da6626815a91
+./external/test.sh sameboy
 # gb-starter kit fails with any `make` on Windows: https://codeberg.org/ISSOtm/gb-starter-kit/issues/1
 # gb-starter-kit fails with macOS/BSD `make`: https://codeberg.org/ISSOtm/gb-starter-kit/issues/29
 case "${osname%%-*}" in
 	windows | macos | *bsd) ;;
-	*) test_downstream ISSOtm gb-starter-kit all bin/boilerplate.gb b4f130169ba73284e0d0e71b53e7baa4eca2f7fe;;
+	*) ./external/test.sh gb-starter-kit
 esac