Conversation
WalkthroughシステムテストのTagify操作をヘルパー呼び出しへ置換し、テストヘルパーは固定sleepを廃止してタグ数やhidden入力の変化を待つ同期に変更。いくつかの冗長なタグアサーション削除、メール通知テスト内のメッセージ検証タイミング調整、承認スタンプ日付アサーションを修正。 Changes
Sequence Diagram(s)sequenceDiagram
participant Test as システムテスト
participant Helper as fill_in_tag ヘルパー
participant UI as ブラウザ DOM (.tagify / hidden input)
Note over Test,Helper: ヘルパー駆動の同期フロー（変更点）
Test->>Helper: fill_in_tag("追加タグ")
Helper->>UI: 現在の .tagify__tag 数を取得
Helper->>UI: input をクリック・テキスト入力・Enter送信
UI-->>Helper: DOM 更新（.tagify__tag が増加）
alt hidden input present
UI-->>Helper: `input[name="tag_list"]` に新タグが反映（最大10秒待機）
end
Helper-->>Test: assert_selector で増加と（必要なら）hidden入力の値を確認
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 注意点:
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (6)
💤 Files with no reviewable changes (1)
🧰 Additional context used📓 Path-based instructions (2)**/*.rb⚙️ CodeRabbit configuration file
Files:
test/**/*⚙️ CodeRabbit configuration file
Files:
🧠 Learnings (3)📓 Common learnings📚 Learning: 2025-07-04T01:28:44.174ZApplied to files:
📚 Learning: 2025-08-31T12:08:33.904ZApplied to files:
🧬 Code graph analysis (2)test/system/movie/tags_test.rb (1)
test/system/question/tags_test.rb (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (6)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 RuboCop (1.81.7)test/components/product/product_component_test.rbrubocop-minitest extension supports plugin, specify ... [truncated 255 characters] ... fig_loader_resolver.rb:76:in test/supports/tag_helper.rbrubocop-minitest extension supports plugin, specify ... [truncated 255 characters] ... fig_loader_resolver.rb:76:in test/system/mail_notification_test.rbrubocop-minitest extension supports plugin, specify ... [truncated 255 characters] ... fig_loader_resolver.rb:76:in
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
test/system/movie/tags_test.rb(1 hunks)test/system/question/tags_test.rb(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.rb
⚙️ CodeRabbit configuration file
**/*.rb: # refactoring
- まずFat Controllerを避け、次にFat Modelを避ける。
- Serviceクラスの乱用を避ける。
- controller concernを作ろうとしたらPORO(Plain Old Ruby Object)やActiveRecordモデルでの実装で代替できないか検討する。
Rails Patterns
- ViewHelperにメソッドを実装する時にはまずDecoratorパターンを使うことを検討する。（active_decorator gemを導入しているのでそれを使う）
- 複雑なActiveRecordクエリがあり、再利用できそうな場合はQueryObjectパターンを検討する。（rails-patterns gemを導入しているのでそれのQuery機能を使う）
- Viewにpartialを作る場合はViewComponentを使うことを検討する。
- 複数のActiveRecordモデルを操作する１つの責務がある時や外部APIとやりとりする処理がある場合にはInteractorオブジェクトパターンを検討する。（interactor gemを導入しているのでそれを使う）
- 複数のInteractorを実行するような処理がある場合Organizerオブジェクトパターンを検討する。（interactor gemを導入しており、その中にOrganizerの機能があるので使う）
Files:
test/system/question/tags_test.rbtest/system/movie/tags_test.rb
test/**/*
⚙️ CodeRabbit configuration file
test/**/*: # Test
- TestCase名は英語で書く。
- どうしても避けられない時以外にsystem testでsleepは使わない。
Unit Test
model, helper, decorator, view_componentについてはメソッドを追加した場合は必ず対応したUnit TestのTestCaseを1つは書く。
Files:
test/system/question/tags_test.rbtest/system/movie/tags_test.rb
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build_and_test
🔇 Additional comments (1)
test/system/movie/tags_test.rb (1)
20-20: LGTM! 一貫したリファクタリングでテストの品質が向上しています
fill_in_tagヘルパーメソッドの使用により、test/system/question/tags_test.rbと同様にTagify操作が抽象化されています。複数のテストファイルで一貫したアプローチを採用することで、保守性とテストの安定性が全体的に向上します。
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
test/supports/tag_helper.rb (1)
18-18: アラート発生時の検証が最小限です。
assert_selector(selector)はセレクタの存在のみを確認していますが、アラートが表示された場合の期待動作（例：タグが追加されないこと）を明示的に検証していません。テストの意図に応じて、より具体的なアサーションの追加を検討してください。
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
test/supports/tag_helper.rb(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.rb
⚙️ CodeRabbit configuration file
**/*.rb: # refactoring
- まずFat Controllerを避け、次にFat Modelを避ける。
- Serviceクラスの乱用を避ける。
- controller concernを作ろうとしたらPORO(Plain Old Ruby Object)やActiveRecordモデルでの実装で代替できないか検討する。
Rails Patterns
- ViewHelperにメソッドを実装する時にはまずDecoratorパターンを使うことを検討する。（active_decorator gemを導入しているのでそれを使う）
- 複雑なActiveRecordクエリがあり、再利用できそうな場合はQueryObjectパターンを検討する。（rails-patterns gemを導入しているのでそれのQuery機能を使う）
- Viewにpartialを作る場合はViewComponentを使うことを検討する。
- 複数のActiveRecordモデルを操作する１つの責務がある時や外部APIとやりとりする処理がある場合にはInteractorオブジェクトパターンを検討する。（interactor gemを導入しているのでそれを使う）
- 複数のInteractorを実行するような処理がある場合Organizerオブジェクトパターンを検討する。（interactor gemを導入しており、その中にOrganizerの機能があるので使う）
Files:
test/supports/tag_helper.rb
test/**/*
⚙️ CodeRabbit configuration file
test/**/*: # Test
- TestCase名は英語で書く。
- どうしても避けられない時以外にsystem testでsleepは使わない。
Unit Test
model, helper, decorator, view_componentについてはメソッドを追加した場合は必ず対応したUnit TestのTestCaseを1つは書く。
Files:
test/supports/tag_helper.rb
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build_and_test
🔇 Additional comments (2)
test/supports/tag_helper.rb (2)
4-10: sleepの削除とassert_selectorによる同期は良い改善です。FLAKYテストの原因となる
sleepを削除し、assert_selectorで明示的に待機する実装に変更したことで、テストの安定性が向上します。コーディングガイドラインの「どうしても避けられない時以外にsystem testでsleepは使わない」に準拠しています。
5-9: 複数のTagifyインスタンスに関する懸念は根拠がありません。コードベース全体のシステムテスト（8ファイル以上）を確認した結果：
- すべてのテストで
find('.tagify__input')またはfirst('.tagify__input')を使用して、ページごとに1つのTagifyインスタンスを前提としている- グローバルな
.tagify__tagセレクタを使用したアサーションはテスト側でも同じパターン- 複数のTagifyインスタンスが存在するページは確認できない
- 現在の実装でテストは成功している
タグカウントのスコープ限定は不要です。
ただし、
fill_in_tag_with_alertメソッドのアサーション（assert_selector(selector)）は、ページの安定性の観点からさらに強化することは検討の価値があります。例えば、タグが追加されなかったことを明示的に検証する方が意図的です。Likely an incorrect or invalid review comment.
machida
force-pushed
the
fix-tags-test
branch
from
34b008b to
535a32e
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
test/supports/tag_helper.rb (1)
18-18: より具体的なアサーションを検討してください
assert_selector(selector)は要素の存在のみを確認していますが、アラート処理が成功したこと、またはTagifyが適切に初期化されていることを保証していません。アラート処理後の期待される状態（例：タグの数、特定のTagify属性の存在など）をより具体的に検証することを検討してください。
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
test/supports/tag_helper.rb(1 hunks)test/system/article/tags_test.rb(0 hunks)test/system/movie/tags_test.rb(1 hunks)test/system/question/tags_test.rb(1 hunks)
💤 Files with no reviewable changes (1)
- test/system/article/tags_test.rb
🚧 Files skipped from review as they are similar to previous changes (1)
- test/system/movie/tags_test.rb
🧰 Additional context used
📓 Path-based instructions (2)
**/*.rb
⚙️ CodeRabbit configuration file
**/*.rb: # refactoring
- まずFat Controllerを避け、次にFat Modelを避ける。
- Serviceクラスの乱用を避ける。
- controller concernを作ろうとしたらPORO(Plain Old Ruby Object)やActiveRecordモデルでの実装で代替できないか検討する。
Rails Patterns
- ViewHelperにメソッドを実装する時にはまずDecoratorパターンを使うことを検討する。（active_decorator gemを導入しているのでそれを使う）
- 複雑なActiveRecordクエリがあり、再利用できそうな場合はQueryObjectパターンを検討する。（rails-patterns gemを導入しているのでそれのQuery機能を使う）
- Viewにpartialを作る場合はViewComponentを使うことを検討する。
- 複数のActiveRecordモデルを操作する１つの責務がある時や外部APIとやりとりする処理がある場合にはInteractorオブジェクトパターンを検討する。（interactor gemを導入しているのでそれを使う）
- 複数のInteractorを実行するような処理がある場合Organizerオブジェクトパターンを検討する。（interactor gemを導入しており、その中にOrganizerの機能があるので使う）
Files:
test/supports/tag_helper.rbtest/system/question/tags_test.rb
test/**/*
⚙️ CodeRabbit configuration file
test/**/*: # Test
- TestCase名は英語で書く。
- どうしても避けられない時以外にsystem testでsleepは使わない。
Unit Test
model, helper, decorator, view_componentについてはメソッドを追加した場合は必ず対応したUnit TestのTestCaseを1つは書く。
Files:
test/supports/tag_helper.rbtest/system/question/tags_test.rb
🧠 Learnings (2)
📓 Common learnings
Learnt from: e-yanagita-gs
Repo: fjordllc/bootcamp PR: 8881
File: app/javascript/transform-head-sharp.js:1-8
Timestamp: 2025-07-04T01:28:44.174Z
Learning: Tagifyライブラリは空文字列のタグを自動的に追加しないという仕様があるため、変換処理で空文字列を生成することで不要なタグの追加を防ぐことができる。
Learnt from: masyuko0222
Repo: fjordllc/bootcamp PR: 8277
File: test/models/searcher_test.rb:0-0
Timestamp: 2025-07-07T12:46:01.650Z
Learning: テストにおいて、`result.all? { |condition| }` でコレクションの全要素が条件を満たすことを確認できれば、その条件を満たさない要素が含まれていないことも論理的に保証されるため、追加のアサーションは冗長となる。
📚 Learning: 2025-07-04T01:28:44.174Z
Learnt from: e-yanagita-gs
Repo: fjordllc/bootcamp PR: 8881
File: app/javascript/transform-head-sharp.js:1-8
Timestamp: 2025-07-04T01:28:44.174Z
Learning: Tagifyライブラリは空文字列のタグを自動的に追加しないという仕様があるため、変換処理で空文字列を生成することで不要なタグの追加を防ぐことができる。
Applied to files:
test/system/question/tags_test.rb
🧬 Code graph analysis (1)
test/system/question/tags_test.rb (1)
test/supports/tag_helper.rb (1)
fill_in_tag(4-10)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build_and_test
🔇 Additional comments (2)
test/system/question/tags_test.rb (1)
20-20: LGTM! ヘルパーメソッドの使用でテストの安定性が向上しています
fill_in_tagヘルパーの使用により、Tagifyの初期化待ちが適切に処理され、直接的なDOM操作が抽象化されています。これによりテストの保守性と安定性が向上し、FLAKYテストの問題が解決されます。test/supports/tag_helper.rb (1)
5-9: このレビューコメントは却下してください検証の結果、このレビューコメントで指摘されている「複数のTagifyインスタンスがページに存在する場合の問題」は、実装コードベースに存在しません。
実装上の根拠：
article/tags_test.rbでは既に2つのタグを連続追加するテストが実装されており、正常に動作しています- 各
fill_in_tag呼び出しでタグ数を前後比較してアサートしており、連続追加時も正確に検証できています- 実際のコードベース内でページ上に複数の独立したTagifyインスタンスが存在する場面は見つかりません
- 報告されているテストの不安定性や失敗もありません
現在の実装は、実際の使用パターンに対して正確に機能しており、問題はありません。
Likely an incorrect or invalid review comment.
machida
force-pushed
the
fix-tags-test
branch
2 times, most recently
from
8b9fa8b to
5b5180b
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
test/supports/tag_helper.rb (1)
11-13: React コンポーネント向けの hidden input 検証についてhidden input の値検証に部分一致
[value*='#{name}']を使用していますが、理論的にはタグ名が他のタグの部分文字列である場合に誤マッチの可能性があります。ただし、acts-as-taggable-on がカンマ区切りで保存する仕様上、実用上は問題ありません。より厳密にしたい場合は、カンマや前後の空白を考慮したパターンマッチも検討できます。
より厳密なマッチングの例：
# カンマ区切りを考慮した検証（オプション） tag_pattern = "(^|, )#{Regexp.escape(name)}(, |$)" assert_selector("input[type='hidden'][name*='tag_list']", visible: :all, wait: 10) do |input| input.value.match?(/#{tag_pattern}/) end
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
test/supports/tag_helper.rb(1 hunks)test/system/article/tags_test.rb(0 hunks)test/system/movie/tags_test.rb(1 hunks)test/system/question/tags_test.rb(1 hunks)
💤 Files with no reviewable changes (1)
- test/system/article/tags_test.rb
🚧 Files skipped from review as they are similar to previous changes (1)
- test/system/question/tags_test.rb
🧰 Additional context used
📓 Path-based instructions (2)
**/*.rb
⚙️ CodeRabbit configuration file
**/*.rb: # refactoring
- まずFat Controllerを避け、次にFat Modelを避ける。
- Serviceクラスの乱用を避ける。
- controller concernを作ろうとしたらPORO(Plain Old Ruby Object)やActiveRecordモデルでの実装で代替できないか検討する。
Rails Patterns
- ViewHelperにメソッドを実装する時にはまずDecoratorパターンを使うことを検討する。（active_decorator gemを導入しているのでそれを使う）
- 複雑なActiveRecordクエリがあり、再利用できそうな場合はQueryObjectパターンを検討する。（rails-patterns gemを導入しているのでそれのQuery機能を使う）
- Viewにpartialを作る場合はViewComponentを使うことを検討する。
- 複数のActiveRecordモデルを操作する１つの責務がある時や外部APIとやりとりする処理がある場合にはInteractorオブジェクトパターンを検討する。（interactor gemを導入しているのでそれを使う）
- 複数のInteractorを実行するような処理がある場合Organizerオブジェクトパターンを検討する。（interactor gemを導入しており、その中にOrganizerの機能があるので使う）
Files:
test/supports/tag_helper.rbtest/system/movie/tags_test.rb
test/**/*
⚙️ CodeRabbit configuration file
test/**/*: # Test
- TestCase名は英語で書く。
- どうしても避けられない時以外にsystem testでsleepは使わない。
Unit Test
model, helper, decorator, view_componentについてはメソッドを追加した場合は必ず対応したUnit TestのTestCaseを1つは書く。
Files:
test/supports/tag_helper.rbtest/system/movie/tags_test.rb
🧠 Learnings (1)
📓 Common learnings
Learnt from: Miya096jp
Repo: fjordllc/bootcamp PR: 9102
File: app/controllers/mentor/buzzes_controller.rb:0-0
Timestamp: 2025-09-04T01:39:22.261Z
Learning: fjordllc/bootcamp プロジェクトでは、更新処理の成功時の通知メッセージについて、I18n を使用せずに日本語文字列を直接記述する方針で一貫性を保っている。
Learnt from: e-yanagita-gs
Repo: fjordllc/bootcamp PR: 8881
File: app/javascript/transform-head-sharp.js:1-8
Timestamp: 2025-07-04T01:28:44.174Z
Learning: Tagifyライブラリは空文字列のタグを自動的に追加しないという仕様があるため、変換処理で空文字列を生成することで不要なタグの追加を防ぐことができる。
🧬 Code graph analysis (1)
test/system/movie/tags_test.rb (1)
test/supports/tag_helper.rb (1)
fill_in_tag(4-14)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build_and_test
🔇 Additional comments (3)
test/supports/tag_helper.rb (2)
5-9: タグ追加の同期処理が適切に実装されています固定sleepを廃止し、タグ数の変化を待つ明示的な同期に変更されています。
visible: :allを使用することで、非表示のタグ要素も正しくカウントされます。10秒の待機時間も十分です。コーディングガイドラインに従い、system testでのsleep使用を避ける方針に沿っています。
16-23: アラート処理を伴うタグ追加が改善されていますLine 18 で input をクリックしてから入力することで、フォーカス状態が確実になります。Line 22 のアラート後のアサーションも、要素が期待通りの状態にあることを確認する適切な防御的コーディングです。
test/system/movie/tags_test.rb (1)
17-23: ヘルパーを使用したテストの可読性向上直接的な Tagify 入力操作を
fill_in_tagヘルパー呼び出しに置き換えることで、テストの意図が明確になり、保守性が向上しています。同期処理の詳細はヘルパー内にカプセル化され、テストコードがシンプルになりました。
fill_in_tagヘルパーでタグが正しく追加されないフレーキーを修正。 タグ要素の確認機能を追加し、sleepを削除してCapybaraの待機機能 を使用するように改善。clickを追加してフォーカスを確実にし、set からnative.send_keysに変更してReactの入力イベントを正しく処理 するようにした。React版ではhidden input更新を待機し、フォーム 送信時の値を確実に反映。冗長なアサーションを削除し、movie/ questionテストでヘルパーを統一的に使用。また、RuboCopの指摘に 従いガード節に変更。
machida
force-pushed
the
fix-tags-test
branch
from
194c5fb to
c5ea914
Compare
1 participant
Summary by CodeRabbit