Closed
Conversation
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughSelectLineScreen.test.tsxに、路線切り替え時の状態リセット動作を検証する新しいテストスイートが追加されました。列車タイプと駅関連のデータが適切にクリアされ、新しい路線データが上書きされることを確認するテストです。 Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
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.
Actionable comments posted: 2
🤖 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/screens/SelectLineScreen.test.tsx`:
- Around line 623-625: Move the jest.clearAllMocks() call out of the beforeEach
hook and place it in an afterEach hook; specifically remove the existing
beforeEach(() => { jest.clearAllMocks(); }) and add afterEach(() => {
jest.clearAllMocks(); }) so tests in SelectLineScreen.test.tsx follow the
guideline to clear mocks after each test.
- Around line 627-753: The tests currently construct and invoke the state
updater themselves, so they pass even if SelectLineScreen's handler doesn't
reset state; instead render/mount SelectLineScreen and trigger the actual
line-selection action (call the handler via user event e.g., fireEvent.press on
the line item or invoke the component prop that calls handleLineSelected) so the
component supplies the updater to mockSetNavigationState/mockSetStationState;
then grab the updater passed by the component
(mockSetNavigationState.mock.calls[0][0] or
mockSetStationState.mock.calls[0][0]) and apply it to the stale state to assert
fetchedTrainTypes/trainType/pendingTrainType and
pendingStation/pendingStations/selectedDirection/wantedDestination/selectedBound
were cleared before fetch and that subsequent fetch updates (simulate fetch
completion) overwrite only intended fields; ensure you no longer call mockSet...
with an inline updater in the test itself but simulate the UI event that
triggers SelectLineScreen.handleLineSelected.
Comment on lines
+623
to
+625
| beforeEach(() => { | ||
| jest.clearAllMocks(); | ||
| }); |
Contributor
There was a problem hiding this comment.
jest.clearAllMocks() は afterEach で呼んでください
ガイドラインに合わせて beforeEach ではなく afterEach に移してください。
修正案
- beforeEach(() => {
+ afterEach(() => {
jest.clearAllMocks();
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| beforeEach(() => { | |
| jest.clearAllMocks(); | |
| }); | |
| afterEach(() => { | |
| jest.clearAllMocks(); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/screens/SelectLineScreen.test.tsx` around lines 623 - 625, Move the
jest.clearAllMocks() call out of the beforeEach hook and place it in an
afterEach hook; specifically remove the existing beforeEach(() => {
jest.clearAllMocks(); }) and add afterEach(() => { jest.clearAllMocks(); }) so
tests in SelectLineScreen.test.tsx follow the guideline to clear mocks after
each test.
Comment on lines
+627
to
+753
| it('路線選択時に前回の列車種別データ（fetchedTrainTypes, trainType, pendingTrainType）がクリアされる', () => { | ||
| const mockSetNavigationState = jest.fn(); | ||
| // 大江戸線を選択した後の状態を再現 | ||
| const staleNavigationState = { | ||
| headerState: 'CURRENT', | ||
| leftStations: [], | ||
| trainType: { id: 10, groupId: 500, name: '都営大江戸線' }, | ||
| pendingTrainType: { id: 10, groupId: 500, name: '都営大江戸線' }, | ||
| autoModeEnabled: true, | ||
| stationForHeader: null, | ||
| arrived: false, | ||
| approaching: false, | ||
| isLEDTheme: false, | ||
| fetchedTrainTypes: [{ id: 10, groupId: 500, name: '都営大江戸線' }], | ||
| headerTransitionDelay: false, | ||
| targetAutoModeStation: null, | ||
| firstStop: true, | ||
| presetsFetched: false, | ||
| presetRoutes: [], | ||
| }; | ||
|
|
||
| (useAtom as jest.Mock).mockReturnValue([ | ||
| staleNavigationState, | ||
| mockSetNavigationState, | ||
| ]); | ||
|
|
||
| // handleLineSelected で fetch 前に行われるリセットをシミュレート | ||
| mockSetNavigationState((prev: typeof staleNavigationState) => ({ | ||
| ...prev, | ||
| fetchedTrainTypes: [], | ||
| trainType: null, | ||
| pendingTrainType: null, | ||
| })); | ||
|
|
||
| const updateFunction = mockSetNavigationState.mock.calls[0][0]; | ||
| const updatedState = updateFunction(staleNavigationState); | ||
|
|
||
| expect(updatedState.fetchedTrainTypes).toEqual([]); | ||
| expect(updatedState.trainType).toBeNull(); | ||
| expect(updatedState.pendingTrainType).toBeNull(); | ||
| }); | ||
|
|
||
| it('路線選択時に前回のpending駅データがクリアされる', () => { | ||
| const mockSetStationState = jest.fn(); | ||
| // 大江戸線を選択した後の状態を再現 | ||
| const staleStationState = { | ||
| station: { id: 1, name: '大門' }, | ||
| stations: [], | ||
| pendingStation: { id: 2, name: '六本木' }, | ||
| pendingStations: [ | ||
| { id: 2, name: '六本木' }, | ||
| { id: 3, name: '新宿西口' }, | ||
| ], | ||
| selectedDirection: 'INBOUND', | ||
| wantedDestination: null, | ||
| selectedBound: { id: 4, name: '光が丘' }, | ||
| stationsCache: [], | ||
| }; | ||
|
|
||
| (useAtom as jest.Mock).mockReturnValue([ | ||
| staleStationState, | ||
| mockSetStationState, | ||
| ]); | ||
|
|
||
| // handleLineSelected で fetch 前に行われるリセットをシミュレート | ||
| mockSetStationState((prev: typeof staleStationState) => ({ | ||
| ...prev, | ||
| pendingStation: null, | ||
| pendingStations: [], | ||
| selectedDirection: null, | ||
| wantedDestination: null, | ||
| selectedBound: null, | ||
| })); | ||
|
|
||
| const updateFunction = mockSetStationState.mock.calls[0][0]; | ||
| const updatedState = updateFunction(staleStationState); | ||
|
|
||
| expect(updatedState.pendingStation).toBeNull(); | ||
| expect(updatedState.pendingStations).toEqual([]); | ||
| expect(updatedState.selectedDirection).toBeNull(); | ||
| expect(updatedState.wantedDestination).toBeNull(); | ||
| expect(updatedState.selectedBound).toBeNull(); | ||
| // 現在の最寄り駅は変更されないことを確認 | ||
| expect(updatedState.station).toEqual({ id: 1, name: '大門' }); | ||
| }); | ||
|
|
||
| it('リセット後に新しい路線データで上書きされる', () => { | ||
| const mockSetStationState = jest.fn(); | ||
| // リセット済みの状態 | ||
| const resetState = { | ||
| station: { id: 1, name: '大門' }, | ||
| stations: [], | ||
| pendingStation: null, | ||
| pendingStations: [], | ||
| selectedDirection: null, | ||
| wantedDestination: null, | ||
| selectedBound: null, | ||
| stationsCache: [], | ||
| }; | ||
|
|
||
| (useAtom as jest.Mock).mockReturnValue([resetState, mockSetStationState]); | ||
|
|
||
| // fetch 完了後に行われる状態更新をシミュレート | ||
| const newPendingStation = { | ||
| id: 5, | ||
| name: '大門', | ||
| line: { id: 200, name: '都営浅草線' }, | ||
| }; | ||
| const newPendingStations = [ | ||
| { id: 5, name: '大門' }, | ||
| { id: 6, name: '新橋' }, | ||
| ]; | ||
| mockSetStationState((prev: typeof resetState) => ({ | ||
| ...prev, | ||
| pendingStation: newPendingStation, | ||
| pendingStations: newPendingStations, | ||
| })); | ||
|
|
||
| const updateFunction = mockSetStationState.mock.calls[0][0]; | ||
| const updatedState = updateFunction(resetState); | ||
|
|
||
| expect(updatedState.pendingStation).toEqual(newPendingStation); | ||
| expect(updatedState.pendingStations).toEqual(newPendingStations); | ||
| // リセット済みのフィールドは null/空のまま（fetch 結果では上書きされない） | ||
| expect(updatedState.selectedDirection).toBeNull(); | ||
| expect(updatedState.selectedBound).toBeNull(); | ||
| }); |
Contributor
There was a problem hiding this comment.
実装コードを実行していないため、テストが実質的に自己検証になっています
mockSetNavigationState/StationState に渡す更新関数をテスト側で作って直接評価しているので、実装側がリセット処理を行っていなくてもテストが通ります。SelectLineScreen の実処理（例: ルート選択イベントやハンドラ）を発火させ、そこで渡された更新関数の結果を検証する形にしてください。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/screens/SelectLineScreen.test.tsx` around lines 627 - 753, The tests
currently construct and invoke the state updater themselves, so they pass even
if SelectLineScreen's handler doesn't reset state; instead render/mount
SelectLineScreen and trigger the actual line-selection action (call the handler
via user event e.g., fireEvent.press on the line item or invoke the component
prop that calls handleLineSelected) so the component supplies the updater to
mockSetNavigationState/mockSetStationState; then grab the updater passed by the
component (mockSetNavigationState.mock.calls[0][0] or
mockSetStationState.mock.calls[0][0]) and apply it to the stale state to assert
fetchedTrainTypes/trainType/pendingTrainType and
pendingStation/pendingStations/selectedDirection/wantedDestination/selectedBound
were cleared before fetch and that subsequent fetch updates (simulate fetch
completion) overwrite only intended fields; ensure you no longer call mockSet...
with an inline updater in the test itself but simulate the UI event that
triggers SelectLineScreen.handleLineSelected.
Member
Author
|
#5368 で |
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
1 participant
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.
#5352 の対応漏れ
Summary by CodeRabbit