Zenn
SUPER STUDIOテックブログPublicationへの投稿
🙆‍♀️

Rspec社内運用まとめ その1 可読性について

Yuji Miyashita
2024/08/16に公開
Rails
Ruby
RSpec
tech

はじめに

弊社のサービスである ecforce は Rspec を用いてテストコードを実装しています。
現在はグループ内でルールや方針を定めたため、それらに則って各チームがテストコードを実装するようになっています。
数年前はルールや方針が何も定まっておらず、可読性や拡張性が著しく低下していました。
今回はテストコードにおいて数年前の状態から今に至るまで、何をどのように変えていったのかをご紹介していきます。

本題

数年前の状態について

まずはどこに問題があるかを特定するために、開発メンバーにヒアリングを行いました。
その結果、以下の問題点を抱えていることが分かりました。

  1. 高いカバレッジ率を達成することに重きを置きすぎていた
  2. 可読性を考慮していなかったため、メンテナンスができなくなっていた
  3. システムにおけるDB構造の理解が追いつかず、テストデータの作成が容易にできなかった
  4. 基本はパスするものの稀にパスしないテストがあり、調査に工数が取られてしまっていた

※3については次回の章で触れます
※4については次々回の章で触れます

高いカバレッジ率を達成することに、重きを置きすぎていた

カバレッジ率が低い = テストケースが網羅されていない、というのはメンバー全員が理解していました。
ただカバレッジ率が高いと何が良いのかについては、認識できていませんでした。
開発メンバーには カバレッジ率は高ければ高いほど良いというわけではない、テストケースを書いた結果自然と上がるもの ということを伝え、以下を重点的に認識合わせを行いました。

  • カバレッジをできるだけ高めたとしても、障害が発生することはあるためコスパが悪い
  • 数字に振り回されると、本来テストの役割である リグレッションテストの自動化 や バグを検知する仕組み を見失ってしまう
  • その機能に必要なテストケースが揃っているかどうかにフォーカスする
  • 人間が開発している以上障害は付き物なので、再発防止ですぐにテストケースを追加できる拡張性の高い状態になっているかが重要

可読性を完全に捨てていたことで、メンテナンスができない状態になっていた

今回の知見共有で開発メンバーに伝えたのは、

  • 無理やり共通化せず、上から下に読めるように書くこと
  • ネストを可能な限り最小限にすること

という2点です。
その他細かい点も伝えていますが、大きくはこの2点を守って実装することを伝えました。
以下でコードを例にいくつか説明します。

NG例

describe '#method_name' do
  let!(:data_x) { create(:xxx, attr1: xxx, attr2: xxx) }

  before '予め処理を実行' do
    # 何か処理を実行
  end

  context '正常系' do
    before '追加で事前に処理を実行' do
      # 何か処理を実行
    end

    let!(:data_y) { create(:yyy) }

    context 'Aの場合' do
      let!(:data_z) { create(:zzz) }

      it 'trueが返ること' do
        expect(data_z.method_name).to be_truthy
      end
    end

    context 'Bの場合' do
      let!(:data_z) { create(:zzz) }

      it 'trueが返ること' do
        expect(data_z.method_name).to be_truthy
      end
    end
  end

  context '異常系' do
    before '追加で事前に処理を実行' do
      # 何か処理を実行
    end

    let!(:data_z) { create(:yyy) }

    context 'Cの場合' do
      let!(:data_z) { create(:zzz) }

      it 'falseが返ること' do
        expect(data_z.method_name).to be_falsey
      end
    end

    context 'Dの場合' do
      let!(:data) { create(:zzz) }

      it 'falseが返ること' do
        expect(data_z.method_name).to be_falsey
      end
    end
  end
end

無理やり共通化の廃止

可読性を低下させている一番の原因は、 無理やり共通化すること にあると仮定しました。

  • letが describe 直下にあるもの、 before 下にあるもの、 context 下にあるものが混在している
  • beforeが describe 直下にあるものと context 下にあるものが混在している

このような共通化をすることで得られるメリットは特になく、以下のようなデメリットだけが発生します。

  • テストケースがいくつあるかが読み解きづらい
  • どの順番でコードが実行されるかが読み解きづらい
  • テストコードが合っているかどうかの判断が困難になる
  • そのテストケースで使っている変数やデータがどれかがわからなくなる
  • 視線が上に行ったり下に行ったりして読みづらい

共通化することでコードの行数が減らせるという考え方もありますが、実際に削減できるのは微々たるものです。
オブジェクト指向言語とはいえど、プログラムの基本は上から下に一直線に読めることが重要です。
特にテストコードは、本体コードよりもコード量が増えていきますので、上から下に読めることの重要性が、本体コードよりも増してきます。

変数定義は必要最低限に

以下のようにlet/let!によって定義された変数は3つですが、実際に使っている箇所は data_z.method_name だけです。

context 'xxxの場合' do
  let!(:data_x) { create(:xxx, attr1: xxx, attr2: xxx) }
  let!(:data_y) { create(:yyy) }
  let(:data_z) { create(:zzz) }

  it{ expect(data_z.method_name).to eq true }
end

ここについては様々な書き方がありますが、例として以下のようにit内に直接createを書くことで、必要な分だけ変数定義することができます。

context 'xxxの場合' do
  it do
    create(:xxx, attr1: xxx, attr2: xxx)
    create(:yyy)
    data_z = create(:zzz)

    expect(data_z.method_name).to eq true
  end
end

ここで言いたいのは、すべてのデータに対して変数定義をする必要はないということです。

正常系と異常系という括りについて

