見出し画像

note flaky spec 対処の事例

noteのサーバーサイドRailsではRspecでテストを書いています。

みなさんはこういうことよく起きませんか?

「月が変わった最初の営業日テストが落ちる」

月またいた処理の考慮が漏れていて、またタイムゾーンの考慮漏れなど、
いわゆる
「昨日動いてたテストが落ちる」
「5回に1回くらいの確率で落ちる」
というパターンですね。

基本的に問題なさそうであれば、テストをリランして通す場合もありますが、デリバリーの足を鈍らせ、品質に影を残すため、noteでは度々このflaky(不安定な) specを撲滅すべく委員会が立ち上がります。

flaky specの原因は単純なロジック不良に依存しない場合が多く、非常に難易度が高かったり、RubyやRailsといった環境に対するマニアックな知見の宝庫だったりします。

よって、今回はその事例をいくつか紹介したいと思います。

なお、noteのRspecは概ね一般的なgem構成をしています

  • faker

  • factory_bot

  • database_cleaner 

  • など……

不安定なRspecの対処事例 〜 flaky spec 撲滅委員会活動報告

ケース① 順序保証の無いデータ

let(:note1) { create(:note) }
let(:note2) { create(:note) }
let(:note3) { create(:note) }

notes = Note.all.order(publish_at: :desc)
expect(notes.first.id).to eq(note1.id)

こんな感じです。というflaky testの例です。

このテストケースでは実行順やpublish_atが同一になることによって、順序保証がありません。

+    let(:note1) { create(:note, publish_at: Time.current) }
+    let(:note2) { create(:note, publish_at: 1.day.ago) }
+    let(:note3) { create(:note, publish_at: 2.day.ago) }

publish_atを指定して結果を固定するだけですね。

ケース② スタブが別のところで悪さしていた

allow(File).to receive(:open).and_return('content')

組み込みライブラリのスタブは本コード以外のいずこで使われているかわからないため避けましょう。

ケース③ マルチスレッド問題

database_cleaner gemを利用すると、データベースはトランザクションで処理され、ケースごとにbegin & rollbackされます。これによりゴミデータが残るのを防いだり、並列処理によるテストデータの干渉を防ぐことができます。

Rubyは基本的にシングルスレッドで動きますが、本処理内でforkやparallel gemを使った並列処理を行うとdatabase_cleanerのトランザクションと異なるセッションとなり参照できなくなります。database_cleanerは別途データをcommit & destroyするモードも提供していますが、これを利用した場合、そのケースを実行しているタイミングで、他のテストケースを並列で動かすとデータの干渉が発生します。

Parallel.each(ids, in_processes: Rails.env.test? ? 0 : 10) do |id|
  ...
end

Rails.env == testの場合、マルチスレッド処理を無効化するなどで対処する。

ケース④ database_cleanerが貼ったトランザクションが吹き飛んでいた

database_cleanerが貼ったトランザクションが吹き飛んでいた。

結論から言うと、

raise ActiveRecord::Rollback

ActiveRecord::Base.transaction do ~ end のブロックの外で実行されていたのが原因だった(もとは囲まれていたが修正の際に消し忘れ)

もともとActiveRecordはネストされたトランザクションをサポートしていて、database_cleanerはテストケースごとに大枠でトランザクションを貼っているイメージになる。そのため本処理でトランザクションが貼られていないRollbackが発行されると、database_cleanerのトランザクションが剥がれ、以降の処理で作成されたデータはCommitされる。それにより別のテストケースに干渉してエラーになるという結果だった(エラーになるのは別のテストケースなため、非常にたちが悪い)

なお、ActiveRecordは(MySQLにおいて)トランザクションのネストをSAVEPOINTを利用して実現している。

ケース⑤ 1つのファイルに2つのクラスを定義していた

Railsでこんな記述をしたことがあるだろうか?

require "notes_controller"

わたしはありません。

通常のRubyプログラムでは requireを実行し、依存関係にあるプログラムを読み込みますが、Railsではオートロードという仕組みで特定のパス(/controllerや/model, /libなど)に存在するファイル名をクラス名に読み替えて自動読み込みをしています。

よって以下の場合、Clients::ApiClientは対象となるがClients::ApiErrorは対象とならず NameError: uninitialized constantとなる場合がある。

# /lib/clients/api_client.rb

module Clients
  class ApiError < StandardError; end

  class ApiClient
  end
end

しかし、オートロードの対象となるクラスが呼び出されたタイミングでもう一方が読み込まれることがほとんどなのでflaky specになる。

# /lib/clients/api_error.rb

module Clients
  class ApiError < StandardError; end
end

別のファイルに切り出すほうが安定。

ケース⑥ 標準ライブラリrequire漏れ

File.read(path).toutf8
=> NoMethodError: undefined method `toutf8'

String#toutf8は組み込みのStringではなく標準ライブラリkconvにて定義されるメソッドである。よって原則require 'kconv'が必要だが、ケース⑤にも合ったとおり、他のファイルでrequireされていると動いてしまう場合がある。Railsのような大きなアプリケーションで動いているとそれが顕著で、なんかどっかで読み込んでいるから動くというのが往々にある。しかし、rspecなど並列でランダムな順序で実行している場合、ごくまれに引っかかる。

Railsを使ってると忘れがちになるが、Rubyには3種類のライブラリがある。

  • 組み込みライブラリ ex. Array, Hash など

  • 標準(添付)ライブラリ ex. csv, json など

  • Gemライブラリ ex. kaminari, nokogiri など

組み込みライブラリ以外は本来requireが必要となるが、

  • 標準ライブラリ … Railsがなんか勝手に適当に読み込んでたりする

  • Gemライブラリ … bundlerが勝手に読み込んでくれる

ので注意が必要(だったりそうでもなかったりするので困ったものです)。

flaky spec の対応方法

ほかにもデータベースの設定だったり、note固有のものだったりたくさんのflaky specに対応してきた。再現性が高く、原因さえわかってしまえば対応はさほど難しいものでは無い。しかし、flaky specは概ね再現性が低く、原因が表出しにくいものが多い。

ローカルの開発環境で発生しうる場合、

100.times.each do
  it 'flaky spec' do
    ...
  end
end

とでもして、デバックすれば良いので、対処非常に簡単だが、CI環境および並列環境でしか発生しないケースは、

  • デバッグを仕込む

  • 発生するまで10回も100回もフル実行

  • 発生したアウトプットからさらなるデバッグを仕込む

  • 上記を繰り返す

というなかなかの労力を伴う作業となる。

よって、

  • 対象のソースコードを片っ端から読み込み怪しいところを探す

  • なんだかよくわからないアウトプットから原因を論理的に切り分ける

  • 想像力を膨らませて何が起きているかを推測する

  • 持ちうるRubyやRailsの知識を総動員する

  • 諦めて対象のケースをskipする

というのが(正直あまり参考にならない)対応方法となる。

flaky spec を対応することの意義

テストはテストである。極端なことをいうならば無くても成立するものであるが、noteではテストを大事にしているし、わたしはflaky specを積極的に修正している。理由は、

  • 開発チーム全体の生産性に関わる

という名目を持ちながら、

  • 非常にイレギュラー性を持っていて、新しい知見が得られる

  • 究極言ってしまえばテストなので気楽にコミットできる(障害起こさない)

  • 原因が判明したときの気持ちよさがはんぱない

というのが本心だったりもする。

さあ、みなさんテスト書いていきましょう。

いいなと思ったら応援しよう!