fix(update): use .tar.gz for Windows assets in gnu-vs-android test - #7528
Conversation
The test find_asset_url_picks_correct_gnu_over_android hardcoded .zip assets for Windows targets, but is_installable_release_asset only accepts .tar.gz / .tgz. On a Windows host the test panics with 'should find an asset' because no qualifying asset matches the host triple. Use .tar.gz for all platform entries so the test is host-triple independent — its purpose is to assert the URL picks gnu over android, not to validate the asset format filter. Fixes zeroclaw-labs#7509
WareWolf-MoonWall
left a comment
There was a problem hiding this comment.
PR Review — #7528 fix(update): use .tar.gz for Windows assets in gnu-vs-android test
Reviewer: WareWolf-MoonWall
PR: #7528 — fix(update): use .tar.gz for Windows assets in gnu-vs-android test (+2/-2, src/commands/update.rs tests only)
Round: 1 (first review)
Prior blocks: none
CI: passing
Verdict: approve
Summary
Changes two fixture entries in find_asset_url_picks_correct_gnu_over_android from zeroclaw-x86_64-pc-windows-msvc.zip / zeroclaw-aarch64-pc-windows-msvc.zip to .tar.gz. Fixes #7509 (test panic on Windows host because .zip was not accepted by is_installable_release_asset). Test-only diff; no production code touched.
Findings
🟢 Test-only, host-agnostic fix
The purpose of find_asset_url_picks_correct_gnu_over_android is to assert that GNU triples are preferred over Android triples for the same architecture. That invariant is entirely independent of asset format. Using .tar.gz fixtures separates the two concerns cleanly and lets the test pass on any host triple without masking a real format-filter gap — the gap is addressed in the complementary #7530.
🟡 PR template incomplete
The Security & Privacy Impact and Compatibility sections are absent. Answers are trivially No/Yes for a test-only change; please fill them in before merge.
🔵 Relationship to #7530
#7530 is also open and adds runtime .zip support to is_installable_release_asset. The two PRs touch different hunks of update.rs and do not conflict — both can merge independently in either order. Once #7530 lands, the .zip format becomes valid for Windows assets, meaning the original .zip fixtures would also pass the format filter. Even so, keeping .tar.gz here remains the cleaner expression of this test's intent: the gnu-vs-android preference is unrelated to format, and the fixture should not carry accidental format-filter coupling. No change needed after #7530 merges.
Audacity88
left a comment
There was a problem hiding this comment.
I reviewed PR #7528 at head 19eecc0 against the live PR state, linked issue #7509, WareWolf-MoonWall's prior approval, the related #7530 diff/review state, the one-file src/commands/update.rs test diff, and the current GitHub checks.
There are no inline review threads or top-level comments on this PR, and no active CHANGES_REQUESTED review on this PR. #7530 still has a separate active review block about the production-updater path, but #7528 only changes the test fixture, so I agree with approving this test-only fix.
✅ Resolved — The PR body now covers the template sections
WareWolf-MoonWall's earlier warning about missing Security & Privacy Impact and Compatibility sections is resolved in the current PR body. The answers are the expected shape for a test-only change: no new permissions, network calls, secrets handling, privacy impact, config surface, or compatibility risk.
🟢 What looks good — The test now checks target preference without format coupling
find_asset_url_picks_correct_gnu_over_android is meant to prove that the updater chooses the GNU/Linux asset instead of the Android asset for the same architecture. Changing the Windows fixture entries from .zip to .tar.gz keeps every host triple using a format that the current is_installable_release_asset filter already accepts, so the test can exercise its actual target-preference invariant on Windows too. That also keeps #7530 separate: #7530 still owns the production updater behavior for Windows .zip release assets, while this PR fixes the host-dependent test fixture without changing runtime behavior.
Summary
find_asset_url_picks_correct_gnu_over_androidhardcoded.zipassets for Windows targets, butis_installable_release_assetonly accepts.tar.gz/.tgz. On a Windows host the test panics with "should find an asset" because no qualifying asset matches the host triple.Fix
Use
.tar.gzfor all platform entries so the test is host-triple independent — its purpose is to assert the URL picks gnu over android, not to validate the asset format filter.Validation Evidence
ignores_non_installable_assetsSecurity & Privacy Impact
Yes, describe the risk and mitigation: N/ACompatibility
NoorYesto either: exact upgrade steps for existing users: N/ARollback
Low-risk PR:
git revert <sha>is the plan.Fixes #7509