fix(tests): the Windows arm cannot compile test_backend_cross_device (#514, #540) - #578
Open
localai-bot wants to merge 1 commit into
Open
fix(tests): the Windows arm cannot compile test_backend_cross_device (#514, #540)#578localai-bot wants to merge 1 commit into
localai-bot wants to merge 1 commit into
Conversation
…514, #540) FOLLOWING_AGENTS_PROTOCOL `windows-msvc-vulkan` fails on EVERY pull request, and because both Windows jobs are `if: github.event_name == 'pull_request'` (.github/workflows/ci.yml :613, :637) they are skipped on `main` pushes -- so `main` reads green while every PR opened against it reads red. Confirmed on three unrelated PRs (#566, #568, #570), all failing the same pair and nothing else. The job's single compiling translation unit is this file, and it fails TWICE: test_backend_cross_device.cpp(513,20): error C2220 <- C4456 x6, /W4 /WX test_backend_cross_device.cpp(1004,5): error C3861: 'setenv' test_backend_cross_device.cpp(1048,5): error C3861: 'setenv' test_backend_cross_device.cpp(1050,5): error C3861: 'unsetenv' Fixing only the C3861s leaves the lane red on the C2220, so both are repaired here; they are the same compile of the same file. #514 -- MSVC has neither `setenv` nor `unsetenv`. Route the three call sites through file-local `SetEnv`/`UnsetEnv`, mirroring the existing idiom at tests/vllm/test_gguf.cpp:83-92 (`#if defined(_WIN32)` / `_putenv_s`, POSIX otherwise). SET and UNSET stay SEPARATE entry points rather than folding unset into `SetEnv(name, "")`: the restore arm needs absence, and collapsing the two would silently turn "absent" into "empty". The unset semantics are the thing worth proving, not assuming. On POSIX, `::unsetenv` is the byte-identical call the file already made. On MSVC, `_putenv_s(name, "")` is the documented removal form. That documentation is not executable here, so the safety does not rest on it: `vt::FusedTier()` (include/vt/fused_recipe.h:175-178, `e != nullptr && e[0] == '1'`) is the ONLY reader of VT_FUSED_TIER in the tree, and it returns 0 for absent AND for empty -- so even the pathological "left an empty value behind" outcome is observationally identical. Verified by running both properties. #540 -- the unbind-flash-layout CPU-oracle block redeclared six names from its enclosing test (`cpu`, `cq`, `cd`, `ck`, `cv`, `cslots`). Renamed the INNER ones with an `unbind_` prefix. No data, shape, stride, call or assertion moves. Behavior and coverage are unchanged on Linux: 19 test cases, 3 assertions, Status SUCCESS before and after (Release, CUDA/Vulkan OFF, gcc 13.3.0, disk 85% used). Mutating `SetEnv` to a no-op turns it RED -- 18 passed | 1 failed, Status FAILURE -- so the two `REQUIRE(vt::FusedTier() == tier)` assertions do guard the set path; the tree was then restored byte-for-byte (md5 verified). The Windows arm cannot be compiled on this Linux box and is NOT claimed here; CI is the proof. Refs #514, #540. Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5 [Claude Code]
This was referenced Aug 13, 2026
Merged
localai-bot
added a commit
that referenced
this pull request
Aug 13, 2026
…o-arg test uses (#512) (#583) FOLLOWING_AGENTS_PROTOCOL Closes #512. windows-msvc-cpu failed on EVERY pull request with "Cannot bind argument to parameter 'Arguments' because it is an empty array": Invoke-Checked's $Arguments is [Parameter(Mandatory)] and SIX call sites pass @(), and PowerShell treats an empty collection as not supplied. Fixed with [AllowEmptyCollection()] rather than a = @() default, because the intent is that a caller must STATE its argument list. A fresh review proved both halves instead of accepting the argument: omission still raises "missing mandatory parameters: Arguments" under the chosen form, while the rejected default silently binds count=0. The more useful half is where the defect gets caught. The pre-existing contract suite passed green today, which PROVES it never exercised an empty-argument call -- so a 21-minute Windows build step was catching what a seconds-long local step should have. pwsh runs this path on Linux, so Invoke-CheckedContractTests now does, and its new test fails with the CI message byte-for-byte rather than an approximation. This does NOT make Windows CI green, and the PR body was corrected to stop implying it. Closing #512 UNMASKED a hard crash: test_openai_api_server.exe dies with -1073740791 = 0xC0000409 STATUS_STACK_BUFFER_OVERRUN before doctest prints anything -- zero Status: lines, zero assertions: lines, just the version banner. That binary had never once executed on Windows, so the crash was latent for as long as #512 was. Filed as #584. The absence of a summary line IS the signature. Verified from completed jobs with distinct durations, not cancellations: PR #578 dies on the empty-array message; this branch has zero occurrences of it and dies on #584 instead, thrown from the checked-invocation error path working as designed. windows-msvc-vulkan stays red on #514. Also filed from the review: #585, the identical empty-array defect in Assert-CrtPolicy, latent only because dumpbin is invoked without /nologo and always prints a banner -- a coincidence of the call site, not a property of the function. Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5 [Claude Code]
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #514. Closes #540.
Why now
windows-msvc-vulkanfails on every pull request in the repo. Both Windows jobs are gatedif: github.event_name == 'pull_request'(.github/workflows/ci.yml:613,:637), so they are skipped onmainpushes —mainreads green while every PR opened against it reads red. That makes CI useless exactly where it matters: a real regression in someone's PR is indistinguishable from the baseline, and the failure looks like the contributor's fault.Baseline confirmed on three unrelated PRs, all failing this same pair and nothing else:
windows-msvc-cpuwindows-msvc-vulkanThe Vulkan lane fails twice, not once
test_backend_cross_device.cppis the one translation unit that job compiles beyond the shared targets, and MSVC rejects it for two independent reasons (job 94314458748):Those are issues #514 and #540. Repairing only #514 leaves the lane red on the C2220, so both are here — same compile, same file, one fix.
#514 — the POSIX environment calls
MSVC has neither
setenvnorunsetenv. The three call sites now route through file-localSetEnv/UnsetEnv, mirroring the idiom already in the tree attests/vllm/test_gguf.cpp:83-92(#if defined(_WIN32)/_putenv_s, POSIX otherwise).SET and UNSET are deliberately separate entry points.
test_gguf.cppfolds unset intoSetEnvironment(name, ""); this file must not, because the restore arm at:1050needs absence, and collapsing the two would silently turn "absent" into "empty".Unset semantics — proven, not assumed
::unsetenv— the byte-identical call the file already made._putenv_s(name, "")is the documented removal form. That documentation is not executable on this box, so the change does not rest on it.vt::FusedTier()(include/vt/fused_recipe.h:175-178,e != nullptr && e[0] == '1') is the only reader ofVT_FUSED_TIERin the tree, and it returns0for absent and for empty. So even the pathological "left an empty value behind" outcome is observationally identical.Both properties were executed:
The
_WIN32branch bodies were separately compiled-Wall -Wextra -Werroragainst the documented MSVC signatureerrno_t _putenv_s(const char*, const char*), which proves name, arity, argument types and order — not that MSVC's headers accept them. Only CI proves that.#540 — the shadowed KV-cache locals
The unbind-flash-layout CPU-oracle block redeclared six names from its enclosing
ReshapeAndCachetest. The inner ones are renamed with anunbind_prefix. No data, shape, stride, call or assertion moves.Verification
Linux arm, Release,
VLLM_CPP_CUDA=OFF VLLM_CPP_VULKAN=OFF, gcc 13.3.0, disk 85% used:51ec6bed5)Assertion count is unchanged, which is the point — this is a portability change, not a coverage change.
Mutation check. Stubbing
SetEnvto a no-op turns the gate RED —18 passed | 1 failed,Status: FAILURE!— so the twoREQUIRE(vt::FusedTier() == tier)assertions genuinely guard the set path. Tree restored byte-for-byte afterwards (md5 verified).Not claimed: the Windows arm was not compiled locally. This box is Linux and has no MSVC, mingw or wine. CI is the proof and the result will be reported from the checks below, not predicted.
What this PR does not fix
windows-msvc-cpufails for an unrelated reason — a PowerShell defect, not a compile error:That is #512, and PR #524 (
row/ENG-RELEASE-WINDOWS-512) is already in flight for it. This PR is expected to leavewindows-msvc-cpured until #524 lands. See also #503, which is why this class of breakage survives onmainat all.🤖 Generated with Claude Code