test(vk): restore source fixture dependencies after admission changes #1819

Merged

Arkex-ai merged 1 commit from test/renderer-fixture-integration into main

2026-11-10 17:39:14 +00:00

Arkex-ai commented

2026-11-10 17:33:39 +00:00

(Migrated from github.com)

Extract the actual stream_insert before its callers, include its exception dependency, provide the descriptor-pool destroy spy and initialize the fixture's live recording pool. Existing assertions and compiler warnings are unchanged. Production GPU, host, lifter and third-party trees are unchanged.

Closes #1813. Claim; CI-first priority, S3 x1.0 x1.0 =1.0.

Validation:

Windows LLVM and Linux GCC/ASan/UBSan: all16 stream scenarios and108 sampler cases pass.
Five negative controls: removing either dependency reproduces compilation failure; removing the initial pool fails the existing binder assertion; removing stream visibility or sampler rejection still fails the behavioral assertions.
Full Windows suite at 9eb317e / main bd5086a:274 passed/5 failed/20 skipped versus272/7/20. Exactly the two renderer failures are removed. Remaining failures are the two local symlink/SDK limitations and the three checks addressed by maintainer #1793.
Refreshed onto main 484cfd1; production GPU and all tested fixture sources are byte-identical to the validated head. Both Windows fixtures pass again, as do the17 updated Linux-DLSS build-configuration tests. Current-head source checks, Windows builder and AppImage are dispatched.

This is a test-only integration repair. It adds no native allocation-failure or game-performance claim; descriptor-pool rollback behavior remains covered by the recording-admission fixture.

Current-head validation

Current head 2f4c072, based on main 484cfd1. Earlier local/runtime results describe their recorded source hashes.

Linux AppImage: pending.
Windows builder: in_progress.
Game-free source regressions: pending.

Fork runs are separate from upstream Actions approval; no maintainer approval gate is bypassed.

Two renderer source checks stop compiling after #1658/#1754: the stream fixture omits the new production helper, and the sampler fixture lacks the descriptor-pool environment now required by the complete binder. Extract the actual `stream_insert` before its callers, include its exception dependency, provide the descriptor-pool destroy spy and initialize the fixture's live recording pool. Existing assertions and compiler warnings are unchanged. Production GPU, host, lifter and third-party trees are unchanged. Closes #1813. [Claim](https://discord.com/channels/1568810484818378804/1569019786766188665/1569763504758464609); CI-first priority, S3 x1.0 x1.0 =1.0. Validation: - Windows LLVM and Linux GCC/ASan/UBSan: all16 stream scenarios and108 sampler cases pass. - Five negative controls: removing either dependency reproduces compilation failure; removing the initial pool fails the existing binder assertion; removing stream visibility or sampler rejection still fails the behavioral assertions. - Full Windows suite at `9eb317e` / main `bd5086a`:274 passed/5 failed/20 skipped versus272/7/20. Exactly the two renderer failures are removed. Remaining failures are the two local symlink/SDK limitations and the three checks addressed by maintainer #1793. - Refreshed onto main `484cfd1`; production GPU and all tested fixture sources are byte-identical to the validated head. Both Windows fixtures pass again, as do the17 updated Linux-DLSS build-configuration tests. Current-head [source checks](https://github.com/Arkex-ai/Hookshot/actions/runs/42865995085), [Windows builder](https://github.com/Arkex-ai/Hookshot/actions/runs/42866000673) and [AppImage](https://github.com/Arkex-ai/Hookshot/actions/runs/42866005759) are dispatched. This is a test-only integration repair. It adds no native allocation-failure or game-performance claim; descriptor-pool rollback behavior remains covered by the recording-admission fixture.  ### Current-head validation Current head `2f4c072`, based on main `484cfd1`. Earlier local/runtime results describe their recorded source hashes. - [Linux AppImage](https://github.com/Arkex-ai/Hookshot/actions/runs/42866005759): pending. - [Windows builder](https://github.com/Arkex-ai/Hookshot/actions/runs/42866000673): in_progress. - [Game-free source regressions](https://github.com/Arkex-ai/Hookshot/actions/runs/42865995085): pending. Fork runs are separate from upstream Actions approval; no maintainer approval gate is bypassed.

hckrman101 (Migrated from github.com) approved these changes

2026-11-10 17:38:26 +00:00

hckrman101 (Migrated from github.com) left a comment

