Findyの爆速開発を支える「システムを守るテストコード」の実例3選

こんにちは。

Findy で Tech Lead をやらせてもらってる戸田です。

弊社では本番環境へのデプロイを1日に複数回実行していますが、本番環境での不具合の発生率は低いです。

次の画像は弊社のあるプロダクトの直近1年のFour Keysの数値です。

平均で1日2.3回の本番デプロイを行っていますが、変更障害率は0.4%程度を維持しています。単純計算ですが、1年で障害が2件程度の水準です。

また、平均修復時間は0.3hとなっており、障害が発生しても20分以内には復旧できていることがわかります。

この数値を維持できている理由の1つにテストコードの品質があると考えています。

システムで発生する不具合を自動テストが検知することで本番環境への不具合の混入を事前に防ぐことができ、仮に不具合が発生したとしても修正内容が他の箇所に影響が出ないことをテストコードが保証してくれるため迅速に修正できるからです。

弊社ではテストカバレッジよりも、テストで何を守るのか？という観点を重視しています。その上でテストカバレッジの90％超えは珍しいことではありません。

しかし、実際にはカバレッジを意識したことはなく、システムを守るテストを意識した結果、勝手にカバレッジが上がっていただけなのです。

我々としては「全てのテストコードが通る」ことよりも、「コケるべき時にコケるテストコード」の方が重要だと定義しています。

この記事では、弊社で実践している「テストを通すためのテストコード」ではなく、「システムを守るテストコード」の書き方をいくつか紹介していきます。

それでは見ていきましょう！

実例紹介
まとめ

実例紹介

関数の実行状態のチェック

特定の処理を実行したあとに、結果に応じてコールバック関数を実行するようなケースは少なくありません。

コンポーネントに関数を渡し、ボタンをクリックした時にその関数を実行するようなケースもあるでしょう。

このようなケースの場合、関数を特定の処理で実行する際に、その関数が適切に扱われているのかを守るテストケースが必要になります。

例えば正常系の次のようなテストコードがあるとします。

const mockOnSuccessCallback = jest.fn();
const mockOnErrorCallback = jest.fn();
const { result } = renderHook(() => useHoge({ onSuccessCallback: mockOnSuccessCallback, onErrorCallback: mockOnErrorCallback }));

act(() => {
  result.current.handleSubmit();
});

expect(mockOnSuccessCallback).toHaveBeenCalledWith('test');

hooksの関数を実行し、成功時に引数で渡したコールバック関数がパラメータ付きで実行されることを確認しています。

一見何の変哲もないテストコードですが、このテストコードには次の視点が抜けており、システムを守るテストとしては不十分です。

成功時の関数が複数回実行されたとしても通ってしまう
エラー時のコールバック関数が実行されたとしても通ってしまう

こういったケースでは、弊社では次のようなテストコードが推奨されます。

const mockOnSuccessCallback = jest.fn();
const mockOnErrorCallback = jest.fn();
const { result } = renderHook(() => useHoge({ onSuccessCallback: mockOnSuccessCallback, onErrorCallback: mockOnErrorCallback }));

act(() => {
  result.current.handleSubmit();
});

expect(mockOnSuccessCallback).toHaveBeenCalledTimes(1);
expect(mockOnSuccessCallback).toHaveBeenCalledWith('test');
expect(mockOnErrorCallback).not.toHaveBeenCalled();

このテストコードにより、「成功時に1回だけコールバック関数がパラメータ付きで実行され、エラーのコールバック関数が実行されない」ことを守ることができます。

この2つのテストコードのカバレッジは変わりませんが、システムを守ることができる内容が変更前よりも増えており、アプリケーションの振る舞いをより厳密に守ることができるようになりました。

出力対象外のチェック

特定の情報のみを返す処理がある場合、その情報が正しく取得できることを確認するテストケースが必要になります。

特定のユーザー情報を取得し、そのユーザーに紐付く各種情報を取得する処理はよくあるケースです。

このようなケースの場合、特定の情報のみを取得できることを守るテストケースが必要になります。

例えば次のようなテストコードがあるとします。

RSpec.describe User do
  describe '.skills' do
    let!(:user) { create(:user) }
    let!(:user_skills) { create_list(:user_skill, 3, user:) }

    it 'returns user skills' do
      expect(user.skills).to eq(user_skills)
    end
  end
