test(menu): the options check reads the keys save() owns on a line of their own, as gpu_device_id since #736 #806

Open
d4nkn3ss wants to merge 1 commit from d4nkn3ss/Longshot:fix/test-options-owned-keys into main
Contributor

Type: test · Risk: low · Tested on: Windows 11, RTX 3080 · Needs: none

Why

tests/test_options_budget_once.py has failed on main on every OS since #736 (the GPU device row keeps its card). It reports that the options file keeps an old gpu_device_id line next to the new one, but the game doesn't: save() does drop the old line. The check reads only one of the two places save() lists the keys it owns. A check that is always red hides the day this bug really comes back. I searched the tracker (forge.py issue list --state all) and the open PRs and found nothing on it.

What changed

The check now reads the keys save() owns on a mine |= key == "..." line of their own, as well as those in the bool mine = ... list. It passes on main's code and still fails when the gpu_device_id line is taken out.

  • tests: tests/test_options_budget_once.py: owned also takes mine |= key == "<key>"; from gpu/vk/vk_menu.cpp.

Measurements

Verdict: passes: the check fails on main @ aa95d35f and passes on this PR @ 09fbf742. With save()'s gpu_device_id line taken out of vk_menu.cpp, it fails again with the same message, so it still catches the bug it was written for.

Hardware: Windows 11 Pro 26300, RTX 3080 10 GB, i9-12900K, 64 GB; Python 3.12, llvm-mingw from the Longshot Builder
Scene: none (game-free checks)
Method: the check on its own on each commit, and with the ownership line removed by hand; python -I -B tests/run_all.py once on each commit, the failing checks compared
How to reproduce: python -I -B tests/test_options_budget_once.py on main and on this branch

Before: main @ aa95d35f After: this PR @ 09fbf742 After, gpu_device_id ownership line removed: this PR @ 09fbf742
test_options_budget_once.py FAIL pass FAIL (as it should)
run_all.py in all 541 passed, 17 failed 542 passed, 16 failed not run (one check matters here)
main @ aa95d35f:
FAIL: save() writes ['gpu_device_id'] but keeps the file's old lines of them too

this PR @ 09fbf742:
ok: ['auto_for', 'auto_steps', 'auto_tuned', 'gpu_device_id', 'memory_budget', 'microphone_device', 'options_version']

this PR, the gpu_device_id line removed from vk_menu.cpp:
FAIL: save() writes ['gpu_device_id'] but keeps the file's old lines of them too

Flags and settings

None

Testing

