weblog of key_amb

主にIT関連の技術メモ

RSpec で example の外で定義したローカル変数を使うのはアリか?

※9/3 @jnchito さんのコメントを受けて追記しました。

RSpec で example の外で定義したローカル変数を使う

テストコードの例

こういうの:

outside_var = :outside_var # (1)

describe :top_scope do
  top_scope_var = :top_scope_var # (2)

  it :example_in_top_scope do
    expect(outside_var).to be_truthy
    expect(top_scope_var).to be_truthy
  end

  context :second_scope do
    second_scope_var = :second_scope_var # (3)

    it :example_in_second_scope do
      expect(outside_var).to be_truthy
      expect(top_scope_var).to be_truthy
      expect(second_scope_var).to be_truthy
    end
  end
end

これまでのところ、他人が書いたコードでこういう書き方を見たことはなかった。
外側で定義した変数を使う場合、 let を使うか before ブロックでインスタンス変数に代入するのがふつうだろう。

が、上のような書き方でも、ふつうに期待通りに動く。

特徴・用途

ローカル変数を使う場合には、let を使って宣言する場合や beforeインスタンス変数を使う場合と比較すると、以下のような特徴がある。

  • 変数定義は1回だけ実行される 〜 before :context 相当
    • letbefore :each では、example ごとに実行される
    • describe ブロックが評価され、RSpec の ExampleGroup が作られる時点で実行されているようだ
  • スコープが有効
    • 上の例で、 :top_scope から second_scope_var は見えない
    • let で宣言する変数や before で定義するインスタンス変数と同様
  • let 同様、typo に気づける。インスタンス変数だとエラーにならないので、気づかない恐れが有る(参考記事を参照)

RSpec の内部構造に踏み込んで深く追ったことはないのだけど、挙動を確認した限り、上のようだった。

ローカル変数のユースケースとしては、一々毎回実行・評価しなくていいような性質のもので、テスト中に状態が変わらないものに使えそうだと考えている。リファクタ用途としても使えそうだ。

「アリ」なのか、「ナシ」なのか?

自分自身としては、何ヶ月か前 fireap *1などを作っていた頃は、RSpec の作法をよく知らなかったので、ローカル変数を多用していた。

しかし、後になって RSpec について色々調べている内に、このような書き方を全く見かけないことに気がついた。
とはいえ、逆に「こういうのは駄目だ」とはっきり書いてある記事やドキュメントもなかった。

…ので、こういう書き方がアリなのかナシなのかというのが、数カ月ぐらい気になっていた。

で、最近初めて自分以外の人でそういう書き方をしている人を見かけたので、気になり度がしきい値を突破して、とある Rubyist が集うチャンネルで聞いてみた。


私「RSpec 詳しい人いますかね? example の外でローカル変数定義して使うのってアリなのかなぁというのが気になってます。こんなの↑(上のようなコード)」

S氏「1. rspecdsl で書くと、ほらわかりやすいでしょ?というのが走りなので、dsl を使わなくなったらなぜ rspec を使うのか、という話になりそう。
 2. let はオプションで thread-safe にしたり thread non-safe にしたり切り替えられるはず。
 3. let は それぞれの it の中で、それぞれ呼ばれるのでちょっと動きが違う。
 ぱっと思いつくやつ↑」

O氏「

describe "parameterized test" do
  [ 5, 10, 15 ].each do |i|
   it { expect(fizzbuzz(i)).to eq("buzz") }
  end
end

 みたいなパラメタライズドテストを rspec-parameterized みたいなのをかかなかったら普通にそうなってるので、特に気にしないでやればいいんじゃないかな?」


確かに、そういう書き方をしたこともあった。
ってか、 rspec-parameterized 知らなかった。

2016/9/2 現時点の結論

…というわけで、今のところ自分の中では「アリ」ということになっている。

ただ、テスト実行中に変数の状態が変わったりしたら、なんかまずそうなので、定数にしたり freeze した方が安全かも、と思ったり思わなかったり。

どうなんでしょうね。