例えばrequest specにおいて 500が返ること は、正常系or異常系どちらでしょうか。
考え方として、 500が返るから異常 と 正常に500として処理した という2パターンがあります。
バリデーションが発火するor発火しない、というのも同じカテゴリーです。
弊社では以下の理由から、正常系/異常系をカテゴライズしないようにしました。

  • 正常系/異常系かは人によって解釈が異なる
  • 余分なcontextのネストが1つ増えるので、可読性が下がる

OK例

NG例をリファクタリングした結果、以下のようになりました。

describe '#method_name' do
  context 'Aの場合' do
    it do
      create(:xxx, attr1: xxx, attr2: xxx)
      create(:yyy)
      data_z = create(:zzz)
      # 何か処理を実行
      # 何か処理を実行
      expect(data_z.method_name).to eq true
    end
  end

  context 'Bの場合' do
    it do
      create(:xxx, attr1: xxx, attr2: xxx)
      create(:yyy)
      data_z = create(:zzz)
      # 何か処理を実行
      # 何か処理を実行
      expect(data_z.method_name).to eq true
    end
  end

  context 'Cの場合' do
    it do
      create(:xxx, attr1: xxx, attr2: xxx)
      create(:yyy)
      data_z = create(:zzz)
      # 何か処理を実行
      # 何か処理を実行
      expect(data_z.method_name).to eq false
    end
  end

  context 'Dの場合' do
    it do
      create(:xxx, attr1: xxx, attr2: xxx)
      create(:yyy)
      data_z = create(:zzz)
      # 何か処理を実行
      # 何か処理を実行
      expect(data_z.method_name).to eq false
    end
  end
end
  • contextの数を数えれば、テストケースがどれくらい揃っているかが一目で分かる
  • it内を読めば、各context内でどのような処理や期待値でテストが書かれているかが一目で分かる

というのがポイントです。

その他細かい指摘

shared_contextやshared_examplesを扱うのは難しい

これは先程の 無理やり共通化 と関連していて、共通化するための文法 shared_context や shared_examples を扱うと上から下に読めるコードではなくなるため、どうしても可読性が落ちやすくなります。
このあたりを使うか否か、チーム内で認識合わせをしたほうが良いでしょう。

be_truthy, be_falseyは避ける

Rubyにおける「偽」に相当するのは以下2点のみです。

  • nil
  • false

空文字や0も含め、それ以外は「真」という扱いになります。
be_truthy や be_falsey は、Rubyにおける真偽値を判定するためのマッチャです。
ただしテストコード上では、戻り値がtrueでも空文字でもOKというケースは少ないため、 eq マッチャのほうが精度が高いです。

it{ expect(true).to be_truthy }
it{ expect('').to be_truthy } # 意図せずテストがパスしてしまう可能性が高い

it{ expect(true).to eq true } # これはOK
it{ expect('').to eq true } # テストがパスしない (= 異常を検知しやすい)

ラベルつけなくても分かるものは省略する

以下のように eq true から、「trueになること」は読めるので、itのラベルを付けなくても良いものは省略するようにしました。

it 'trueになること' do
  expetct(result).to eq true
end

it{ is_expetcted to eq true }

書き方に困ったらRspecのRspecを見て欲しい

Rspecも1つのRubyGemsですので、テストが書かれています。
RspecのRspec を読み、書き方や使っている文法を参考にするのは良いのではないかと思っています。

before/before(:each) と before(:all) は挙動が違う

  • before/before(:each)
  • before(:all)

は実行タイミングだけでなく、 テスト終了後にロールバックされるかどうか も異なります。
before(:all) では、テストが終了しても作成したデータがDBに残り続けます。
基本は before/before(:each) を使うのが無難です。

subject を使うことで読みづらくなることがある

僕がRSpecでsubjectを使わない理由 にチーム内でも100%同意したため、そのまま開発メンバーに伝えました。

まとめ

いかがでしたでしょうか。
今回は、テストに対する考え方と可読性を上げるためのテストの書き方についてお伝えしました。
テストを書く上で気をつける点はいくつかあるものの、これらを開発メンバーに共有したことで、

  • どの機能のテストが足りていないかが、ある程度読めば判断できるようになった
  • 中身が分からなくても、contextやitで何をテストしているかが読めるようになった

といった感じで、ようやくスタートラインに立つことができました。

  • カバレッジ率にこだわらず、その機能に必要なテストケースが揃っているかどうかにフォーカスを当てること
  • 無理やり共通化しないこと

を意識するだけでも、メンテナンスや可読性が大きく改善することが今回の知見共有で分かり、大きな収穫だったのではないかと考えています。

次回は、 システムにおけるDB構造の理解が追いつかず、テストデータの作成が容易にできなかった 場合に関して、どのようなアプローチをしたかについてご紹介します。

SUPER STUDIOの採用について

SUPER STUDIOでは、積極的にエンジニアを採用しています。
少しでも興味がありましたら、以下の記事をご覧ください。

https://hrmos.co/pages/superstudio/jobs/0010024
https://hrmos.co/pages/superstudio/jobs/0000400
https://hrmos.co/pages/superstudio/jobs/0000414
https://hrmos.co/pages/superstudio/jobs/0010025

また、下記はSUPER STUDIOで年に一度開催されるKICKOFFイベントにて、社内表彰されたエンジニアの受賞インタビューです。SUPER STUDIOのエンジニア組織についてより理解を深められる内容となっておりますので、ぜひご一読ください。

https://www.wantedly.com/companies/super-studio/post_articles/497997
https://www.wantedly.com/companies/super-studio/post_articles/487617

SUPER STUDIOテックブログ
SUPER STUDIOテックブログPublication

統合コマースプラットフォーム「ecforce」を開発・提供する、株式会社SUPER STUDIOのテックブログです。 システムの開発や運用、データ移行で得た話を中心に知見をお届けします。

Discussion