test(e2e): add deterministic offline update-detection smoke#209
Conversation
Adds an Electron e2e spec proving an older app version observes a newer release as available, without depending on GitHub/Electron net reaching out -- closing the gap where v0.23.0 release closeout could not verify live update detection (net::ERR_TIMED_OUT from a temp-launched app). Determinism / offline / safety (by construction): - A localhost http server (ephemeral port via listen(0)) serves a controlled latest-mac.yml advertising version 99.0.0. - A test-only seam, initAutoUpdaterForE2E, points the real electron-updater at that feed (provider: generic), forces dev update config (the e2e build is unpacked), and lowers currentVersion to 0.0.1 so the feed compares as newer. autoDownload stays false, so the dummy artifact is never fetched and no install ever runs. - The seam is reached only when E2E_UPDATE_FEED_URL is set, which the spec injects and production never sets. The production updater gate (app.isPackaged && E2E_DISABLE_UPDATE !== '1') is preserved verbatim. - Runs under an isolated HOME; never touches the real HOME, userData, or /Applications. The spec asserts the renderer Redux update slice reaches status === 'available' with version === '99.0.0', exercising the full real chain: feed -> electron-updater semver compare -> UPDATE_AVAILABLE IPC -> setAvailable. Also extracts the updater event-handler registration into a shared private helper (no behavior change) and moves the 3000ms boot-check delay to a UPDATE_CHECK_DELAY_MS constant.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
WalkthroughElectron の自動アップデータ周りをリファクタし、E2E用の initAutoUpdaterForE2E と環境分岐を追加。ローカル HTTP フィードで更新検知をテストする E2E スペック、関連定数、およびユニットテストを実装しています。 ChangesE2E update detection testing
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #209 +/- ##
==========================================
+ Coverage 66.68% 66.75% +0.07%
==========================================
Files 198 198
Lines 6127 6146 +19
Branches 1384 1388 +4
==========================================
+ Hits 4086 4103 +17
Misses 1623 1623
- Partials 418 420 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/updater.test.ts (1)
20-30:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winmain-process test で
fs/promisesをモックしてください。
src/main/updater.tsは./services/settingsを import しているので、このテストは import graph の変更次第で実ファイル I/O に漏れます。既存のモック群にvi.mock('fs/promises')を追加して、main-process test を hermetic にしてください。修正例
vi.mock('electron-updater', () => ({ autoUpdater: mockAutoUpdater, })) +vi.mock('fs/promises') + // updater.ts transitively imports `electron` via typedSend (BrowserWindow) // and the settings service (app). Neither is touched by the unit under test, // so a minimal surface keeps the import graph from throwing at load time. vi.mock('electron', () => ({As per coding guidelines,
**/*.test.ts: "Main process tests: mockfs/promiseswithvi.mock('fs/promises')."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/updater.test.ts` around lines 20 - 30, Add a vi.mock('fs/promises') call in src/main/updater.test.ts alongside the existing mocks (the vi.mock('electron-updater', () => ({ autoUpdater: mockAutoUpdater })) and vi.mock('electron', ...) lines) so the test hermetically stubs filesystem promises used by updater.ts's import of ./services/settings; place it before any imports that trigger the updater import graph.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@e2e/spec/update-detection.e2e.ts`:
- Around line 113-190: The test currently launches the app with
_electron.launch() outside the main try, so if launch fails server and
isolatedHome leak, and a failing electronApp.close() can abort the remaining
cleanup; move the _electron.launch(...) call inside the existing try block so
launch failures are cleaned up, then make the finally block perform independent,
resilient cleanup: wrap each cleanup action (await electronApp?.close(), await
stopUpdateFeed(server), destroyIsolatedHome(isolatedHome)) in its own try/catch
or null-check so one failure doesn't prevent the others; reference the symbols
_electron.launch, electronApp, stopUpdateFeed, destroyIsolatedHome, server, and
isolatedHome when applying these changes.
In `@src/main/updater.ts`:
- Around line 146-172: The initAutoUpdaterForE2E function must validate
options.feedUrl is a loopback address before calling autoUpdater.setFeedURL;
reject any non-loopback URL (e.g., throw or return early with a clear error/log)
to prevent real network requests. Implement a check in initAutoUpdaterForE2E
that parses options.feedUrl, accepts only hosts that resolve to localhost
variants (localhost, 127.0.0.1, ::1) and denies everything else, and only call
autoUpdater.setFeedURL({ provider: 'generic', url: options.feedUrl }) when the
check passes; otherwise abort the setup immediately. Ensure the validation lives
in initAutoUpdaterForE2E and references options.feedUrl and
autoUpdater.setFeedURL so reviewers can find the change.
---
Outside diff comments:
In `@src/main/updater.test.ts`:
- Around line 20-30: Add a vi.mock('fs/promises') call in
src/main/updater.test.ts alongside the existing mocks (the
vi.mock('electron-updater', () => ({ autoUpdater: mockAutoUpdater })) and
vi.mock('electron', ...) lines) so the test hermetically stubs filesystem
promises used by updater.ts's import of ./services/settings; place it before any
imports that trigger the updater import graph.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: ca1c7a72-6716-4ce5-9774-b5ef3e51cbde
📒 Files selected for processing (5)
e2e/constants.tse2e/spec/update-detection.e2e.tssrc/main/index.tssrc/main/updater.test.tssrc/main/updater.ts
- updater.ts: restrict the E2E update feed URL to a loopback http origin (127.0.0.1 / localhost / ::1, bracket-aware for the WHATWG `[::1]` form) before setFeedURL, so the test-only seam can never be pointed at a real network host (which would also bypass the production app.isPackaged gate). - update-detection.e2e.ts: move _electron.launch inside the try and run cleanup via Promise.allSettled, so a launch failure or a failing close() no longer leaks the localhost feed server or the isolated HOME.
✅ CodeRabbit findings addressed (
|
2 participants
Why
v0.23.0release closeout could confirm the publiclatest-mac.yml+ asset URLs, but live old-version update detection timed out (net::ERR_TIMED_OUT) from a temp-launched app. This adds a deterministic, offline smoke so update detection is provable without depending on GitHub/Electron net.What
A new Electron e2e spec —
e2e/spec/update-detection.e2e.ts— proves an app atcurrentVersion 0.0.1observes a feed advertising99.0.0as available, end-to-end through the realelectron-updater→ IPC → Redux chain.Determinism / offline / safety (by construction)
listen(0)) http server serves a controlledlatest-mac.yml(v99.0.0).provider: 'generic', never the GitHub provider.autoDownload = false→ the dummy artifact is never fetched;downloadUpdate/quitAndInstallare never called.HOME,userData, or/Applications.initAutoUpdaterForE2Eis reached only whenE2E_UPDATE_FEED_URLis set (injected by the spec, never in production). The production gateapp.isPackaged && E2E_DISABLE_UPDATE !== '1'is preserved byte-for-byte in the else-branch.Production changes (minimal seam + tidy refactor)
src/main/updater.ts: extracted the 6autoUpdater.on(...)handlers into a privateregisterUpdaterEventHandlers()(verbatim, no behavior change) shared by both entry points;3000→UPDATE_CHECK_DELAY_MS; added the test-onlyinitAutoUpdaterForE2E({ feedUrl, currentVersion? }).src/main/index.ts: env-gatedE2E_UPDATE_FEED_URLbranch; original packaged gate preserved.src/main/updater.test.ts: 5 new behavior-named unit tests for the seam.Verification (run locally on this branch's content)
pnpm exec vitest run src/main/updater.test.ts→ 8/8 passednpx tsc -p e2e/tsconfig.json --noEmit→ cleanpnpm typecheck→ cleaneslinton all 5 files → cleanpnpm build:e2e && playwright test e2e/spec/update-detection.e2e.ts→ 1 passed (offline, ~0.9s)Known design note
The spec re-triggers
update.check()every 500ms inside the wait poll — this is the load-bearing delivery path (the main-process immediate emit can race the renderer's IPC subscription, which doesn't buffer). It routes through the existingupdate:checkIPC withautoDownload=false, so it stays safe. Flagged for reviewer awareness rather than hidden.Satisfies the standing TODO
"deterministic auto-update detection smoke that does not depend on Electron net reaching GitHub" — simulated availability from older→newer against a controlled local endpoint, proving old observes newer, leaving real HOME/app data/
/Applicationsuntouched.🤖 Generated with Claude Code
Summary by CodeRabbit