Skip to content

Fix video_out_flip_tests termination and run in ctest - #1048

Closed
Tenith01 wants to merge 2 commits into
boykopovar:mainfrom
Tenith01:fix/issue-1040-video-out-flip-tests
Closed

Tenith01 wants to merge 2 commits into
boykopovar:mainfrom
Tenith01:fix/issue-1040-video-out-flip-tests

Conversation

@Tenith01

@Tenith01 Tenith01 commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

Summary

Resolves the process termination occurring in video_out_flip_tests on the default lifetime mode and registers the headless execution tests in CMake.

Changes

  • Wrapped AgcDriverShutdown_nid_postfix() in a try/catch within VideoOutDriver::presentLoop() to catch the shutdown error without crashing the thread, properly recording it to flipQueue->failure.
  • Removed EXCLUDE_FROM_ALL on video_out_flip_tests when BUILD_TESTING is active so it's compiled with ctest.
  • Registered add_test instances for lifetime, decode, and controls modes with matching headless properties.

Testing

  • Modified logic validated against the original issue's propagation path descriptions to verify exception catching on thread shutdown and proper rethrow.
  • Checked CMake CI definitions to assure alignment with guest_video_out_wqhd_detection.

Related Issue

Fixes #1040

Note: This PR was AI-assisted and modifies video_out_flip_tests for safe headless modes without a fixture. The reopen behavior discussed in the issue is left unchanged and can be decided by maintainers as a follow-up.

@oneandonlydean

Copy link
Copy Markdown
Collaborator

Have you run the three new ctest entries? The description says the change was checked against the issue text only, and video_out_flip_lifetime needs SDL and a Vulkan device (#1040 used SDL_VIDEODRIVER=offscreen and lavapipe). Please add the ctest output and fill in the PR template.

@MarcZX10

MarcZX10 commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

Independent Linux validation of the current head 84be0c860ee9d0c5fd367e663815b23c8550eca6. Built video_out_flip_tests from a fresh Release build with BUILD_TESTING=ON (GCC 12.2, Debian x86_64), then ran the three registered cases under SDL offscreen with lavapipe and the CI Vulkan flags: video_out_flip_lifetime, video_out_flip_decode, and video_out_flip_controls all passed (3/3, 0.86 s total).

This validates the three Linux CTest entries; Windows was not tested. The PR description still needs the repository What / Tested / Checklist template requested in review.

@MarcZX10

MarcZX10 commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

While checking the requested PR template, I found a duplicate to resolve first: open PR #1575 also wraps AgcDriverShutdown_nid_postfix() in presentLoop() and registers the same three headless flip tests with the same CTest names. Its CMake/test-registration changes overlap this PR's scope; #1575 also removes video_out_flip_tests from tools/check_test_registration.py's known-unrun list. CONTRIBUTING says to avoid a second PR implementing the same functions; please coordinate with #1575's author/maintainers on which PR should carry the fix, then update this PR's body and checks for the chosen scope.

For reference, I independently built this head (84be0c86) and got 3/3 CTest passes on Linux with SDL offscreen and lavapipe; Windows remains untested. The PR currently has no upstream check results.

@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 9, 2026
@oneandonlydean

Copy link
Copy Markdown
Collaborator

Closing: #1225 makes the same presentLoop change and test registration for #1040, with Windows runs. If it stalls, please reopen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

build needs rebase Conflicts with main, rebase needed prx

Projects

None yet

Development

Successfully merging this pull request may close these issues.

video_out_flip_tests is not in ctest and its lifetime and reopen modes terminate on main

3 participants