Conversation
FLAKY なテストになっていたため、assert_selector 等を使用し、要素の状態を確認してから後続処理を実行するように修正
Walkthroughシステムテストのみの変更。 Changes
Sequence Diagram(s)Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
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.76.1)test/system/markdown_test.rbrubocop-minitest extension supports plugin, specify 📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (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)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
sjabcdefin
force-pushed
the
bug/only-pass-headful
branch
from
51a565e to
f90681c
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
test/system/markdown_test.rb (1)
7-8: 重複前処理のDRY化（軽微）同じ2行が2テストで重複しています。可読性向上のため軽くまとめるとよいです（どちらでも可）。
最小限の置換（変数を介さずに1行で呼び出す）:
- user = users(:mentormentaro) - reset_avatar(user) + reset_avatar users(:mentormentaro)両テストで使う小ヘルパーをこのクラスに追加する案（参考）:
private def reset_mentor_avatar reset_avatar users(:mentormentaro) end呼び出し側（各テスト内）は以下のように1行化できます:
reset_mentor_avatarAlso applies to: 24-25
test/system/page/tags_test.rb (1)
114-124: 同様の厳密さを「ドットのみ無効タグ」テストにも適用することを検討（任意）上のテストと同じく、無効入力後の
.tagify__tag不在と、page[tag_list]hidden inputの値確認まで行うと一貫性が高まります。test 'alert when enter one dot only tag on creation page' do visit_with_auth new_page_path, 'kimura' fill_in_tag_with_alert '.' + assert_no_selector('.tagify__tag') fill_in_tag 'foo' + assert_selector('input[name="page[tag_list]"][value="foo"]', visible: false) within('form[name=page]') do fill_in('page[title]', with: 'test title') fill_in('page[body]', with: 'test body') click_button 'Docを公開' end assert_equal ['foo'], all('.tag-links__item-link').map(&:text).sort end
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
test/system/markdown_test.rb(2 hunks)test/system/page/tags_test.rb(1 hunks)test/system/questions_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/markdown_test.rbtest/system/page/tags_test.rbtest/system/questions_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/markdown_test.rbtest/system/page/tags_test.rbtest/system/questions_test.rb
🧬 Code Graph Analysis (2)
test/system/markdown_test.rb (1)
test/supports/avatar_helper.rb (1)
reset_avatar(6-14)
test/system/questions_test.rb (1)
test/system/page/tags_test.rb (1)
test(5-151)
⏰ 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 (4)
test/system/markdown_test.rb (1)
7-8: ヘッドレス環境での安定化のための前処理追加は妥当ですフィクスチャからアバターを明示的にアタッチすることで、ネットワーク依存やデフォルト画像分岐による不安定さを排除できています。意図に合致しており良い修正です。
test/system/questions_test.rb (2)
127-127: 作成フロー（スペース含む無効タグ）の検証強化が適切です
- 無効入力後に
.tagify__tag不在を確認- 有効入力時に非表示のhidden inputへ確実に反映されていることを確認
- 投稿後にタグリンクの出現を件数まで厳密に検証
ヘッドレス差異を吸収できる堅牢なアサーションになっています。
Also applies to: 130-130, 136-136
142-142: 作成フロー（ドットのみ無効タグ）の検証強化も適切です無効入力のDOM不在確認、hidden inputの値確認、投稿後の件数検証まで一貫しており、ヘッドレスでも安定する構成です。
Also applies to: 144-144, 150-150
test/system/page/tags_test.rb (1)
102-102: ページ作成時のタグ入力検証をヘッドレスに強くした点は良いです
- 無効入力時の
.tagify__tag不在確認- 有効入力時のhidden inputの値確認（visible: false）
- 投稿後のタグリンク出現の件数検証
TagifyのUIと送信値の両面を押さえた堅牢なテストになっています。
Also applies to: 105-105, 111-111
|
@komagata 2点ご連絡があります。
|
test/system/markdown_test.rb
Outdated
| user = users(:mentormentaro) | ||
| reset_avatar(user) |
There was a problem hiding this comment.
| user = users(:mentormentaro) | |
| reset_avatar(user) | |
| reset_avatar(users(:mentormentaro)) |
1箇所でしか出てこないのであればこれでいいかもしれません。また、複数のテストケースで出てくるのであればsetupブロックで共有のユーザーとしてインスタンス変数に入れるのも良いかもしれません。
There was a problem hiding this comment.
コミット：6465df4
reset_avatar(users(:mentormentaro))に修正いたしました。
複数のテストケースで出てくるのであればsetupブロックで共有のユーザーとしてインスタンス変数に入れるのも良いかもしれません。
users(:mentormentaro)は他のテストにも多数出てくるため、 別のissueでリファクタリングとして実施したほうがよいと思い、上記修正としています。
とってもいいと思います！
でしたら修正は不要です〜 |
user が1箇所でしか出てこず、可読性向上のため
2 participants
Issue
概要
環境により以下のテストがヘッドレスだと失敗するため、ヘッドレスでもテストが通るように修正。
私のローカル環境では、修正前のテストでもヘッドレスで成功。そのため、テストが通らない環境をお持ちの @mousu-a さんにご協力いただき、修正後のテストが成功することを確認済み。
修正内容詳細
1. Error::UnexpectedAlertOpenError: unexpected alert open: {Alert text : already exists}
assert_no_selector('.tagify__tag')を追加し、不正タグが削除されたことを確認した上で正しいタグの入力が確実に行われるよう修正。2. Failure: Expected: ["foo"], Actual: []
"foo"の反映は完了しているものの、フォーム送信データinput[name="question[tag_list]"]への反映に時間がかかり、その過程で一時的に[登録する]ボタンが disabled の状態になり、クリックできていないことが原因と推測。assert_selector('input[name="question[tag_list]"][value="foo"]', visible: false)を追加し、フォーム送信データにタグが反映されていることを確認するよう修正。3.
assert_equalをassert_selectorに変更assert_equalはMinitestのメソッドであり、要素が出現するまで待機しない。assert_selectorに修正。その他
変更確認方法
bug/only-pass-headfulをローカルに取り込むrails test test/system/page/tags_test.rbrails test test/system/questions_test.rbrails test test/system/markdown_test.rbScreenshot
FLAKY なテストの修正であり、見た目の変更はないためスクリーンショットはありません。
Summary by CodeRabbit