Checked against main's gpu/vk/vk_draw.cpp: stream_insert exists (called from stream_next and the recording setup) and is a try/catch on std::exception, so extracting it first and including is what the stream fixture needs. bind_descriptors now takes cur->pool and calls vkDestroyDescriptorPool on the fallback path, so the destroy spy and the initial recording.pool match what the sampler fixture has to provide. Test and docs only, no production files touched. CI on this head was still pending when I looked.

Checked against main's gpu/vk/vk_draw.cpp: stream_insert exists (called from stream_next and the recording setup) and is a try/catch on std::exception, so extracting it first and including <exception> is what the stream fixture needs. bind_descriptors now takes cur->pool and calls vkDestroyDescriptorPool on the fallback path, so the destroy spy and the initial recording.pool match what the sampler fixture has to provide. Test and docs only, no production files touched. CI on this head was still pending when I looked.

NotRexed commented

2026-11-10 17:53:21 +00:00

(Migrated from github.com)

Two of the renderer source fixtures fail on main in CI after this and #1658 landed. The same failures show in pull-request runs built on current main (for example #1795's run 42864754292 and #1814's run 42864698601, both from 17:24-17:25 UTC). Could you take a look?

tests/test_sampler_admission.py, on both ubuntu-24.04 and windows-2022:

fixture.cpp:462:13: error: use of undeclared identifier 'vkDestroyDescriptorPool'
fixture.cpp:468:13: error: use of undeclared identifier 'vkDestroyDescriptorPool'

tests/test_draw_stream_visibility.py, on both OSes (Linux adds did you mean 'std::__ostream_insert'?):

fixture.cpp:84:14: error: use of undeclared identifier 'stream_insert'
fixture.cpp:175:28: error: use of undeclared identifier 'stream_insert'

vkDestroyDescriptorPool and stream_insert are both called from code #1658 changed (6e93f80, "recover recording stream and descriptor allocation failures"), so the extracted blocks now need them and the fixtures don't declare or stub them. Both fixtures reportedly pass locally with llvm-mingw clang++/libc++ and Vulkan headers v1.4.345. So it is either a stub the fixtures lack or a declaration that only arrives through libc++/llvm-mingw's headers, not CI's.

(nx2/test_draw_parameters.py's no member named 'mesh_fallback' in 'Ctx' is a separate failure, already covered by #1793.) Thanks!

Two of the renderer source fixtures fail on `main` in CI after this and #1658 landed. The same failures show in pull-request runs built on current `main` (for example #1795's run 42864754292 and #1814's run 42864698601, both from 17:24-17:25 UTC). Could you take a look? **`tests/test_sampler_admission.py`**, on both `ubuntu-24.04` and `windows-2022`: ``` fixture.cpp:462:13: error: use of undeclared identifier 'vkDestroyDescriptorPool' fixture.cpp:468:13: error: use of undeclared identifier 'vkDestroyDescriptorPool' ``` **`tests/test_draw_stream_visibility.py`**, on both OSes (Linux adds `did you mean 'std::__ostream_insert'?`): ``` fixture.cpp:84:14: error: use of undeclared identifier 'stream_insert' fixture.cpp:175:28: error: use of undeclared identifier 'stream_insert' ``` `vkDestroyDescriptorPool` and `stream_insert` are both called from code #1658 changed (6e93f80, "recover recording stream and descriptor allocation failures"), so the extracted blocks now need them and the fixtures don't declare or stub them. Both fixtures reportedly pass locally with llvm-mingw clang++/libc++ and Vulkan headers v1.4.345. So it is either a stub the fixtures lack or a declaration that only arrives through libc++/llvm-mingw's headers, not CI's. (`nx2/test_draw_parameters.py`'s `no member named 'mesh_fallback' in 'Ctx'` is a separate failure, already covered by #1793.) Thanks!

NotRexed commented

2026-11-10 18:19:37 +00:00

(Migrated from github.com)

Correction, sorry: the two failures in my last comment came from pull-request runs started before this PR merged. main's first run with it (ae08298, run 42867154808) passes both tests/test_sampler_admission.py and tests/test_draw_stream_visibility.py, so this PR fixed them. Nothing to do here. Thanks!

Correction, sorry: the two failures in my last comment came from pull-request runs started before this PR merged. `main`'s first run with it (ae08298, run 42867154808) passes both `tests/test_sampler_admission.py` and `tests/test_draw_stream_visibility.py`, so this PR fixed them. Nothing to do here. Thanks!