(9/3 追記)

@jnchito さんが、本件についての見解・回答を示すブログを書かれています。

結論だけ書くと「興味深いけど積極的に使うのは NG」という感じです。
興味のある方は是非、上の記事もご覧ください。

9/3 追記:ローカル変数が使えそうな例

なんと、参考記事等でおなじみの @jnchito さんからコメントをいただきました。
正直、Qiita のコメントで聞いてみようかと思ったぐらいなので、僥倖でした。ブログ書いてよかった。

「絶対ローカル変数が良い、というサンプルコードがあれば教えてください」

とのこと。

「絶対」ということはないかな、と思いますが、「まあ、こういうケースならアリかな(?)」と個人的に思う例を3つほど書いておきます。

(1) 長いメソッド等のリファクタ系

特に、実行結果が毎回変わらないものがよさそうです。

describe MyReservationsController do
  in_sale = Reservation.statuses[:in_sale]
  reserved = Reservation.statuses[:reserved]
  cancelled = Reservation.statuses[:cancelled]
  out_of_sale = Reservation.statuses[:out_of_sale]

  let(:client) { FactoryGirl.create(:client) }
  let(:parsed_response_body) { JSON.parse(response.body) }
  
  describe '#index' do
    context '販売中のもの'
      context 'ログイン前' do
        it '予約が見つからない' do
          get :index, status: in_sale, count: 10
          expect(parsed_response_body['Reservations']).to be nil
        end
      end
      context 'ログイン後' do
        before do
          client.login
        end
        it '販売中の予約が取得できる' do
          get :index, status: in_sale, count: 10
          parsed_response_body['Reservations'].each do |r|
            expect(r['status'])
          end
        end
      end
      context 'メンテナンス中' do
        before do
          expect(MyService).to receive(:maintenance?).and_return(true)
        end
        it '予約が見つからない' do
          get :index, status: in_sale, count: 10
          expect(parsed_response_body['Reservations']).to be nil
        end
      end
    end
  end
end

もちろん、 let で書けないことはないし、↑の例だと get ... をまとめて subject 化するのもアリと思います。

(2) セットアップに時間のかかるオブジェクトを1回だけ生成し、結果を利用したい

下はプリミティブな DB のコネクション・ハンドラを自作しているような前提です。

conn = MyDatabaseConnector.setup!

describe MyTransactionApp do
  context 'normal' do
    it do
      Foo.query(conn, type: :normal)
      :
    end
  end
  context 'strange' do
    it do
      Foo.query(conn, type: :strange)
      :
    end
  end
end

RSpec の流儀に合いそうな別解としては、下のように書き換えられることもあるかもしれません:

describe MyTransactionApp do
  let(:conn) { MyDatabaseConnector.connection }

  before :all do
    MyDatabaseConnector.setup!
  end
  
  :
end

あるいは、 helper module にできるケースもあるかもしれません。

(3) 時間がかかるわけでもないが、1回だけセットアップして結果を利用したい

target_path = Foo.method_to_get_path(some_arguments, ...)
tmp_path = Pathname.new(Dir.tmp_dir) + 'foo'

describe MyApp do
  before :all do
    # 対象ファイルがあれば退避
    if File.readable?(target_path) do
      FileUtils.move(target_path, tmp_path)
    end
  end

  after :all do
    # 対象ファイルを復帰
    if File.readable?(tmp_path) do
      FileUtils.move(tmp_path, target_path)
    end
  end
  
  context 'A' do
    before do
      File.write(target_path, 'A')
    end
    after do
      FileUtils.remove(target_path)
    end
    
    it 'target_path の内容に依存するテスト' do
      :
    end
  end

  context 'B' do
    before do
      File.write(target_path, 'B')
    end
    after do
      FileUtils.remove(target_path)
    end
    
    it 'target_path の内容に依存するテスト' do
      :
    end
  end
end

この場合の別解としては、 target_path を helper method にしたり、 tmp_path をインスタンス変数にして before :all でセットする、というやり方もできそうです。

参考