Repository navigation
test: the updater digest check runs where only a later .NET SDK is installed - #2294
Conversation
…stalled tests/test_updater_asset_digest.py builds the real builder/HookshotBuilder/Updater.cs into a net8.0 fixture with NuGet's sources cleared. A PC whose only SDK is a later one (here .NET SDK 10.0.113 on Arch, as distributions now ship) has no 8.0 reference pack, cannot fetch one, and the check failed in its restore: "NU1100: Unable to resolve 'Microsoft.NETCore.App.Ref (= 8.0.32)' for 'net8.0'". The fixture is still built for net8.0, the builder's framework, wherever the SDK can restore it (CI: Windows installs 8.0.x, the ubuntu-24.04 image carries 8.0 beside 9.0 and 10.0). When that restore fails with NU1100 it is built for the SDK's own framework instead, and the check says so in a line. All 20 cases pass for net10.0, and a copy of Updater.cs that no longer compares the SHA-256 fails 5 of them there, as it should. Assisted-by: Claude Opus 5.5 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Rebased onto main |
54016ac to
5335366
Compare
Joey-Magnus
left a comment
There was a problem hiding this comment.
This looks right to me. The fallback is narrow: only a failed restore that mentions NU1100 switches the framework. Since the fixture has no package references and the NuGet sources are cleared, that can only mean the net8.0 reference pack is missing. Any other restore failure still fails the check with dotnet's output. The cases themselves don't depend on the framework, and your mutation run (5 corrupt-digest cases failing under net10.0) shows the check keeps its teeth there.
CI: the updater digest check passed on both the ubuntu-24.04 and windows-2022 jobs, built for net8.0. The two failing jobs are unrelated. Python source checks (windows-2022, part 1/2) failed in test_native_astc_texture_count.py (the scanf deprecation, fixed on main by #2397), and appimage failed on a Launchpad connection timeout during add-apt-repository. A rebase or re-run should turn both green.
One suggestion, not blocking:
- minor
tests/test_updater_asset_digest.py:45: on CI the fallback would hide exactly the regression the builder cares about. If a runner image ever drops the .NET 8 pack, orUpdater.csstarts using an API newer than .NET 8, the check would quietly build for net10.0 and pass, while the real builder no longer compiles. CI only prints one line in that case. Consider failing instead of falling back whenGITHUB_ACTIONS(orCI) is set, or whenrun_all.pyruns with--strict, so net8.0 stays enforced where it is supposed to be available, while local machines with only a newer SDK keep the fallback.
…#2354) **What this is for:** frame times from headless runs, the kind scripted tests and measurements use. `RX_VK_FRAME_LOG=<csv>` ("A CSV line per present: frame, seconds, frame time in ms", docs/development.md) writes nothing in a headless run (`HOS_HEADLESS`): the file is never even created. Fixes #2353. **What is measured:** on Linux (RTX 4090, NVIDIA 615.78.08) a headless run with the change wrote 6480 lines over 108.9 s, and its first interval equals the game's own present trace of the same run; none of 23 headless runs of main left a file; a game-free test that fails on main; the whole of the checks. **Not measured:** a windowed run with the change (that path calls the same code at the same place as before), Windows game runtime, AMD, the Steam Deck under gamescope. ## What changed - `gpu/vk/vk_replay.cpp`: the writer was only in `present_out`, after `vkQueuePresentKHR`. Without a window a present copies the picture into the offscreen image and `do_present` returns before `present_image`, so it never got there. The writer is now `frame_log_present()`, unchanged (same file, header, columns and flush every 60 lines), called where it was and at the end of a headless `do_present`, once the copy into the offscreen image is submitted. - Frame generation is off without a window (`framegen_on`), so a headless run writes one line per game frame, as a window does without frame generation. - The times are the same steady clock as before. A headless run has no swapchain to wait on, so they show the game's and the replay's pace without the display's (V-Sync, the compositor). - `tests/test_frame_log_headless.py` (game-free): compiles the actual `frame_log_present` and checks its CSV header, sequence numbers, elapsed/interval times, and disabled output. It does not infer execution from call-site spelling. - `docs/development.md`: documents headless CPU intervals and the absence of swapchain/display waits. ## How it was tested Latest refresh, 2026-11-11: head `e3119ce0` on `47ed9f14`, including the merged #2397 Windows ASTC fixture repair. The `present_native_pq` conflict was resolved by retaining main’s moved declaration and this PR’s logger. The compiled CSV writer passes with GCC and Clang. No new game run or full local suite was made for this refresh; [Source-regression CI](https://github.com/NotRexed/Hookshot/actions/runs/43093270209) now passes on this head: both Windows and both Ubuntu shards. RTX 4090, NVIDIA 615.78.08, Arch Linux; headless (`HOS_HEADLESS=1`, the log says `vk: no window: presents render offscreen (1920x1080)`), from a fresh profile, no input, 120 s, muted, `RX_VK_FRAME_LOG` and `HOS_PRESENT_TRACE` set. - Main: none of 23 such headless runs left a frame log (the switch was set in every one). - With this change: `frames.csv` holds the header and 6480 lines, frames 0 to 6479 over 108.9 s, times rising from 0; the frame times average 16.80 ms (59.5 fps; the last 30 s 59.9 fps) with the game at its 60 cap through the intro movie and the title screen (pictures of the run looked at). The game's own present trace of the same run has the same first interval (43.28 ms) and 6432 presents over 108.1 s. (Not a frame-rate claim: other work ran beside it.) - That run was of this change on main bdfe09c (commit 7da36252). Rebased onto 49f75dc the change is the same apart from one blank line. Added 2026-11-11 16:27 UTC: the rebased build (cc36308 on 49f75dc, `hookshot v2.1.0-rc.3-54-gcc363089`) ran the same way, 150 s: `frames.csv` holds the header and 5880 lines, frames 0 to 5879 over 98.4 s, times rising from 0; its first interval (32.909 ms) equals the game's own present trace of that run (32,908,677 ns); no FATAL or device-lost line, no driver line in the kernel log. (Load average 22 falling to 9: not a frame-rate claim.) - Updated 2026-11-11: rebased onto `7759a6f3`, head `5ce5970f`. #1995 changed `present_out` to return `bool`; the old test pinned its full signature. The source-text call-site assertions are removed rather than repinned. The compiled writer check passes with GCC 16.2.1 and Clang 23.1.3: `PASS: frame log CSV header, sequence, elapsed/interval times, and disabled output`. Against main it fails because main has no extracted `frame_log_present` helper; that is not a headless integration test. Headless integration evidence is the two older game runs above. - On `5ce5970f`, `tests/run_all.py`: `385 passed, 2 failed, 10 skipped in 141 s`. `test_updater_asset_digest.py` cannot resolve the .NET 8 reference packs on this machine. `nx2/test_texture_phi_dominance.py` had a compiler killed by SIGKILL; its narrow rerun alone passed (`1 passed, 0 failed, 0 skipped in 20 s`). No second full suite was run. - Earlier suite on `cc363089` (load 57 to 113), `python -I -B tests/run_all.py`: `362 passed, 3 failed, 10 skipped in 193 s (linux, 32 at once)`. Two of the failures are this machine's and fail on main as well, each with a pull request of its own: `tests/test_updater_asset_digest.py` (only a .NET 10 SDK here, #2294) and `tests/test_wayland_libdecor_hint.py` (Clang 23, #2290). The third, `tests/test_display_pointer.py`, failed under that load at `reads > 0` (its reader thread had made no read yet when the check ended) and passes when run alone (3 of 3, at load 55). Added 2026-11-11: [Windows part 2/2](https://github.com/NotRexed/Hookshot/actions/runs/43072060527/job/130702262513) passes `tests/test_frame_log_headless.py`. The job fails only on the unrelated ASTC fixture’s `scanf` deprecation, fixed by #2397, which has now merged and is included in the latest refresh. This is Windows coverage of the compiled writer, not a game run. ## Not tested - No new game build/run after the subsequent rebases, including `47ed9f14`; the game runs above name their earlier heads. The renderer patch remains the same; #1995's surface-loss return stays before the existing window log position. - A windowed run of this branch: the window's path calls the same function at the same place as before; not run here. - Windows game runtime, AMD, the Steam Deck under gamescope (the switch's first use). ## Risks - A windowed run writes exactly what it wrote: the same function is called at the same place in `present_out`. - A headless line is written when the offscreen copy is submitted, not when a display shows it: there is no display. Its times are the game's and the replay's pace. Nobody should read them as presentation times, which the description of the switch does not promise either. - The file is opened at the first present, as before; a run without the switch pays one `getenv` at its first present and a null check after that. ## Overlaps Updated 2026-11-11 after the refresh: 49 open PRs touch one of this branch’s three files; one additional text conflict was reported, and its hunk was read. - #2325 (frame critical-path recorder / asynchronous presenter): its presenter-specific surface-loss and deferred-resize handling meets the old inline frame logger in `present_out`. Keep that handling and call the extracted logger once at the corresponding present boundary; do not restore a second inline writer. No combined runtime test was made. - The earlier `present_native_pq` declaration conflict is resolved in this rebase. Assisted-by: Claude Opus 5.5 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Assisted-by: GPT-6 Astra Co-Authored-By: GPT-6 Astra <noreply@openai.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: GPT-6 Astra <noreply@openai.com>
|
@Joey-Magnus thanks for the review. Your suggestion is in #2543: on a GitHub runner a failed |
…s runners (#2543) **What this is for:** on GitHub's runners the updater digest check must keep building for the builder's `net8.0`. Since #2294 it falls back to the SDK's own framework when the .NET 8 reference pack is missing. That is right on a PC with only a later SDK and wrong in CI, where it would hide a runner image without the pack, or an `Updater.cs` that no longer builds for .NET 8. This is the follow-up @Joey-Magnus suggested in his review of #2294. **What is measured:** on Linux with only the .NET 10 SDK (10.0.113): run as a PC the check passes through the fallback (20 cases); with `GITHUB_ACTIONS=true` it fails with dotnet's NU1100 output; main's file passes in that same situation. **Not measured:** a runner that really lacks the pack. This pull request's own jobs have .NET 8 installed, so they only show that the normal path is unchanged. ## Problem From the review of #2294: "on CI the fallback would hide exactly the regression the builder cares about. If a runner image ever drops the .NET 8 pack, or `Updater.cs` starts using an API newer than .NET 8, the check would quietly build for net10.0 and pass, while the real builder no longer compiles. CI only prints one line in that case." ## What changed - `tests/test_updater_asset_digest.py`: when the `net8.0` restore fails with NU1100 and `GITHUB_ACTIONS` is `true`, the check exits with a message and dotnet's output instead of switching the framework. `GITHUB_ACTIONS == "true"` is the test `tests/run_all.py` already uses to tell a runner from a PC. Everywhere else the fallback is unchanged. ## How to try it ``` python -I -B tests/test_updater_asset_digest.py # a PC with only a later SDK: falls back, 20 passed GITHUB_ACTIONS=true python -I -B tests/test_updater_asset_digest.py # the same PC as a runner: fails with NU1100 ``` ## How it was tested Arch Linux, .NET SDK 10.0.113 only (no .NET 8 reference pack), head `2294f29d` on main `fa2b25b2`. | File | Environment | Result | |---|---|---| | this branch | `GITHUB_ACTIONS` unset | the fallback's line, then `Updater digest cases: 20 passed, 0 failed.`, exit 0 | | this branch | `GITHUB_ACTIONS=true` | `on a GitHub runner the updater must build for the builder's net8.0, and its reference pack is missing: no fallback to a later framework here`, then dotnet's NU1100 lines, exit 1 | | main's file | `GITHUB_ACTIONS=true` | falls back, 20 passed, exit 0 (what this change prevents) | ## Not tested - A real runner without .NET 8, and Windows on my side (no Windows machine). - `--strict` on a PC is unchanged: the fallback still applies there. The review named `--strict` as an alternative; `run_all.py` does not pass it on to a check, and `GITHUB_ACTIONS` is what it already reads. ## Risks - A self-hosted runner (or a local runner tool) that sets `GITHUB_ACTIONS=true` without .NET 8 now fails this check instead of passing for a later framework. That is the intent. ## Overlaps - #2373 touches the same file and merges cleanly with this change (`git merge-tree`). Assisted-by: Claude Fable 5.2 Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com> Co-authored-by: Claude Fable 5.2 <noreply@anthropic.com>
tests/test_updater_asset_digest.pyfails on a PC whose only .NET SDK is newer than 8 (here Arch's .NET SDK10.0.113): its fixture targets net8.0 with NuGet's sources cleared, and that SDK has no 8.0 reference pack to build
against.
Part of #2334.
.NET 8's support ended on 2026-11-10, so more machines, and in time CI images, will have only later SDKs.
What changed
tests/test_updater_asset_digest.pystill builds the realbuilder/HookshotBuilder/Updater.csfor net8.0, thebuilder's framework, wherever the SDK can restore it. It restores first; when that fails with NU1100 (the
reference pack is not installed, and nothing is fetched since the sources are cleared), it builds for the SDK's own
framework (
net<major of dotnet --version>.0) and says so in one line:Any other restore failure still fails the check with dotnet's output.
The message when
dotnetis missing says a .NET SDK is needed, since 8 is no longer the only one that runs it.Why not skip instead: a skip passes silently (
run_all.pyhas no way for a check to report a skip at run time), andthe cases do not depend on the framework (the HTTP, process and file boundaries are the fixture's fakes). The builder
itself still targets net8.0; CI's Windows job installs .NET 8.0.x (
actions/setup-dotnet), and the ubuntu-24.04 imagelists .NET SDKs 8.0, 9.0 and 10.0 side by side (runner-images readme, image 20261101), so CI should keep building for
net8.0; not checked on CI itself.
How it was tested
Linux (Arch, .NET SDK 10.0.113 only):
Updater digest cases: 20 passed, 0 failed.(6.6 s inrun_all.py).Updater.cswhose SHA-256 comparison was made to accept anything: 5 cases fail(
setup-corrupt-no-use,folder-setup-corrupt-no-use,zip-corrupt-no-extract,api-corrupt-terminal,chunked-corrupt), so the check still catches a broken updater when built for net10.0.python -I -B tests/run_all.pyon main 49f75dc with this change (2026-11-11, load about 100 to 135): 363 passed, 1failed, 10 skipped;
tests/test_updater_asset_digest.pypasses. The one failure,tests/test_wayland_libdecor_hint.py,fails on main as well on this machine (Clang 23; fixed by test: the Wayland libdecor fixture builds with Clang 23 (-Wunused-but-set-global) #2290).
Not tested
Assisted-by: Claude Opus 5.5
Co-Authored-By: Claude Opus 5.5 noreply@anthropic.com