end

特定のユーザーデータに紐付くスキルデータを取得することを確認しています。

一見何の変哲もないテストコードですが、このテストコードには次の視点が抜けており、守るテストとしては不十分です。

他のユーザーのスキルデータが混在してしまう可能性を否定できていない
対象のユーザーがスキルデータを持っていない場合の挙動を確認できていない

特に他のユーザーのスキルデータが混在してしまうケースが発生してしまった場合、最悪の場合インシデントにもなりかねません。

こういったケースでは、弊社では次のようなテストコードが推奨されます。

RSpec.describe User do
  describe '.skills' do
    let!(:user) { create(:user) }

    before do
      # NOTE: 他ユーザーのレコードが対象外になることを守る
      create_list(:user_skill, 3, user: create(:user))
    end

    context 'when user has skill' do
      let!(:user_skills) { create_list(:user_skill, 3, user:) }

      it 'returns user skills' do
        expect(user.skills).to eq(user_skills)
      end
    end

    context 'when user not has skill' do
      it 'returns empty array' do
        expect(user.skills).to eq([])
      end
    end
  end
end

このテストコードにより、「他のユーザーのスキルデータが混在してしまう可能性を否定」し、「対象のユーザーがスキルデータを持っていない場合でも正常に処理を実行できる」ことを守ることができるようになりました。

対象のレコードをチェックすることだけではなく、対象外のレコードが本当に対象外になっているのかどうか、レコードが存在しなかった場合にエラーが起きないかどうかまで確認することで、アプリケーションの振る舞いをより厳密に守ることができるようになりました。

チェックする値の厳密化

特定の値を返す関数がある場合、実行時に期待する値が返ってくることを確認するテストケースが必要になります。

例えば次のようなテストコードがあるとします。

RSpec.describe User do
  describe '.has_multi_skills' do
    let!(:user) { create(:user) }

    before do
      # NOTE: 他ユーザーのレコードが対象外になることを守る
      create_list(:user_skill, 3, user: create(:user))
    end

    context 'when user has multi skills' do
      before { create_list(:user_skill, 3, user:) }

      it 'returns true' do
        expect(user.has_multi_skills).to be_truthy
      end
    end

    context 'when user has skill' do
      before { create(:user_skill, user:) }

      it 'returns false' do
        expect(user.has_multi_skills).to be_falsy
      end
    end

    context 'when user not has skill' do
      it 'returns false' do
        expect(user.has_multi_skills).to be_falsy
      end
    end
  end
end

対象のユーザーが複数のスキルデータを持っている場合にtrueを、それ以外の場合にはfalseが返ってくることを確認しています。

パッと見で特に問題は無さそうですが、be_truthy be_falsy の仕様には注意が必要です。

it { expect(true).to be_truthy }      # passes
it { expect("hoge").to be_truthy }    # passes
it { expect(nil).to be_truthy }       # fails
it { expect(false).to be_truthy }     # fails

it { expect(false).to be_falsy }      # passes
it { expect(nil).to be_falsy }        # passes
it { expect("hoge").to be_falsy}      # fails
it { expect(true).to be_falsy }       # fails

be_truthy be_falsy はbooleanの値以外でも実行結果が変わります。

そのため、例えば関数が返す値がbooleanではなく文字列になってしまった場合にテストが通ってしまう可能性があります。

何かしらの値が入っていることを確認するテストケースであれば問題ありませんが、今回のケースではbooleanの値を厳密にチェックすることが要求されます。

こういったケースでは、弊社では次のようなテストコードが推奨されます。

RSpec.describe User do
  describe '.has_multi_skills' do
    let!(:user) { create(:user) }

    before do
      # NOTE: 他ユーザーのレコードが対象外になることを守る
      create_list(:user_skill, 3, user: create(:user))
    end

    context 'when user has multi skills' do
      before { create_list(:user_skill, 3, user:) }

      it 'returns true' do
        expect(user.has_multi_skills).to be true
      end
    end

    context 'when user has skill' do
      before { create(:user_skill, user:) }

      it 'returns false' do
        expect(user.has_multi_skills).to be false
      end
    end

    context 'when user not has skill' do
      it 'returns false' do
        expect(user.has_multi_skills).to be false
      end
    end
  end
end