5
0

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?

More than 3 years have passed since last update.

[RSpec]changeをandで複数繋げたときに順序の違いで成否が変わってハマった話

Last updated at Posted at 2021-06-28

タイトルの通り、changeをandで複数繋げたときに順序の違いで成否が変わってしまいハマってしまいました。
このときchangeの挙動にちょっとだけ詳しくなったので頭の整理も兼ねて記事にしてみました。

どのようなテストを書いたのか?

説明のため簡素化していますが、下記のようなテストを書いたときに発生しました。

Models

まずは対象のモデルです。
childを作成/削除するとparent.statusがupdateされるように実装されています。

# `status`カラムを持つ。default: 0
class Parent < ApplicationRecord
  has_one :child
end

class Child < ApplicationRecord
  belongs_to :parent

  after_create -> { parent.update!(status: 1) }
  after_destroy -> { parent.update!(status: 0) }
end

RSpec

childを削除したときのテストを下記に書きました。
changeで"parent.statusの変化"と"childが存在しなくなること"の2つを検証しています。

RSpec.describe Child, type: :model do
  let(:parent) { create(:parent) }

  describe 'destroy child' do
    subject { parent.child.destroy }

    let(:parent) { create(:parent) }
    let(:child) { create(:child, parent: parent) }

    # こちらは失敗する
    it 'statusが0に更新されること' do
      expect { subject }.to change { Child.exists?(id: child.id) }.from(true).to(false).
        and change { Parent.find(parent.id).status }.from(1).to(0)
    end

    # こちらは成功する
    it 'statusが0に更新されること' do
      expect { subject }.to change { Parent.find(parent.id).status }.from(1).to(0).
        and change { Child.exists?(id: child.id) }.from(true).to(false)
    end
  end
end

上記テストの2つのitはchangeが逆なだけで同じことを検証しているのですが、上記itは失敗して下記itは成功します。
下記が実行結果です。

% rspec spec/models/child_spec.rb
F.

Failures:

  1) Child destroy child status0に更新されること
     Failure/Error:
       expect { subject }.to change { Child.exists?(id: child.id) }.from(true).to(false).
         and change { Parent.find(parent.id).status }.from(1).to(0)

       expected `Parent.find(parent.id).status` to have initially been 1, but was 0
     # ./spec/models/child_spec.rb:14:in `block (3 levels) in <top (required)>'

Finished in 0.3997 seconds (files took 9.11 seconds to load)
2 examples, 1 failure

Failed examples:

rspec ./spec/models/child_spec.rb:13 # Child destroy child statusが0に更新されること

changeの順序が逆になるだけで結果が変わる意味がわからなく結構ハマってたのですが、実行順序を正しく理解することでなぜ失敗するのかを理解することができました。
次の章で実行順序について説明したいと思います。

どのような順序で実行されているのか?

成功テストの場合

まずは成功するテストの挙動を見ていきます。
処理順がわかるように①〜⑤の番号を振りました。

スクリーンショット_2021-06-25_13_55_44.png


subjectを実行する前にchangeの内容を実行して実行前の値を取得します。
ここが注意点なのですが、changeを複数繋げている場合、後ろから実行されます。

Child.exists?(id: child.id)

これを実行するときにchildが参照されるのでlet(:child)が実行されchildが生成されます。
childを生成するときにparentが参照されているのでparentも生成されます。
childが生成されるのでafter_createが動いて、parentのstatusが1に更新されます。
このときはchildが存在するので結果はtrueとなります。


次にもう1つのchangeの実行前の値を取得します。

Parent.find(parent.id).status

先程説明した通り、①で更新されたので、1となります。


subjectを実行します。

parent.child.destroy

これにより、childは削除され、after_destroyでparent.statusが0に更新されます。


changeの実行後の確認は前のchangeから実行されます。
そのため②と同じ処理が実行されます。
③でparent.statusが0に更新されるので、結果は0となります。


もう1つのchangeが実行されます。①と同じ処理です。
③でchildを削除しているので、結果はfalseとなります。

すべての結果がchangeのfromやtoに記載されている値を一致するためテストは成功します。

失敗テストの場合

次に失敗テストです。成功テストとやることは一緒なので差分のみ説明します。


後ろのchangeが実行されます。

Parent.find(parent.id).status

parentが参照されるので、let(:parent)でparentが生成されます。
このとき、先程の成功テストと異なりchildがまだ生成されていません。
そのため、結果はstatusのデフォルト値である0になります。

この時点でchangeのfrom(1)と結果が異なるため、このテストは失敗することが確定します(実際にはsubjectが実行された後に失敗と判定されるようです)。

②以降は同じ結果となるので省略します。

何がダメだったか?

今回、事前に必要なparentやchildをletで定義して参照されたときに生成されるようにしていたことが問題だと思います。
事前に必要なデータはlet!を使うか、beforeで定義するなど、明確に事前に生成されるようにしておくべきです。
例えば下記のようにしておくと両方のテストが成功します。letをlet!に変更したのみです。

RSpec.describe Child, type: :model do
  let(:parent) { create(:parent) }

  describe 'destroy child' do
    subject { parent.child.destroy }

    let!(:parent) { create(:parent) }
    let!(:child) { create(:child, parent: parent) }

    it 'statusが0に更新されること' do
      expect { subject }.to change { Child.exists?(id: child.id) }.from(true).to(false).
        and change { Parent.find(parent.id).status }.from(1).to(0)
    end

    it 'statusが0に更新されること' do
      expect { subject }.to change { Parent.find(parent.id).status }.from(1).to(0).
        and change { Child.exists?(id: child.id) }.from(true).to(false)
    end
  end
end

今回のテストはシンプルなので問題点が見つけやすかったですが、実際のテストはたくさんの変数が出てきて複雑になるで気づきづらくなると思います。
(少なくとも私は気づくのに結構時間かかりました・・・)

今回のような問題にハマらないように、全部let!で定義すればいいじゃん!!という意見も見かけたことがありますが、個人的にはletの参照された場合のみロードされるという仕組みは使わなきゃもったいない機能だと思っています。
テストだから気にしない方が多いとは思いのですが、無駄なlet!の積み重ねでテスト実行時間が大幅に変わることもあります。
おまじないのようにletで定義する場合はlet!を使う。と覚えるのではなく、正しく仕組みを理解することで適切に使い分けることができるようになる方が良いと思います。

最後に過去にもRSpecの高速化についての記事をいくつか書いているのでリンクしておきます。
興味がある方は見ていただけると嬉しいです!

5
0
0

Register as a new user and use Qiita more conveniently

  1. You get articles that match your needs
  2. You can efficiently read back useful information
  3. You can use dark theme
What you can do with signing up
5
0

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?