アプリ再起動時にFLPサブスクリプションの蓄積を防止してAndroid 16のバックグラウンド位置更新を改善#5423
Merged
TinyKitten merged 2 commits intodevfrom Feb 22, 2026
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 📝 WalkthroughWalkthrough背景での位置情報更新を開始する前に、同名の既存タスクが起動していないか Changes
Sequence Diagram(s)sequenceDiagram
participant Hook as useStartBackgroundLocationUpdates
participant LocationAPI as Location API
participant Logger as Logger
Hook->>LocationAPI: hasStartedLocationUpdatesAsync(LOCATION_TASK_NAME)
alt ステールタスクあり
LocationAPI-->>Hook: true
Hook->>LocationAPI: stopLocationUpdatesAsync(LOCATION_TASK_NAME)
LocationAPI-->>Hook: success
else ステールタスクなし
LocationAPI-->>Hook: false
else クリーンアップ失敗
LocationAPI-->>Hook: error
Hook->>Logger: warn("stale cleanup failed")
end
Hook->>LocationAPI: startLocationUpdatesAsync(LOCATION_TASK_NAME, options)
LocationAPI-->>Hook: success
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/hooks/useStartBackgroundLocationUpdates.test.tsx (2)
112-128: 呼び出し順序の検証を追加することを検討してください。このテストは
stopLocationUpdatesAsyncとstartLocationUpdatesAsyncの両方が呼び出されることを検証していますが、「stopがstartの前に呼ばれる」という順序の検証がありません。♻️ 呼び出し順序を検証する例
+ const callOrder: string[] = []; + mockStopLocationUpdatesAsync.mockImplementation(() => { + callOrder.push('stop'); + return Promise.resolve(); + }); + mockStartLocationUpdatesAsync.mockImplementation(() => { + callOrder.push('start'); + return Promise.resolve(); + }); + renderHook(() => useStartBackgroundLocationUpdates()); await new Promise(process.nextTick); expect(mockHasStartedLocationUpdatesAsync).toHaveBeenCalledWith( LOCATION_TASK_NAME ); expect(mockStopLocationUpdatesAsync).toHaveBeenCalledWith( LOCATION_TASK_NAME ); expect(mockStartLocationUpdatesAsync).toHaveBeenCalled(); + expect(callOrder).toEqual(['stop', 'start']);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useStartBackgroundLocationUpdates.test.tsx` around lines 112 - 128, Add an assertion that verifies stopLocationUpdatesAsync was invoked before startLocationUpdatesAsync in the test for useStartBackgroundLocationUpdates: after rendering and awaiting the tick (where you already assert both calls occurred), read the mock invocation order values from mockStopLocationUpdatesAsync.mock.invocationCallOrder and mockStartLocationUpdatesAsync.mock.invocationCallOrder and assert the stop invocation order is less than the start invocation order so the test confirms stop happens before start; reference the existing mocks mockStopLocationUpdatesAsync and mockStartLocationUpdatesAsync and the hook useStartBackgroundLocationUpdates.
130-144:stopLocationUpdatesAsyncが呼ばれないことの明示的なアサーションが不足しています。コメントには「stopはクリーンアップ用のstartの前には呼ばれない」と記載されていますが、実際のアサーションがありません。unmount時のクリーンアップでも
stopLocationUpdatesAsyncが呼ばれるため、このテストではstartLocationUpdatesAsync呼び出し時点でstopLocationUpdatesAsyncが呼ばれていないことを検証すべきです。♻️ アサーションを追加する例
expect(mockHasStartedLocationUpdatesAsync).toHaveBeenCalledWith( LOCATION_TASK_NAME ); - // stopはクリーンアップ用のstartの前には呼ばれない + // プリクリーンアップのstopは呼ばれない（startの前にstopが呼ばれていないことを確認） + expect(mockStopLocationUpdatesAsync).not.toHaveBeenCalled(); expect(mockStartLocationUpdatesAsync).toHaveBeenCalled();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useStartBackgroundLocationUpdates.test.tsx` around lines 130 - 144, テストで startLocationUpdatesAsync を呼んだ時点で stopLocationUpdatesAsync がまだ呼ばれていないことを明示的に検証してください：useStartBackgroundLocationUpdates のテスト内で process.nextTick 後に expect(mockStopLocationUpdatesAsync).not.toHaveBeenCalled() を追加し、続けて expect(mockStartLocationUpdatesAsync).toHaveBeenCalled() をアサートしてください（アンマウント時のクリーンアップで stopLocationUpdatesAsync が呼ばれることを確認したい場合は、renderHook の返り値の unmount を呼んだ後に expect(mockStopLocationUpdatesAsync).toHaveBeenCalled() を追加してください）。
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/hooks/useStartBackgroundLocationUpdates.test.tsx`:
- Around line 112-128: Add an assertion that verifies stopLocationUpdatesAsync
was invoked before startLocationUpdatesAsync in the test for
useStartBackgroundLocationUpdates: after rendering and awaiting the tick (where
you already assert both calls occurred), read the mock invocation order values
from mockStopLocationUpdatesAsync.mock.invocationCallOrder and
mockStartLocationUpdatesAsync.mock.invocationCallOrder and assert the stop
invocation order is less than the start invocation order so the test confirms
stop happens before start; reference the existing mocks
mockStopLocationUpdatesAsync and mockStartLocationUpdatesAsync and the hook
useStartBackgroundLocationUpdates.
- Around line 130-144: テストで startLocationUpdatesAsync を呼んだ時点で
stopLocationUpdatesAsync
がまだ呼ばれていないことを明示的に検証してください：useStartBackgroundLocationUpdates のテスト内で
process.nextTick 後に expect(mockStopLocationUpdatesAsync).not.toHaveBeenCalled()
を追加し、続けて expect(mockStartLocationUpdatesAsync).toHaveBeenCalled()
をアサートしてください（アンマウント時のクリーンアップで stopLocationUpdatesAsync
が呼ばれることを確認したい場合は、renderHook の返り値の unmount を呼んだ後に
expect(mockStopLocationUpdatesAsync).toHaveBeenCalled() を追加してください）。
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/hooks/useStartBackgroundLocationUpdates.test.tsx (1)
45-52:⚠️ Potential issue | 🟡 Minor
afterEachでjest.clearAllMocks()を実行してください。
失敗時にモックが残る可能性があるため、ガイドラインどおりafterEachに移してください。修正案
beforeEach(() => { - jest.clearAllMocks(); mockAutoModeEnabled = false; mockStartLocationUpdatesAsync.mockResolvedValue(undefined); mockStopLocationUpdatesAsync.mockResolvedValue(undefined); mockHasStartedLocationUpdatesAsync.mockResolvedValue(false); mockWatchPositionAsync.mockResolvedValue({ remove: mockRemove }); }); + + afterEach(() => { + jest.clearAllMocks(); + });As per coding guidelines, "Call
jest.clearAllMocks()inafterEachblocks in all test files".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useStartBackgroundLocationUpdates.test.tsx` around lines 45 - 52, The test currently calls jest.clearAllMocks() inside beforeEach; remove that call from the beforeEach and add an afterEach block that calls jest.clearAllMocks() instead (e.g., afterEach(() => jest.clearAllMocks())); keep the existing mock setup for mockStartLocationUpdatesAsync, mockStopLocationUpdatesAsync, mockHasStartedLocationUpdatesAsync, mockWatchPositionAsync and mockRemove in the beforeEach, ensuring only jest.clearAllMocks() is moved to afterEach to follow the guideline.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/hooks/useStartBackgroundLocationUpdates.test.tsx`:
- Around line 137-145: The file
src/hooks/useStartBackgroundLocationUpdates.test.tsx failed Biome formatting
around the test "should not stop when no stale task exists"; fix it by running
the Biome formatter (biome format) on that file (or the repo), verify the diff
is clean, and re-commit the formatted changes so
useStartBackgroundLocationUpdates.test.tsx and the test block around
renderHook(() => useStartBackgroundLocationUpdates()) match the project's Biome
formatting rules.
---
Outside diff comments:
In `@src/hooks/useStartBackgroundLocationUpdates.test.tsx`:
- Around line 45-52: The test currently calls jest.clearAllMocks() inside
beforeEach; remove that call from the beforeEach and add an afterEach block that
calls jest.clearAllMocks() instead (e.g., afterEach(() =>
jest.clearAllMocks())); keep the existing mock setup for
mockStartLocationUpdatesAsync, mockStopLocationUpdatesAsync,
mockHasStartedLocationUpdatesAsync, mockWatchPositionAsync and mockRemove in the
beforeEach, ensuring only jest.clearAllMocks() is moved to afterEach to follow
the guideline.
TinyKitten
force-pushed
the
claude/fix-android-location-updates-65U9y
branch
from
7e9dba0 to
6e6abf0
Compare
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/hooks/useStartBackgroundLocationUpdates.test.tsx (1)
45-52:⚠️ Potential issue | 🟡 Minor
jest.clearAllMocks()はafterEachに移してください。
現状はbeforeEachのみなのでガイドライン違反です。修正案
beforeEach(() => { - jest.clearAllMocks(); mockAutoModeEnabled = false; mockStartLocationUpdatesAsync.mockResolvedValue(undefined); mockStopLocationUpdatesAsync.mockResolvedValue(undefined); mockHasStartedLocationUpdatesAsync.mockResolvedValue(false); mockWatchPositionAsync.mockResolvedValue({ remove: mockRemove }); }); + +afterEach(() => { + jest.clearAllMocks(); +});As per coding guidelines, "Call
jest.clearAllMocks()inafterEachblocks in all test files".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useStartBackgroundLocationUpdates.test.tsx` around lines 45 - 52, Move the jest.clearAllMocks() call out of the beforeEach block and into an afterEach block in this test file: keep the existing mock initializations (mockAutoModeEnabled, mockStartLocationUpdatesAsync.mockResolvedValue, mockStopLocationUpdatesAsync.mockResolvedValue, mockHasStartedLocationUpdatesAsync.mockResolvedValue, mockWatchPositionAsync.mockResolvedValue) inside beforeEach, and add an afterEach that calls jest.clearAllMocks() so that mocks are cleared after each test; reference the beforeEach/afterEach hooks and the jest.clearAllMocks() call when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/hooks/useStartBackgroundLocationUpdates.test.tsx`:
- Around line 45-52: Move the jest.clearAllMocks() call out of the beforeEach
block and into an afterEach block in this test file: keep the existing mock
initializations (mockAutoModeEnabled,
mockStartLocationUpdatesAsync.mockResolvedValue,
mockStopLocationUpdatesAsync.mockResolvedValue,
mockHasStartedLocationUpdatesAsync.mockResolvedValue,
mockWatchPositionAsync.mockResolvedValue) inside beforeEach, and add an
afterEach that calls jest.clearAllMocks() so that mocks are cleared after each
test; reference the beforeEach/afterEach hooks and the jest.clearAllMocks() call
when making the change.
6e6abf0 to
2e4e7d2
Compare
Member
Author
|
@coderabbitai ほかになければ承認してもろて |
Contributor
|
🧠 Learnings used✅ Actions performedComments resolved and changes approved. |
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
2 participants
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.
https://claude.ai/code/session_01Kg2KPdwZq34z3uo8L8vmdX
Summary by CodeRabbit
バグ修正
テスト