Tested: the check on its own as above; python -I -B tests/run_all.py from PowerShell on Windows 11 on main @ aa95d35f and this PR @ 09fbf742: 542 passed, 16 failed, 35 skipped here, and the 16 fail on main too: test_appimage_player_build, test_cpm_discovery, test_cpp_preflight, test_crash_dialog, test_env_on, test_lift_cleanup_error, test_linux_preflight, test_make_game_cache, test_make_game_prepare, test_missing_nup, test_oot_state_tap, test_readback_late, test_release, test_tested_configurations, test_vk_pgo, test_walk_key (each has an open PR, among them #804, #592, #622, #144).

Not tested: Linux: the check is plain Python reading a source file, the same on every OS.

Must be tested: None. The check is the change.

Risks

What could break: nothing in the game: only a check changes.

Turning it off: revert the commit.

Compatibility: Nothing changes.

Checklist

Everyone

  • One topic.
  • The title is a Conventional Commit (type(scope): description) of the status line's type.
  • Why gives the reason plainly and links the issues that fit (the tracker searched).
  • Measurements has a verdict, or one line saying why there is nothing to measure.
  • Testing names what was tested, what wasn't, and what must still be tested, where and how.
  • Rebased on current main.
  • python -I -B tests/run_all.py passes (any failure that also fails on main is named).
  • 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.

When it applies

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

🤖 Generated with Claude Code

**Type:** test · **Risk:** low · **Tested on:** Windows 11, RTX 3080 · **Needs:** none ## Why `tests/test_options_budget_once.py` has failed on main on every OS since https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/736 (the GPU device row keeps its card). It reports that the options file keeps an old `gpu_device_id` line next to the new one, but the game doesn't: `save()` does drop the old line. The check reads only one of the two places `save()` lists the keys it owns. A check that is always red hides the day this bug really comes back. I searched the tracker (`forge.py issue list --state all`) and the open PRs and found nothing on it. ## What changed The check now reads the keys `save()` owns on a `mine |= key == "..."` line of their own, as well as those in the `bool mine = ...` list. It passes on main's code and still fails when the `gpu_device_id` line is taken out. - tests: `tests/test_options_budget_once.py`: `owned` also takes `mine |= key == "<key>";` from `gpu/vk/vk_menu.cpp`. ## Measurements **Verdict:** passes: the check fails on main @ aa95d35f and passes on this PR @ 09fbf742. With `save()`'s `gpu_device_id` line taken out of `vk_menu.cpp`, it fails again with the same message, so it still catches the bug it was written for. Hardware: Windows 11 Pro 26300, RTX 3080 10 GB, i9-12900K, 64 GB; Python 3.12, llvm-mingw from the Longshot Builder Scene: none (game-free checks) Method: the check on its own on each commit, and with the ownership line removed by hand; `python -I -B tests/run_all.py` once on each commit, the failing checks compared How to reproduce: `python -I -B tests/test_options_budget_once.py` on main and on this branch | | Before: main @ aa95d35f | After: this PR @ 09fbf742 | After, gpu_device_id ownership line removed: this PR @ 09fbf742 | |---|---|---|---| | `test_options_budget_once.py` | FAIL | pass | FAIL (as it should) | | `run_all.py` in all | 541 passed, 17 failed | 542 passed, 16 failed | not run (one check matters here) | ``` main @ aa95d35f: FAIL: save() writes ['gpu_device_id'] but keeps the file's old lines of them too this PR @ 09fbf742: ok: ['auto_for', 'auto_steps', 'auto_tuned', 'gpu_device_id', 'memory_budget', 'microphone_device', 'options_version'] this PR, the gpu_device_id line removed from vk_menu.cpp: FAIL: save() writes ['gpu_device_id'] but keeps the file's old lines of them too ``` ### Flags and settings None ## Testing **Tested:** the check on its own as above; `python -I -B tests/run_all.py` from PowerShell on Windows 11 on main @ aa95d35f and this PR @ 09fbf742: 542 passed, 16 failed, 35 skipped here, and the 16 fail on main too: `test_appimage_player_build`, `test_cpm_discovery`, `test_cpp_preflight`, `test_crash_dialog`, `test_env_on`, `test_lift_cleanup_error`, `test_linux_preflight`, `test_make_game_cache`, `test_make_game_prepare`, `test_missing_nup`, `test_oot_state_tap`, `test_readback_late`, `test_release`, `test_tested_configurations`, `test_vk_pgo`, `test_walk_key` (each has an open PR, among them #804, #592, #622, #144). **Not tested:** Linux: the check is plain Python reading a source file, the same on every OS. **Must be tested:** None. The check is the change. ## Risks **What could break:** nothing in the game: only a check changes. **Turning it off:** revert the commit. **Compatibility:** Nothing changes. ## Checklist ### Everyone - [x] One topic. - [x] The title is a Conventional Commit (`type(scope): description`) of the status line's type. - [x] Why gives the reason plainly and links the issues that fit (the tracker searched). - [x] Measurements has a verdict, or one line saying why there is nothing to measure. - [x] Testing names what was tested, what wasn't, and what must still be tested, where and how. - [x] Rebased on current `main`. - [x] `python -I -B tests/run_all.py` passes (any failure that also fails on `main` is named). - [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. ### When it applies - [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. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
test(menu): the options check reads the keys save() owns on a line of their own, as gpu_device_id since #736
Some checks are pending
CI / Conventional Commit title (pull_request) Blocked by required conditions
CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Blocked by required conditions
CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Blocked by required conditions
CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Blocked by required conditions
CI / Workflow lint (actionlint) (pull_request) Blocked by required conditions
Guide screenshots / Guide images and local UI source review (pull_request) Blocked by required conditions
Third-party parts / As their notes say (pull_request) Blocked by required conditions
Third-party parts / The same as upstream (pull_request) Blocked by required conditions
PR description / Description follows the template (pull_request_target) Successful in 6s
Website / Website on Cloudflare (pull_request_target) Has been skipped
09fbf74224
save() in gpu/vk/vk_menu.cpp owns gpu_device_id with a 'mine |= key == ...' line after the 'bool mine = ...' list (#736), but test_options_budget_once.py read only the list, so it said save() keeps the file's old gpu_device_id line, and failed on main everywhere. It reads both now; without the gpu_device_id line it still fails.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
d4nkn3ss changed title from WIP: test(menu): the options check reads the keys save() owns on a line of their own, as gpu_device_id since #736 to test(menu): the options check reads the keys save() owns on a line of their own, as gpu_device_id since #736 2026-11-15 13:13:50 +00:00
Some checks are pending
CI / Conventional Commit title (pull_request) Blocked by required conditions
CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Blocked by required conditions
CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Blocked by required conditions
CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Blocked by required conditions
CI / Workflow lint (actionlint) (pull_request) Blocked by required conditions
Guide screenshots / Guide images and local UI source review (pull_request) Blocked by required conditions
Third-party parts / As their notes say (pull_request) Blocked by required conditions
Third-party parts / The same as upstream (pull_request) Blocked by required conditions
PR description / Description follows the template (pull_request_target) Successful in 6s
Website / Website on Cloudflare (pull_request_target) Has been skipped
This pull request doesn't have enough approvals yet. 0 of 1 approvals granted.
Some workflows are waiting to be reviewed.
This branch is out-of-date with the base branch
You are not authorized to merge this pull request.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u 'fix/test-options-owned-keys:d4nkn3ss-fix/test-options-owned-keys'
git switch 'd4nkn3ss-fix/test-options-owned-keys'
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
MagicBeanDev/Longshot!806
No description provided.