Skip to content

Add regression tests for platformAsset selection logic#152

Merged
krazyjakee merged 2 commits into
masterfrom
claude/epic-tesla-jj14ou
Jun 16, 2026
Merged

Add regression tests for platformAsset selection logic#152
krazyjakee merged 2 commits into
masterfrom
claude/epic-tesla-jj14ou

Conversation

@krazyjakee

@krazyjakee krazyjakee commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds comprehensive test coverage for the UpdateController.platformAsset() selection logic to prevent a recurring bug where unextractable package formats (.deb, .rpm, .appimage, etc.) were incorrectly chosen for in-place installation, causing "Unsupported archive" crashes.

Changes

  • Import universal_platform to enable platform-specific test assertions

  • Add test helper functions:

    • setLatest() — sets up a mock release with specified assets
    • _asset() — creates test asset objects
    • _fullReleaseAssets — realistic asset list mirroring a real multi-platform release (including both .deb and .tar.gz for Linux, in GitHub's alphabetical order)
    • _hostInstallableExt — getter that returns the only extractable extension the current host platform supports (.apk on Android, .zip on Windows, .dmg on macOS, .tar.gz on Linux, null on web/iOS)

  • Add regression test group 'platformAsset selection (Unsupported archive regression)' with five tests:

    1. never hands an unextractable package to the in-place installer — verifies that .deb, .rpm, .appimage, -setup.exe, .msi, .pkg are never selected
    2. selects the extension the host installer can actually apply — confirms the correct platform-specific format is chosen
    3. prefers .tar.gz over .deb when the .deb is listed first — reproduces the exact GitHub ordering bug and verifies the fix
    4. a package-only release has no in-place asset but is downloadable — ensures graceful fallback when only unextractable formats exist
    5. platformAssetUrl prefers .tar.gz over .deb for manual download — validates download URL selection respects priority
    6. canInstallInPlace is true only when an extractable asset exists — confirms the flag correctly reflects availability

Implementation Details

  • Tests are platform-aware: Linux-specific tests skip on other platforms; web/iOS tests handle the null case
  • The _fullReleaseAssets list intentionally mirrors the real v0.2.4 release structure to catch the exact ordering trap that caused the original bug
  • All tests use the existing makeContainer() and controllerOf() helpers for consistency with the test suite

https://claude.ai/code/session_01YMPGSUtRSXJExgsvLxPoCm

claude added 2 commits June 15, 2026 21:30
@claude
test(updates): guard against re-selecting unextractable .deb for in-p…
de20ea4 
…lace install

The self-updater's in-place install can only extract a .tar.gz/.zip, mount a
.dmg, or hand off a .apk; package formats (.deb/.rpm/.appimage) and setup
installers are download-only. #99 fixed asset selection picking the alphabetically
-first .deb over the .tar.gz on Linux ("Unsupported archive: daccord-linux-x86_64.deb")
but shipped no test, so the regression silently re-emerged on v0.2.4.

Add regression tests over UpdateController's asset selection using the real
release asset set (both .deb and .tar.gz, .deb listed first): platformAsset()
never returns an unextractable package, always picks the host-applicable
extension, prefers .tar.gz over .deb, and a package-only release falls back to a
download link instead of failing mid-install.
@claude
Fix Windows zip ambiguity in asset selection and test import ordering
ef464e1 
- _installableExts and _downloadExts for Windows now prefer the
  more-specific 'windows-x86_64.zip' suffix before the generic '.zip'
  fallback, preventing daccord-web.zip (which sorts alphabetically
  before daccord-windows-x86_64.zip in GitHub's asset list) from being
  selected as the in-place Windows installer — the same class of bug
  as the .deb-over-.tar.gz regression being guarded here.
- Add a Windows regression test mirroring the Linux .deb/.tar.gz test.
- Strengthen the 'selects the extension the host installer can apply'
  assertion on Windows to verify the name contains 'windows', not just
  that the extension is .zip.
- Fix import ordering in the test file: dart: imports before package:.

https://claude.ai/code/session_01HPonjbbg1y1GMMTQLKebce
@krazyjakee Claude

krazyjakee commented Jun 16, 2026

Copy link
Copy Markdown
Contributor Author

Code Review — findings and fixes (commit ef464e1)

Reviewed test/features/updates/update_controller_test.dart and the production UpdateController it covers. Three issues found; all fixed and pushed to this branch.

🔴 HIGH — Windows .zip ambiguity produces same bug class as the regression being guarded

Where: update_controller.dart _installableExts / _downloadExts, plus the test's _fullReleaseAssets and 'selects the extension the host installer can actually apply'.

_fullReleaseAssets correctly mirrors the real alphabetical GitHub asset order, which places daccord-web.zip before daccord-windows-x86_64.zip. On Windows, _installableExts returned ['.zip'], so _assetForExts picked the first .zip match — daccord-web.zip, the web bundle — as the in-place Windows installer. This is exactly the same failure mode as the .deb-over-.tar.gz bug this PR guards against, just for Windows.

The test's assertion asset!.name.endsWith('.zip') passed for daccord-web.zip, masking the bug entirely.

Fix (production): _installableExts for Windows now returns ['windows-x86_64.zip', '.zip'] and _downloadExts returns ['windows-x86_64.zip', '.zip', '-setup.exe', '.exe', '.msi'] — the more-specific suffix is checked first, then the generic .zip fallback for future naming conventions.

Fix (test): Added a Windows regression test parallel to the Linux one:

test('prefers windows zip over web zip when web zip is listed first', () {
  if (!UniversalPlatform.isWindows) return;
  // daccord-web.zip sorts before daccord-windows-x86_64.zip alphabetically
  setLatest(c, [_asset('daccord-web.zip'), _asset('daccord-windows-x86_64.zip')]);
  expect(asset!.name, equals('daccord-windows-x86_64.zip'));
});

Also tightened the 'selects the extension the host installer can actually apply' assertion on Windows to require the name contains 'windows', not just ends with '.zip'.

🟡 MEDIUM — dart:io import placed after package: imports

Where: update_controller_test.dart lines 1–9 (pre-fix).

Dart's directives_ordering lint requires dart: imports before package: imports. The dart:io import was placed last, after all package: imports (this was pre-existing in the file; the PR added the universal_platform import in the same wrong position).

Fix: Moved dart:io to line 1, with a blank line separating it from the package: block.

🟢 LOW — Platform-conditional tests use silent return instead of markTestSkipped

Where: All tests with if (!UniversalPlatform.isLinux) return; / if (!UniversalPlatform.isWindows) return;.

Using return makes the test report as passed on non-matching platforms rather than skipped. This is a common pattern in Flutter tests and not a bug, but it means CI on macOS/Windows counts these as passing without executing them. Left unfixed since changing to markTestSkipped would be a noise-only refactor and the pattern is consistent with the existing test suite style.

Generated by Claude Code

@krazyjakee krazyjakee merged commit 782efe5 into master Jun 16, 2026
3 checks passed
@krazyjakee krazyjakee deleted the claude/epic-tesla-jj14ou branch June 16, 2026 08:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@krazyjakee @claude