diff --git a/AGENTS.md b/AGENTS.md index 7ec460d..8d84629 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -107,25 +107,45 @@ not share is a layer of `#ifdef SPRITESTUDIO_GODOT_EXTENSION` adapters, which are includes and type conversions — so the module build's guard is that it still builds. +**A green run still prints `ERROR:` lines, and they belong to that dummy +rasteriser.** `Condition "!actions.custom_samplers.has(...)" is true. Continuing.` +comes from Godot's shader compiler, once per player. +`RendererDummy::MaterialStorage` validates every shader through a compiler built +with a default-constructed `DefaultIdentifierActions`, so its `custom_samplers` +table is empty; the real canvas renderer fills that table with `TEXTURE` and its +siblings. `ss_blur.fs` passes the builtin `TEXTURE` into `ss_input_texture()`, +which headless therefore cannot resolve a sampler for. `ERR_CONTINUE` is not a +failure — compilation still returns OK — and the same suite under a real +renderer prints none of them. The run's verdict is the RESULT line and the +marker after it, not the absence of `ERROR:` in the log. + Cases step with `advance()` under `ANIMATION_PROCESS_MANUAL`, never the frame clock, so a result does not depend on how long a frame took. A case that cannot run on this host declares a **skip**, which is reported apart from the passes -and never counted as one. - -**The first headless import crashes, and `run-tests.*` retries it once. It is a -godot-cpp problem, not ours.** The run in which Godot first *discovers* the -extension aborts on the way out (null dereference, caught by Godot's own crash -handler). Not the import — a project with **zero importable files** does it too; -what triggers it is the extension being loaded mid-scan rather than at startup -from `.godot/extension_list.cfg`, and deleting just that file brings it back. -**godot-cpp's own `test/` extension reproduces it exactly**, a project with no -extension does not, and registering nothing at all still does — so it is the -pairing, not this code. godot-cpp has no 4.6/4.7 release branch: it went from -`godot-4.5-stable` straight to the 10.0 line, so an extension for Godot 4.7 is -built from master against `api_version=4.7`. The scan's work completes, so the -second run is clean; the retry requires that second run to pass, because a crash -that repeats is still a failure. Not test-only — anything running -`godot --headless --import` on a fresh checkout meets it. +and never counted as one. A case that recorded nothing at all — no assertion, no +skip, no failure — is counted as a **failure**: GDScript answers a bad call by +abandoning that one function and carrying straight on, so without that check a +case that never ran would be indistinguishable from one that passed. + +**A first headless import that discovers the extension mid-scan crashes on the +way out, so `run-tests.*` names the extension in `.godot/extension_list.cfg` +before it starts Godot. The bug is Godot's — not this code, and not +godot-cpp's.** `EditorHelp::_gen_extensions_docs()` dereferences the static +`DocTools` without a null check. Loading an extension mid-scan emits +`GDExtensionManager::extensions_reloaded`, which sends `EditorNode` off to +regenerate the class reference on a worker thread; that thread's last act is to +queue `_gen_extensions_docs` as a deferred call. `--import` quits before the +message queue is flushed, so the call lands on the flush at the end of +`Main::cleanup()` — by which time `EditorHelp::cleanup_doc()` has freed the +`DocTools` and set it to null. It is not the import: a project with **zero +importable files** does it too, and godot-cpp's own `test/` extension reproduces +it. Naming the extension up front means it is loaded at startup instead, so +`extensions_reloaded` never fires and nothing is ever queued; Godot rewrites the +file itself, so seeding it decides only the run in which it did not exist yet. +The engine-side fix is a null check in that one function, so a future Godot may +make the seeding redundant — it stays either way, being what makes the first run +deterministic. Not test-only: anything running `godot --headless --import` over +a project whose `.godot/` has never seen the extension meets it. ## Releases diff --git a/scripts/run-tests.ps1 b/scripts/run-tests.ps1 index 72689f6..9b43104 100644 --- a/scripts/run-tests.ps1 +++ b/scripts/run-tests.ps1 @@ -91,33 +91,38 @@ Write-Host "run-tests.ps1: $(& $GodotBin --headless --version | Select-Object -L # --- import pass ---------------------------------------------------------- # A project Godot has never opened has no .godot\, and the textures beside each # .ssab are not importable until it does. Cheap after the first run. -# It is run twice on purpose, and the SECOND run is the one that has to pass. # -# The run in which Godot first DISCOVERS the extension aborts on the way out -# (null dereference, caught by Godot's own crash handler). Not the import: a -# project with zero importable files does it too. What triggers it is the -# extension being loaded mid-scan rather than at startup from -# .godot/extension_list.cfg -- delete just that file and it happens again. +# The extension is named in .godot\extension_list.cfg BEFORE Godot is started, +# because the run in which Godot discovers an extension mid-scan crashes on the +# way out. That crash is Godot's own, and it is not the import -- a project with +# zero importable files does it too: # -# It is NOT this repository's code. godot-cpp's own test extension, with none of -# our sources and a different descriptor, reproduces it exactly; a project with -# no extension does not; and registering nothing at all still does. godot-cpp -# has no 4.6/4.7 release branch (it went from godot-4.5-stable straight to the -# 10.0 line), so an extension for Godot 4.7 is built from master against -# api_version=4.7, and that is the combination that does this. +# EditorHelp::_gen_extensions_docs() dereferences the static DocTools without +# a null check. Loading an extension mid-scan emits extensions_reloaded, which +# sends EditorNode off to regenerate the class reference on a worker thread; +# that thread's last act is to queue _gen_extensions_docs as a deferred call. +# --import then quits before the message queue is flushed, so the call lands +# on the flush at the end of Main::cleanup() -- by which time +# EditorHelp::cleanup_doc() has freed the DocTools and set it to null. # -# The scan's work completes -- the second run exits 0 with nothing left to do. -# So this is a retry, not a tolerance. A crash that repeats is still a failure -# here, and the clean second run is the evidence that the import finished -- -# nothing is being waved through on the strength of the first one. +# Naming the extension up front means it is loaded at startup instead, so +# extensions_reloaded never fires and nothing is ever queued. Godot rewrites +# this file itself, so seeding it decides only the run in which it did not exist +# yet. There is no retry here: with the file seeded the import either works or +# has failed for some other reason, and a crash is a crash. if ($DoImport -eq "yes") { + $extList = Join-Path $Project ".godot\extension_list.cfg" + if (-not (Test-Path $extList)) { + $descriptors = Get-ChildItem -Path (Join-Path $Project "addons\spritestudio") -Filter "*.gdextension" -ErrorAction SilentlyContinue + foreach ($desc in $descriptors) { + New-Item -ItemType Directory -Force -Path (Join-Path $Project ".godot") | Out-Null + $rel = $desc.FullName.Substring($Project.Length).TrimStart('\', '/').Replace('\', '/') + Add-Content -Path $extList -Value "res://$rel" + } + } + $importLog = & $GodotBin --headless --path $Project --import 2>&1 $importStatus = $LASTEXITCODE - if ($importStatus -ne 0) { - Write-Host "run-tests.ps1: the first import exited $importStatus (godot-cpp's known first-scan crash); retrying." - $importLog = & $GodotBin --headless --path $Project --import 2>&1 - $importStatus = $LASTEXITCODE - } $importErrors = $importLog | Select-String -Pattern '^ERROR:' if ($importStatus -ne 0 -or $importErrors) { Write-Host "run-tests.ps1: the import pass failed." @@ -127,10 +132,13 @@ if ($DoImport -eq "yes") { } # --- run ------------------------------------------------------------------ -# --quit-after is a hang guard, not a schedule: a script error inside a case -# aborts run_tests.gd's _init and leaves the tree idling forever. It costs the -# exit code its meaning on that path -- Godot leaves 0 on the way out -- which -# is why the marker below, and not the status, is what says a run completed. +# --quit-after is a hang guard, not a schedule: an error run_tests.gd's _init +# cannot walk away from -- a suite that will not parse -- leaves the tree idling +# forever. It costs the exit code its meaning on that path -- Godot leaves 0 on +# the way out -- which is why the marker below, and not the status, is what says +# a run completed. A script error inside one case is not that: GDScript +# abandons the case and carries on, and run_tests.gd fails it for recording +# nothing. $runArgs = @("--headless", "--path", $Project, "--quit-after", "100000", "--script", "res://run_tests.gd") if ($Only) { $runArgs += @("--", "--only=$Only") } diff --git a/scripts/run-tests.sh b/scripts/run-tests.sh index b5a0a7c..30cdc68 100755 --- a/scripts/run-tests.sh +++ b/scripts/run-tests.sh @@ -83,35 +83,39 @@ echo "$APP: $("$GODOT_BIN" --headless --version 2>/dev/null | tail -n 1) at ${GO # --- import pass ---------------------------------------------------------- # A project Godot has never opened has no .godot/, and the textures beside each # .ssab are not importable until it does. Cheap after the first run. -# It is run twice on purpose, and the SECOND run is the one that has to pass. # -# The run in which Godot first DISCOVERS the extension aborts on the way out -# (null dereference, caught by Godot's own crash handler). Not the import: a -# project with zero importable files does it too. What triggers it is the -# extension being loaded mid-scan rather than at startup from -# .godot/extension_list.cfg -- delete just that file and it happens again. +# The extension is named in .godot/extension_list.cfg BEFORE Godot is started, +# because the run in which Godot discovers an extension mid-scan crashes on the +# way out. That crash is Godot's own, and it is not the import -- a project with +# zero importable files does it too: # -# It is NOT this repository's code. godot-cpp's own test extension, with none of -# our sources and a different descriptor, reproduces it exactly; a project with -# no extension does not; and registering nothing at all still does. godot-cpp -# has no 4.6/4.7 release branch (it went from godot-4.5-stable straight to the -# 10.0 line), so an extension for Godot 4.7 is built from master against -# api_version=4.7, and that is the combination that does this. +# EditorHelp::_gen_extensions_docs() dereferences the static DocTools without +# a null check. Loading an extension mid-scan emits extensions_reloaded, which +# sends EditorNode off to regenerate the class reference on a worker thread; +# that thread's last act is to queue _gen_extensions_docs as a deferred call. +# --import then quits before the message queue is flushed, so the call lands +# on the flush at the end of Main::cleanup() -- by which time +# EditorHelp::cleanup_doc() has freed the DocTools and set it to null. # -# The scan's work completes -- the second run exits 0 with nothing left to do. -# So this is a retry, not a tolerance. A crash that repeats is still a failure -# here, and the clean second run is the evidence that the import finished -- -# nothing is being waved through on the strength of the first one. +# Naming the extension up front means it is loaded at startup instead, so +# extensions_reloaded never fires and nothing is ever queued. Godot rewrites +# this file itself, so seeding it decides only the run in which it did not exist +# yet. There is no retry here: with the file seeded the import either works or +# has failed for some other reason, and a crash is a crash. if [ "$DO_IMPORT" = "yes" ]; then + EXT_LIST="${PROJECT}/.godot/extension_list.cfg" + if [ ! -f "$EXT_LIST" ]; then + for desc in "${PROJECT}"/addons/spritestudio/*.gdextension; do + [ -f "$desc" ] || continue + mkdir -p "${PROJECT}/.godot" + echo "res://${desc#"${PROJECT}/"}" >>"$EXT_LIST" + done + fi + IMPORT_LOG=$(mktemp) set +e "$GODOT_BIN" --headless --path "$PROJECT" --import >"$IMPORT_LOG" 2>&1 IMPORT_STATUS=$? - if [ "$IMPORT_STATUS" -ne 0 ]; then - echo "$APP: the first import exited $IMPORT_STATUS (godot-cpp's known first-scan crash); retrying." - "$GODOT_BIN" --headless --path "$PROJECT" --import >"$IMPORT_LOG" 2>&1 - IMPORT_STATUS=$? - fi set -e if [ "$IMPORT_STATUS" -ne 0 ] || grep -q '^ERROR:' "$IMPORT_LOG"; then echo "$APP: the import pass failed." >&2 @@ -123,10 +127,13 @@ if [ "$DO_IMPORT" = "yes" ]; then fi # --- run ------------------------------------------------------------------ -# --quit-after is a hang guard, not a schedule: a script error inside a case -# aborts run_tests.gd's _init and leaves the tree idling forever. It costs the -# exit code its meaning on that path -- Godot leaves 0 on the way out -- which -# is why the marker below, and not the status, is what says a run completed. +# --quit-after is a hang guard, not a schedule: an error run_tests.gd's _init +# cannot walk away from -- a suite that will not parse -- leaves the tree idling +# forever. It costs the exit code its meaning on that path -- Godot leaves 0 on +# the way out -- which is why the marker below, and not the status, is what says +# a run completed. A script error inside one case is not that: GDScript +# abandons the case and carries on, and run_tests.gd fails it for recording +# nothing. ARGS=(--headless --path "$PROJECT" --quit-after 100000 --script res://run_tests.gd) [ -n "$ONLY" ] && ARGS+=(-- "--only=${ONLY}") @@ -151,8 +158,9 @@ EOF fi # run_tests.gd prints this last. Without it the run stopped in the middle -- a -# parse error in a suite, or a script error inside a case, either of which -# leaves Godot to be shut down by --quit-after with a status of 0. +# suite that will not parse, say -- and Godot was shut down by --quit-after with +# a status of 0. A script error inside one case does not stop the run; the +# runner catches that itself, by failing a case that recorded nothing. if ! echo "$OUTPUT" | grep -q "==== SUITE FINISHED ===="; then echo "" >&2 echo "$APP: the run stopped before the suite finished — see above." >&2 diff --git a/test_gdextension/run_tests.gd b/test_gdextension/run_tests.gd index a686603..9dc4893 100644 --- a/test_gdextension/run_tests.gd +++ b/test_gdextension/run_tests.gd @@ -75,11 +75,17 @@ func _init() -> void: var before_failures: int = suite.failures.size() for case in cases: suite.begin_case(case) + var mark := [suite.assertions, suite.skips.size(), suite.failures.size()] suite.setup() suite.call(case) suite.teardown() suite.release_owned() total_cases += 1 + # A case that recorded nothing at all did not run: GDScript abandons a + # function on a bad call without stopping the run, and the case would + # otherwise be indistinguishable from one that passed. + if mark == [suite.assertions, suite.skips.size(), suite.failures.size()]: + suite.record_empty_case() total_assertions += suite.assertions failures.append_array(suite.failures) @@ -110,11 +116,15 @@ func _init() -> void: ## Prints the marker `run-tests.*` looks for, then exits. ## -## The exit code alone is not enough to trust. A script error inside a case -## aborts `_init` before anything below it runs, and the tree then idles forever -## — so the wrapper passes `--quit-after`, which makes Godot exit 0 on the way -## out and turns a crashed run into a green one. The marker is what tells a run -## that finished apart from one that stopped in the middle. +## The exit code alone is not enough to trust. An error `_init` cannot walk away +## from — a suite that will not parse, above all — leaves the tree idling +## forever, so the wrapper passes `--quit-after`, which makes Godot exit 0 on the +## way out and turns a stopped run into a green one. The marker is what tells a +## run that finished apart from one that stopped in the middle. +## +## It says nothing about a single case. GDScript abandons a case's function on a +## bad call and carries straight on to the next one, which is what the +## `record_empty_case` check above is for. func _finish(code: int) -> void: print("==== SUITE FINISHED ====") quit(code) @@ -174,5 +184,10 @@ func _cases_of(suite) -> Array[String]: func _only_filter() -> Array[String]: for arg in OS.get_cmdline_user_args(): if arg.begins_with("--only="): - return Array(arg.trim_prefix("--only=").split(",", false)) + # assign(), not a cast: split() hands back a PackedStringArray, and + # returning it -- or an untyped Array around it -- from an Array[String] + # is a runtime error that takes _init down with it. + var names: Array[String] = [] + names.assign(arg.trim_prefix("--only=").split(",", false)) + return names return [] diff --git a/test_gdextension/suites/test_overrides.gd b/test_gdextension/suites/test_overrides.gd index da9daf5..c0b4eff 100644 --- a/test_gdextension/suites/test_overrides.gd +++ b/test_gdextension/suites/test_overrides.gd @@ -93,7 +93,8 @@ func test_an_unknown_part_is_refused_rather_than_ignored() -> void: ## is a rendering question and out of scope here; that the call reports success ## for a real part and failure for an imaginary one is not. func test_colour_and_cell_overrides_are_accepted_for_a_real_part() -> void: - ok(player.set_part_color_override(part, Color(1, 0, 0, 1), 0, 1.0, 0), + # The two zeroes are COLOR_BLEND_MIX and OVERRIDE_PRIORITY_OVERWRITE_ON_NEXT_KEYFRAME. + ok(player.set_part_color_override(part, Color(1, 0, 0, 1), 0, 0), "a colour override on '%s'" % part) player.advance(dt) ok(player.clear_part_color_override(part), "clearing it") diff --git a/test_gdextension/test_base.gd b/test_gdextension/test_base.gd index 42e689d..2253f7f 100644 --- a/test_gdextension/test_base.gd +++ b/test_gdextension/test_base.gd @@ -134,3 +134,15 @@ func has(collection, value, what: String) -> bool: ## Declares this case unrunnable here, with the reason. Not a pass. func skip(reason: String) -> void: skips.append("%s: %s" % [_case, reason]) + + +## Marks the case just run as a defect because it asserted nothing. +## +## Called by `run_tests.gd` between cases, never by a case itself. GDScript +## answers a bad call -- the wrong argument count, a method that is not there -- +## by printing SCRIPT ERROR and abandoning that one function, not by stopping +## the run. The case then leaves no assertion, no skip and no failure behind, +## and a suite whose failure count did not grow reads as a pass. A case that +## asserted nothing was not run, so it is counted here as a failure. +func record_empty_case() -> void: + _record_failure("asserted nothing -- a SCRIPT ERROR above will say why")