テストのポーリングループを `wait_for` ヘルパーに統一
ActiveRecordのテストに残っていたアドホックなポーリングループを wait_for ヘルパーに置き換え、タイムアウト時に明示的なエラーを発生させるようにしました。これにより、ハング時のデバッグが困難だった問題とサイレントなタイムアウトが解消されます。
背景
#57309 が connection_pool_test.rb の無限ループを修正したことを受け、同様の問題を抱える残りのテストを統一的に修正するフォローアップです。
#57309 では、test_checkout_fairness_by_group がCIで無限にハングする事象が報告されていました。スレッドが正常・エラーのいずれのパスにも到達しない場合、loop { sleep 0.1; break if ... } が永久に回り続け、ビルドエージェントのジョブタイムアウトで強制終了されるまで CI が詰まる状態でした。
今回対象となった3箇所のポーリングループには、それぞれ異なる問題がありました。reaper_test.rb の2箇所は until ... { Thread.pass } という上限なしのビジーウェイトで、条件が満たされなければ永久にハングします。asynchronous_queries_test.rb の1箇所は 500.times { sleep 0.02 } という10秒上限付きのループでしたが、タイムアウト後も例外を発生させずにそのまま処理を続行するため、後続のアサーションが誤った結果を出力する可能性がありました。これらのパターンの共通の問題は、失敗の根本原因(条件が満たされなかったこと)とアサーションの失敗箇所が分離してしまう点にあります。
技術的な変更
3箇所のポーリングループを wait_for ヘルパーに置き換え、タイムアウト時に Timeout::Error を発生させるよう統一しました。
reaper_test.rb では上限なしのビジーウェイトが2箇所修正されています。
変更前:
until fp.flushed
Thread.pass
end
変更後:
wait_for(message: "fake pool was not flushed") { fp.flushed }
asynchronous_queries_test.rb では、サイレントにタイムアウトするポーリングが修正されています。
変更前:
def wait_for_future_result(result)
500.times do
break unless result.pending?
sleep 0.02
end
end
変更後:
def wait_for_future_result(result)
wait_for(message: "future result still pending", timeout: 10, interval: 0.02) { !result.pending? }
end
いずれのファイルも、ActiveRecord::TestCase::WaitForTestHelper を include することで wait_for ヘルパーを利用可能にしています。reaper_test.rb 内の wait_for_conn_idle メソッドは既存の timeout 引数を受け取る実装になっており、wait_for の timeout: キーワード引数にそのまま委譲する形で書き直されています。
設計判断
wait_for ヘルパーへの一元化が採用された背景には、失敗箇所を正確に特定するという明確な目標があります。
アドホックなポーリングパターンはそれぞれ異なる欠陥を持つため、テストが失敗したときの挙動も一貫しません。wait_for を共通インターフェースにすることで、タイムアウト時の動作(Timeout::Error を発生させ、message: でコンテキストを示す)が統一されます。これにより、ハングではなくテスト失敗として扱われ、CIログで原因箇所を即座に特定できます。段階的なリファクタリングを選ぶことで、変更範囲を最小限に抑えながら最も危険な「無限ハング」の問題を解消しています。
まとめ
本PRは、テストの信頼性を損なうアドホックなポーリングパターンを wait_for ヘルパーに統一することで、CIのハングとサイレントな誤検知の両方を排除しました。タイムアウト時に Timeout::Error が実際の失敗箇所で発生するようになったことで、テストの失敗がより診断しやすくなっています。