MagicBeanDev/Longshot

Watch

Fork 71

test(vk): restore the framegen resource and module identifier checks #350

Merged

HumanBean merged 2 commits from mooki/Longshot:fix/main-test-fixtures-after-252 into main

2026-11-14 15:55:41 +00:00

mooki commented

2026-11-14 03:54:02 +00:00

Member

Why

What this is for: Restore two game-free regression checks broken when #252 changed the source they extract. What is measured: Both fail on the named main and pass here with Clang and GCC; deliberately broken source copies still fail. Not measured: Anything in the game, actual Vulkan driver behavior, performance or native Windows execution.

This PR changes no product code. The separate menu-key failure is already repaired by #338, so it is deliberately left out; the pad-layout failure remains with #331.

What changed

tests/test_module_ids.py: identify complete_from by bool complete_from( instead of its entire old parameter list. The existing brace walk still limits the check to that function, which must contain module_id_learned(. The added bool* busy = nullptr parameter no longer stops the test before its assertions run.
tests/framegen_resources_test.cpp, used by tests/test_framegen_resources.py: supply a non-locking IntelDriverGate and forwarding stand-ins for extern_guarded_shader_module and extern_guarded_compute. They model the path outside Intel on Windows and call the existing failure-injecting Vulkan stand-ins, including the pipeline handle returned with an allocation error.
No assertion, failure-injection point, compiler warning flag, production implementation or public interface changes. The framegen Python driver and the menu-key/pad-layout fixtures are unchanged.

Flags and settings

None. No runtime switch, menu row, saved option or default changes.

Measurements

main: cf605c85bb (2026-11-14)
this PR: fix/main-test-fixtures-after-252 @ 3c797f8ae7

Branch state: on main cf605c85bbaeaaa78176b933b3bc1a3366e13a78; head 3c797f8ae7a747b2f3d79b2fe698a9b20e6306c5; whole game-free suite on this head: 542 passed, 1 failed, 16 skipped (main cf605c85bbaeaaa78176b933b3bc1a3366e13a78: 540 passed, 3 failed, 16 skipped). The 1 failing test fails on main the same way: tests/test_pad_layout.py. The commits are unchanged by rebases.

Hardware: Intel Core i9-13900K, MemTotal 98543612 KiB, Arch Linux with kernel 7.2.15-1-omarchy. An RTX 4090 with driver 615.78.08 is installed but not used by these tests. Clang 23.1.3, GCC 16.2.1, Python 3.14.8, Vulkan-Headers v1.4.345.

Scene: none; extracted source and compiled stand-ins, no game or GPU device.

Method: two rounds in turn (main, this PR, main, this PR) of each changed test with each compiler, one runner worker, behind a shared measurement lock with core dumps suppressed, on 2026-11-13 between 20:57 and 21:53 local (UTC-06); the one-minute load ranged from about 7 to 14. Results are exit status and assertion output, the same in both rounds; they are not speed measurements, so no timing median applies. Separate scratch copies test the two failures and another added default parameter.

How to reproduce: with RX_CPM pointing to a CPM cache containing the Vulkan headers, run these in each checkout:

CXX=clang++ python3 -I -B tests/run_all.py --jobs 1 tests/test_module_ids.py tests/test_framegen_resources.py
CXX=g++ python3 -I -B tests/run_all.py --jobs 1 tests/test_module_ids.py tests/test_framegen_resources.py

Check	main @ `cf605c85b`	this PR @ `3c797f8a`
Module identifiers, clang++	FAIL, exit 1 in both rounds: old signature not found	PASS, exit 0 in both rounds
Module identifiers, g++	FAIL, exit 1 in both rounds: old signature not found	PASS, exit 0 in both rounds
Framegen resources, clang++	FAIL, exit 1 in both rounds: three missing declarations	PASS, exit 0 in both rounds
Framegen resources, g++	FAIL, exit 1 in both rounds: three missing declarations	PASS, exit 0 in both rounds
Menu keys, clang++	PASS, exit 0	Unchanged; not rerun, repair belongs to #338
Menu keys, g++	FAIL, exit 1: C assertion spelling in C++	Unchanged; not rerun, repair belongs to #338

The menu rows disclose the original requested check; they are not an after-pass claim for this branch.

main, module_ids (both compiler selections):
ValueError: substring not found

main, framegen_resources (Clang):
error: unknown type name 'IntelDriverGate'
error: use of undeclared identifier 'extern_guarded_shader_module'
error: use of undeclared identifier 'extern_guarded_compute'

main, framegen_resources (GCC):
error: ‘IntelDriverGate’ was not declared in this scope
error: ‘extern_guarded_shader_module’ was not declared in this scope
error: ‘extern_guarded_compute’ was not declared in this scope

main, menu_keys (GCC, unchanged here):
rx_nvn.h:83:15: error: expected constructor, destructor, or type conversion before ‘(’ token

this PR, module_ids (each compiler):
1 passed, 0 failed, 0 skipped in 0 s (linux, 1 at once)

this PR, framegen_resources (each compiler):
1 passed, 0 failed, 0 skipped in 1 s (linux, 1 at once)

The tests still detect the behavior they protect:

Scratch-source control	clang++	g++
Remove learning from `complete_from`, keeping a `module_id_learned(` call in another function	FAIL, intended assertion	FAIL, intended assertion
Remove `drop_setup();` from framegen `init()`'s failure lambda	Compiles, then FAIL on leaked resources	Compiles, then FAIL on leaked resources
Add another default parameter, `int = 0`, to `complete_from`, without changing its body	PASS	PASS

module learning moved outside complete_from:
a compute module made from its code is not learned by its identifier

framegen rollback removed:
FAIL line 366: live.empty()

The parameter control checks extraction and runs the ordinary module-store fixture; it does not compile the whole renderer.

Testing

Tested:

Both repaired checks fail on the pinned main and pass on this head with both compilers, in two rounds run in turn: eight successful final-head invocations, no skipped prerequisites.
Both behavioral mutants fail with both compiler selections; the extra-parameter control passes with both.
Both changed fixtures compile for Windows x86-64 with llvm-mingw 20261104; they were not run there. One thing a stricter reading would flag, and which this pull request does not change: the framegen fixture's existing _CRT_SECURE_NO_WARNINGS definition sits inside #ifdef _WIN32 rather than unconditionally before its first include, while it uses getenv. The compiler accepts it.
Whole-suite result: 542 passed, 1 failed, 16 skipped on this head; main cf605c85b: 540 passed, 3 failed, 16 skipped. The two repaired checks pass now; what still fails here fails on main the same way: tests/test_pad_layout.py.

Not tested:

Native Windows execution of tests/test_module_ids.py and tests/test_framegen_resources.py; cross-target compilation is not execution with the project's Windows compiler/library environment.
Intel's Windows gate, caught driver faults, real Vulkan resource allocation or cleanup, the game, image quality, frame time or frame rate. None of those product paths changes here.
The excluded menu-key and pad-layout repairs are not tested on this branch.

Needs other testers: None needed to merge: these are game-free fixtures and no product code changes. Running the two repaired tests natively on Windows was not done and is named under Not tested.

Screenshots

Nothing on screen.

Risk and rollback

Risk is confined to test coverage and compiler portability. A marker that searched the whole file could accept learning in the wrong function; the scratch control proves this still fails. A guarded-call stand-in that bypassed failure injection could hide a resource leak; the wrappers retain the existing entry points, and removing rollback still fails after compilation.

The gate stand-in does not test Windows Intel serialization or fault recovery. Reverting these two commits restores the previous fixtures and their main failures; no game state or setting changes.

Reviewer notes

The diff is two test-only commits. The module marker keeps the existing extraction algorithm. The framegen declarations follow the neighboring program-cache fixture's forwarding pattern.

Overlaps

Open PRs were searched before editing and again before handover, including all 117 open PR titles/bodies and the matching candidates' changed files.

#338, c734325a, already keeps the C header's assertions active with their C++ spelling in test_menu_keys.py. That file is left untouched here.
#331, bc375c20, supplies hold_toggle.h in test_pad_layout.py; #338 also carries that include. Pad layout is outside this PR.
#33, 03fff16c, touches framegen_resources_test.cpp only to add BUILTIN_SPV_ARGS beside the shader constants. Its inspected hunk is separate and does not supply these guarded-call declarations.
#66 also adapts the excluded menu fixture as part of its Walk feature. No duplicate menu change is included here.

Questions for reviewers: none about the implementation; what is not covered is named above.

Issues

See #252, whose changed source exposed the two fixture dependencies. The separate #137 fixture failures remain with #331 and #338; this PR does not claim to fix them.

Checklist

For everyone

One topic.
The title is a Conventional Commit (type(scope): description).
Rebased on current main.
python -I -B tests/run_all.py run and passing (any failure that also fails on main is named above).
Tested in the game, or what wasn't is named under Testing.
Testing filled in: what was tested, what wasn't, and what needs other testers (with test requests).
Measured: main and this PR at named commits, on the same PC, scene and settings, at least 2 interleaved runs each, the median reported. Two rounds in turn per compiler; the results are pass or fail and were the same in both rounds, so there is no timing median to report.
No game files, dumps, build output or generated files.
third_party/ untouched (a new folder with a PROVENANCE.md instead).
The license is OK: GPL-3.0-or-later, nothing incompatible and nothing decompiled.

If applicable

AI assistance recorded with Assisted-by: / Co-Authored-By: trailers.
A check under tests/ that reads changed source updated in this PR, not weakened.

Assisted-by: GPT-6 Astra
Co-Authored-By: GPT-6 Astra [email protected]

New on this forge; not opened on GitHub.

## Why **What this is for:** Restore two game-free regression checks broken when [#252](https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/252) changed the source they extract. **What is measured:** Both fail on the named main and pass here with Clang and GCC; deliberately broken source copies still fail. **Not measured:** Anything in the game, actual Vulkan driver behavior, performance or native Windows execution. **This PR changes no product code.** The separate menu-key failure is already repaired by [#338](https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/338), so it is deliberately left out; the pad-layout failure remains with [#331](https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/331). ## What changed - `tests/test_module_ids.py`: identify `complete_from` by `bool complete_from(` instead of its entire old parameter list. The existing brace walk still limits the check to that function, which must contain `module_id_learned(`. The added `bool* busy = nullptr` parameter no longer stops the test before its assertions run. - `tests/framegen_resources_test.cpp`, used by `tests/test_framegen_resources.py`: supply a non-locking `IntelDriverGate` and forwarding stand-ins for `extern_guarded_shader_module` and `extern_guarded_compute`. They model the path outside Intel on Windows and call the existing failure-injecting Vulkan stand-ins, including the pipeline handle returned with an allocation error. - No assertion, failure-injection point, compiler warning flag, production implementation or public interface changes. The framegen Python driver and the menu-key/pad-layout fixtures are unchanged. ## Flags and settings None. No runtime switch, menu row, saved option or default changes. ## Measurements main: cf605c85bbaeaaa78176b933b3bc1a3366e13a78 (2026-11-14) this PR: fix/main-test-fixtures-after-252 @ 3c797f8ae7a747b2f3d79b2fe698a9b20e6306c5  Branch state: on main `cf605c85bbaeaaa78176b933b3bc1a3366e13a78`; head `3c797f8ae7a747b2f3d79b2fe698a9b20e6306c5`; whole game-free suite on this head: `542 passed, 1 failed, 16 skipped` (main `cf605c85bbaeaaa78176b933b3bc1a3366e13a78`: `540 passed, 3 failed, 16 skipped`). The 1 failing test fails on main the same way: `tests/test_pad_layout.py`. The commits are unchanged by rebases.  Hardware: Intel Core i9-13900K, MemTotal 98543612 KiB, Arch Linux with kernel 7.2.15-1-omarchy. An RTX 4090 with driver 615.78.08 is installed but not used by these tests. Clang 23.1.3, GCC 16.2.1, Python 3.14.8, Vulkan-Headers v1.4.345. Scene: none; extracted source and compiled stand-ins, no game or GPU device. Method: two rounds in turn (main, this PR, main, this PR) of each changed test with each compiler, one runner worker, behind a shared measurement lock with core dumps suppressed, on 2026-11-13 between 20:57 and 21:53 local (UTC-06); the one-minute load ranged from about 7 to 14. Results are exit status and assertion output, the same in both rounds; they are not speed measurements, so no timing median applies. Separate scratch copies test the two failures and another added default parameter. How to reproduce: with `RX_CPM` pointing to a CPM cache containing the Vulkan headers, run these in each checkout: ```sh CXX=clang++ python3 -I -B tests/run_all.py --jobs 1 tests/test_module_ids.py tests/test_framegen_resources.py CXX=g++ python3 -I -B tests/run_all.py --jobs 1 tests/test_module_ids.py tests/test_framegen_resources.py ``` | Check | main @ cf605c85b | this PR @ 3c797f8a | |---|---|---| | Module identifiers, clang++ | FAIL, exit 1 in both rounds: old signature not found | PASS, exit 0 in both rounds | | Module identifiers, g++ | FAIL, exit 1 in both rounds: old signature not found | PASS, exit 0 in both rounds | | Framegen resources, clang++ | FAIL, exit 1 in both rounds: three missing declarations | PASS, exit 0 in both rounds | | Framegen resources, g++ | FAIL, exit 1 in both rounds: three missing declarations | PASS, exit 0 in both rounds | | Menu keys, clang++ | PASS, exit 0 | Unchanged; not rerun, repair belongs to #338 | | Menu keys, g++ | FAIL, exit 1: C assertion spelling in C++ | Unchanged; not rerun, repair belongs to #338 | The menu rows disclose the original requested check; they are not an after-pass claim for this branch. ```text main, module_ids (both compiler selections): ValueError: substring not found main, framegen_resources (Clang): error: unknown type name 'IntelDriverGate' error: use of undeclared identifier 'extern_guarded_shader_module' error: use of undeclared identifier 'extern_guarded_compute' main, framegen_resources (GCC): error: ‘IntelDriverGate’ was not declared in this scope error: ‘extern_guarded_shader_module’ was not declared in this scope error: ‘extern_guarded_compute’ was not declared in this scope main, menu_keys (GCC, unchanged here): rx_nvn.h:83:15: error: expected constructor, destructor, or type conversion before ‘(’ token this PR, module_ids (each compiler): 1 passed, 0 failed, 0 skipped in 0 s (linux, 1 at once) this PR, framegen_resources (each compiler): 1 passed, 0 failed, 0 skipped in 1 s (linux, 1 at once) ``` The tests still detect the behavior they protect: | Scratch-source control | clang++ | g++ | |---|---|---| | Remove learning from `complete_from`, keeping a `module_id_learned(` call in another function | FAIL, intended assertion | FAIL, intended assertion | | Remove `drop_setup();` from framegen `init()`'s failure lambda | Compiles, then FAIL on leaked resources | Compiles, then FAIL on leaked resources | | Add another default parameter, `int = 0`, to `complete_from`, without changing its body | PASS | PASS | ```text module learning moved outside complete_from: a compute module made from its code is not learned by its identifier framegen rollback removed: FAIL line 366: live.empty() ``` The parameter control checks extraction and runs the ordinary module-store fixture; it does not compile the whole renderer. ## Testing **Tested:** - Both repaired checks fail on the pinned main and pass on this head with both compilers, in two rounds run in turn: eight successful final-head invocations, no skipped prerequisites. - Both behavioral mutants fail with both compiler selections; the extra-parameter control passes with both. - Both changed fixtures compile for Windows x86-64 with llvm-mingw 20261104; they were not run there. One thing a stricter reading would flag, and which this pull request does not change: the framegen fixture's existing `_CRT_SECURE_NO_WARNINGS` definition sits inside `#ifdef _WIN32` rather than unconditionally before its first include, while it uses `getenv`. The compiler accepts it. - Whole-suite result: `542 passed, 1 failed, 16 skipped` on this head; main `cf605c85b`: `540 passed, 3 failed, 16 skipped`. The two repaired checks pass now; what still fails here fails on main the same way: `tests/test_pad_layout.py`. **Not tested:** - Native Windows execution of `tests/test_module_ids.py` and `tests/test_framegen_resources.py`; cross-target compilation is not execution with the project's Windows compiler/library environment. - Intel's Windows gate, caught driver faults, real Vulkan resource allocation or cleanup, the game, image quality, frame time or frame rate. None of those product paths changes here. - The excluded menu-key and pad-layout repairs are not tested on this branch. **Needs other testers:** None needed to merge: these are game-free fixtures and no product code changes. Running the two repaired tests natively on Windows was not done and is named under Not tested. ## Screenshots Nothing on screen. ## Risk and rollback Risk is confined to test coverage and compiler portability. A marker that searched the whole file could accept learning in the wrong function; the scratch control proves this still fails. A guarded-call stand-in that bypassed failure injection could hide a resource leak; the wrappers retain the existing entry points, and removing rollback still fails after compilation. The gate stand-in does not test Windows Intel serialization or fault recovery. Reverting these two commits restores the previous fixtures and their main failures; no game state or setting changes. ## Reviewer notes The diff is two test-only commits. The module marker keeps the existing extraction algorithm. The framegen declarations follow the neighboring program-cache fixture's forwarding pattern. ### Overlaps Open PRs were searched before editing and again before handover, including all 117 open PR titles/bodies and the matching candidates' changed files. - [#338](https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/338), `c734325a`, already keeps the C header's assertions active with their C++ spelling in `test_menu_keys.py`. That file is left untouched here. - [#331](https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/331), `bc375c20`, supplies `hold_toggle.h` in `test_pad_layout.py`; #338 also carries that include. Pad layout is outside this PR. - [#33](https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/33), `03fff16c`, touches `framegen_resources_test.cpp` only to add `BUILTIN_SPV_ARGS` beside the shader constants. Its inspected hunk is separate and does not supply these guarded-call declarations. - [#66](https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/66) also adapts the excluded menu fixture as part of its Walk feature. No duplicate menu change is included here. Questions for reviewers: none about the implementation; what is not covered is named above. ## Issues See [#252](https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/252), whose changed source exposed the two fixture dependencies. The separate [#137](https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/137) fixture failures remain with #331 and #338; this PR does not claim to fix them. ## Checklist ### For everyone - [x] One topic. - [x] The title is a Conventional Commit (`type(scope): description`). - [x] Rebased on current `main`. - [x] `python -I -B tests/run_all.py` run and passing (any failure that also fails on `main` is named above). - [x] Tested in the game, or what wasn't is named under Testing. - [x] Testing filled in: what was tested, what wasn't, and what needs other testers (with test requests). - [x] Measured: main and this PR at named commits, on the same PC, scene and settings, at least 2 interleaved runs each, the median reported. Two rounds in turn per compiler; the results are pass or fail and were the same in both rounds, so there is no timing median to report. - [x] No game files, dumps, build output or generated files. - [x] `third_party/` untouched (a new folder with a `PROVENANCE.md` instead). - [x] The license is OK: GPL-3.0-or-later, nothing incompatible and nothing decompiled. ### If applicable - [x] AI assistance recorded with `Assisted-by:` / `Co-Authored-By:` trailers. - [x] A check under `tests/` that reads changed source updated in this PR, not weakened. Assisted-by: GPT-6 Astra Co-Authored-By: GPT-6 Astra <[email protected]> --- New on this forge; not opened on GitHub.

mooki added 2 commits

2026-11-14 03:54:02 +00:00

test(vk): keep the module identifier check independent of default parameters 2f118d6cc2

Match complete_from by its return type and function name, then keep checking only its brace-delimited body for module_id_learned. The new busy parameter does not change what this check is meant to enforce.

Assisted-by: GPT-6 Astra

Co-Authored-By: GPT-6 Astra <[email protected]>

test(vk): supply guarded allocation calls in the framegen fixture

CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Has been skipped

Details

CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Has been skipped

Details

CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Has been skipped

Details

Third-party parts / The same as upstream (pull_request) Has been skipped

Details

CI / Conventional Commit title (pull_request) Successful in 3s

Details

CI / Workflow lint (actionlint) (pull_request) Successful in 8s

Details

Third-party parts / As their notes say (pull_request) Successful in 10s

Details

PR description / Description follows the template (pull_request_target) Successful in 5s

Details

Website / Website on Cloudflare (pull_request_target) Has been skipped

Details

3c797f8ae7

Model the ungated non-Windows-Intel path and forward both guarded calls to the existing Vulkan stand-ins. Keep their allocation failure injection, returned handles and cleanup assertions unchanged.

Assisted-by: GPT-6 Astra

Co-Authored-By: GPT-6 Astra <[email protected]>

mooki changed title from ~~WIP: test(vk): restore the framegen resource and module identifier checks~~ to test(vk): restore the framegen resource and module identifier checks

2026-11-14 03:54:03 +00:00

mooki added the

area/renderer

kind

chore

labels

2026-11-14 03:54:30 +00:00

mooki added the

priority

low

label

2026-11-14 04:18:42 +00:00

bibiburl approved these changes

2026-11-14 07:18:02 +00:00

bibiburl left a comment

Reviewed 3c797f8ae7 against current main 0caf06184b. No blocking finding; these restore the existing checks without weakening their assertions.

Independently reproduced main's stale complete_from signature failure and the framegen fixture's missing IntelDriverGate/guarded-call declarations. On the PR combined locally with current main, module-ID and framegen-resource checks pass with both GCC and Clang. Used repository-pinned Vulkan-Headers v1.4.345 (74d8a6cb930c68ef617b202c3ff3c59d919e086b), headers only. Current-main merge simulation is clean.

Checked that guarded-call stand-ins forward to the existing failure-injecting allocators, including the owned pipeline handle returned with an allocation error. Independent negative controls: removing module_id_learned from complete_from still fails even with a call elsewhere; removing init's drop_setup rollback compiles then fails live.empty() with GCC. The wider signature marker remains bounded by the existing function brace walk.

Scope: focused game-free regression review. The gate stand-in covers the non-serializing path only; no native Windows, Intel serialization, real driver fault, GPU/game or full-suite run by this reviewer. No product code changes. Assisted-by: Codex (OpenAI).

Reviewed 3c797f8ae7a747b2f3d79b2fe698a9b20e6306c5 against current main 0caf06184b391a5259d3741bb006ef5826eb6417. No blocking finding; these restore the existing checks without weakening their assertions. Independently reproduced main's stale complete_from signature failure and the framegen fixture's missing IntelDriverGate/guarded-call declarations. On the PR combined locally with current main, module-ID and framegen-resource checks pass with both GCC and Clang. Used repository-pinned Vulkan-Headers v1.4.345 (74d8a6cb930c68ef617b202c3ff3c59d919e086b), headers only. Current-main merge simulation is clean. Checked that guarded-call stand-ins forward to the existing failure-injecting allocators, including the owned pipeline handle returned with an allocation error. Independent negative controls: removing module_id_learned from complete_from still fails even with a call elsewhere; removing init's drop_setup rollback compiles then fails live.empty() with GCC. The wider signature marker remains bounded by the existing function brace walk. Scope: focused game-free regression review. The gate stand-in covers the non-serializing path only; no native Windows, Intel serialization, real driver fault, GPU/game or full-suite run by this reviewer. No product code changes. Assisted-by: Codex (OpenAI).