4
1

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 5 years have passed since last update.

ユニークビジョン株式会社Advent Calendar 2018

Day 5

specファイルからテストケース以外のコードを追い出して文書として読みやすくする

Last updated at Posted at 2018-12-04

はじめに

こんにちは。

世の中のプログラマの皆さんはどのようにテストコードを書いているのでしょうか。subjectshared_examplesで同じコードを共通化するといいよ、などのリファクタリング方法を紹介していただいている記事は見かけますが、specファイルの分割の方法などはよくわかりませんでした。

今日はspecファイルとそれ以外のファイルに書く内容について、最近の自分の思うところを話してみます。

やりたいこと

テスターやディレクターの方に、「テストコードでどのような内容を確認しているのか教えてください、(手動で実施する)テストケースを作成する際の参考にしたいので。」と聞かれたときに「こちらです!」とすぐに提出できるものがあればいいのですが、そういう資料を用意しておくのはなかなか大変です。

そもそもRSpecのspecファイルは人間が読みやすいように工夫されているものなので、これをそのまま共有しているところもあるかもしれませんが、できるだけ日本語の文書に近い書き方をして、プログラマ以外の方にも読みやすい内容にできないかということを考えます。

前提

サンプルシステム

例えば、プロジェクトとそのプロジェクトのマイルストーンを登録するシステムを考えます。プロジェクトには開始日とリリース予定日を登録します。マイルストーンには予定日と完了日を登録します。

このシステムでは1日に1回実行するバッチ処理によって、開始前日になってもリリース予定日が登録されていないプロジェクトや、予定日を過ぎても完了日が登録されていないマイルストーンを報告する機能があるとします。

ここでの報告とは、「~レポート」テーブルにレコードを作成することを表すとします。

DBには以下のテーブルがあるとします。

テーブル論理名 テーブル物理名 モデル名
プロジェクト projects Project
マイルストーン milestones Milestone
プロジェクトレポート project_reports ProjectReport
マイルストーンレポート milestone_reports MilestoneReport

ソースコードはこちらをご参照ください。

作成するテストケース

このサンプルのシステムにおいて、以下の3点のテストケースを作成します。

  • 開始2日前でリリース日が登録されていないプロジェクトの場合、報告されないこと。
  • 開始1日前でリリース日が登録されていないプロジェクトの場合、警告ステータスで報告されること。
  • 未完了のマイルストーンがある場合、警告ステータスで報告されること。

specファイルの作成

まっすぐ書く

では、これらのテストケースをまっすぐに書いてみます。

RSpec.describe Batch::DailyBatch do

  let (:config) { AppUtil.load_config }
  subject (:batch) { Batch::DailyBatch.new(config) }

  let (:stub_now) do
    allow(AppUtil).to receive(:get_now) do
      current_at.nil? ? Time.now : Time.parse(current_at)
    end
  end

  let (:actual) do
    get_actual.map { |row| row.serializable_hash(except: :id).symbolize_keys }
  end

  let (:retrieve_project_reports) do
    ProjectReport.joins(:project).select(:project_nm, :status_cd)
  end

  let (:retrieve_milestone_reports) do
    MilestoneReport.joins(:milestone).select(:milestone_nm, :status_cd)
  end

  context '開始2日前でリリース日が登録されていないプロジェクトの場合' do
    let (:current_at) { '2018-12-01 00:00:00+09' }
    let (:get_actual) { retrieve_project_reports }

    it '報告されないこと' do
      create(:project, {
        project_nm: 'project1',
        start_at: '2018-12-03 00:00:00+09'
      })
      batch.execute({ mode: :project_before_2days })
      expect(actual).to eq([])
    end
  end

  context '開始1日前でリリース日が登録されていないプロジェクトの場合' do
    let (:current_at) { '2018-12-02 00:00:00+09' }
    let (:get_actual) { retrieve_project_reports }

    it '警告ステータスで報告されること' do
      create(:project, {
        project_nm: 'project1',
        start_at: '2018-12-03 00:00:00+09'
      })
      batch.execute({ mode: :project_not_scheduled })
      expect(actual).to eq([{
        project_nm: 'project1',
        status_cd: 'bad'
      }])
    end
  end

  context '未完了のマイルストーンがある場合' do
    let (:current_at) { '2018-12-05 00:00:00+09' }
    let (:get_actual) { retrieve_milestone_reports }

    it '警告ステータスで報告されること' do
      project = create(:project, {
        project_nm: 'project1',
        start_at: '2018-12-03 00:00:00+09',
        release_at: '2019-01-03 00:00:00+09'
      })
      milestone1 = create(:milestone, {
        project_id: project.id,
        milestone_nm: 'milestone1',
        schedule_at: '2018-12-04 00:00:00+09',
        completed_at: '2018-12-04 00:00:00+09'
      })
      milestone2 = create(:milestone, {
        project_id: project.id,
        milestone_nm: 'milestone2',
        schedule_at: '2018-12-05 00:00:00+09'
      })
      create(:milestone_report, {
        milestone_id: milestone1.id,
        report_at: '2018-12-04 00:00:00+09',
        status_cd: 'good'
      })
      batch.execute({ mode: :milestone_not_completed })
      expect(actual).to eq([{
        milestone_nm: 'milestone1',
        status_cd: 'good'
      }, {
        milestone_nm: 'milestone2',
        status_cd: 'bad'
      }])
    end
  end

end

やっていることは単純です。テーブルにテスト条件を表すレコード(事前条件と呼ぶことにします)を登録し、バッチ処理を実行して、結果を確認する(実行後のレコードの状態、もしくは期待する値のことを事後条件と呼ぶことにします)、ということを3回記述してあります。

なお、途中に登場するcreateFactoryBotのメソッドで、これでテーブルにレコードを作成しています。

さて、プログラマではない人にこのソースコードを見せて、テストケースを探し出してもらうことにします。すなわち、今回の場合、注目するのはcontextitです。

letsubjectは無視してもらうこととして、真ん中あたりからケースが登場しています。一つひとつのケースが離れていて、やや読みにくいでしょうか。

また、テストケースが3つだけなので探し出すのは難しくありませんが、数が増えると大変になりそうです。

もう少し読みやすくならないか考えてみます。

テストデータをYAMLファイルへ追い出す

それぞれのテストケースの距離が離れているのは、事前条件と事後条件の定義が縦に長いせいでしょうか。では、これらのデータをspecファイルの外に追い出してみます。具体的には、YAMLファイル(02_daily_batch.yml)にデータを記述して、そのファイルを読み込むようにします。

RSpec.describe Batch::DailyBatch do

  let (:config) { AppUtil.load_config }
  subject (:batch) { Batch::DailyBatch.new(config) }

  let (:stub_now) do
    allow(AppUtil).to receive(:get_now) do
      current_at.nil? ? Time.now : Time.parse(current_at)
    end
  end

  let (:actual) do
    get_actual.map { |row| row.serializable_hash(except: :id).symbolize_keys }
  end

  let (:retrieve_project_reports) do
    ProjectReport.joins(:project).select(:project_nm, :status_cd)
  end

  let (:retrieve_milestone_reports) do
    MilestoneReport.joins(:milestone).select(:milestone_nm, :status_cd)
  end

  let (:test_data) do
    file_path = Rails.root.join('spec', 'test_data', 'batch', '02_daily_batch.yml')
    YAML.load_file(file_path).deep_symbolize_keys
  end

  let (:pre_condition) do
    values = test_data[test_pattern]
    writer = TestDataWriter.new(values[:fixtures])
    writer.execute
  end

  let (:execute) do
    pre_condition
    batch.execute({ mode: test_pattern })
    actual
  end

  let (:expected) { test_data[test_pattern][:expected] }

  context '開始2日前でリリース日が登録されていないプロジェクトの場合' do
    let (:current_at) { '2018-12-01 00:00:00+09' }
    let (:test_pattern) { :project_before_2days }
    let (:get_actual) { retrieve_project_reports }

    it '報告されないこと' do
      expect(execute).to eq(expected)
    end
  end

  context '開始1日前でリリース日が登録されていないプロジェクトの場合' do
    let (:current_at) { '2018-12-02 00:00:00+09' }
    let (:test_pattern) { :project_not_scheduled }
    let (:get_actual) { retrieve_project_reports }

    it '警告ステータスで報告されること' do
      expect(execute).to eq(expected)
    end
  end

  context '未完了のマイルストーンがある場合' do
    let (:current_at) { '2018-12-05 00:00:00+09' }
    let (:test_pattern) { :milestone_not_completed }
    let (:get_actual) { retrieve_milestone_reports }

    it '警告ステータスで報告されること' do
      expect(execute).to eq(expected)
    end
  end

end

test_dataというletでYAMLファイルを読み込み、pre_conditionでDBに保存する、という流れにしてみました。FactroyBotでのレコードの作成処理はTestDataWriterというクラスに移動されました。

itの内容が1行だけになり、テストケースどうしの距離が近くなって読みやすくなりました。

しかし、引き続き無視しているletは数が増えて、テストケース部分はどんどんファイルの下の方へ追いやられてしまっています。

letたちをファイルの下の方に書けばいいんじゃない?あ、そうですね。

それはそうなのですが、これらのletはテスト実行の準備や共通化などのために記述しているもので、テストの内容自体とは関係がなく、プログラマとしてもあまり興味のないコードです。

これらもspecファイルの外に追い出してしまうことはできないでしょうか。

準備や共通化のためのコードを別のクラスに追い出す

テスト実行の準備や共通化のためのコードをspecファイルの外に追い出してみます。もう一つ、テストケースごとにitを記述するのではなく、shared_examplesにしてみます。

require './spec/assistants/batch/daily_batch_assistant'

RSpec.describe Batch::DailyBatch do

  shared_context :daily_batch_context do |test_pattern|
    let (:assistant) { Batch::DailyBatchAssistant.new(test_pattern) }
    subject (:batch) { Batch::DailyBatch.new(config) }
  end

  shared_examples :daily_batch_execute do |makes_the_result_with, test_pattern|
    include_context :batch_context
    include_context :daily_batch_context, test_pattern

    it makes_the_result_with do
      batch.execute(values[:params])
      expect(actual).to eq(values[:expected])
    end
  end

  context '開始2日前でリリース日が登録されていないプロジェクトの場合' do
    include_examples :daily_batch_execute, '報告されないこと', :project_before_2days
  end

  context '開始1日前でリリース日が登録されていないプロジェクトの場合' do
    include_examples :daily_batch_execute, '警告ステータスで報告されること', :project_not_scheduled
  end

  context '未完了のマイルストーンがある場合' do
    include_examples :daily_batch_execute, '警告ステータスで報告されること', :milestone_not_completed
  end
end

テストケース以外のコードが減り、かつ、テストケースどうしの距離がさらに近くなって、一覧性が上がりました。

他のテストでも共通で使えそうなletshared_contextとして定義して、別のファイルに追い出しました。batch_contextという名前で取り込んでいます。

また、テスト対象クラス固有の内容が含まれる処理については、~Assistantというクラスを定義して、そちらにコードを移しました。ここではDailyBatchAssistantというクラス名になっています。移動されたコードは、YAMLファイルの読み込みやDBへのレコード保存、バッチ処理実行後のレコード取得などです。assistantが定義されているだけで使われていないように見えるのでわかりにくくなってしまっていますが、assistantbatch_contextの中で使用されています。

このくらいのコードであれば、プログラマ以外の方にもテストケースの内容を読み取ってもらえそうでしょうか。

おわりに

テストコードの内容をディレクターの方と共有できれば、手動でのテストを省略できる部分があるか、逆にどのような内容を自動化したいか、というような相談ができると思います。

また、テストコードはメンテナンスのしやすさが重要ですが、そもそもどんなことをテストしているのかがわかりにくいと、メンテナンスの際にまずそこから読み解くことになり、後回し、あるいは放置されてしまう要因になるかもしれません。

コードの共通化や、その他手の込んだことをやりすぎると逆に柔軟性がなくなってメンテナンスしずらくなることもあるので、結局テストコードをどのように書くのがちょうどよいのかは試行錯誤が続くところですが、テスト内容の読み取りやすさに気をつけよう、というのが、最近の自分の思っていることでした。

長くなりましたが、Advent Calendar 2018 5日目はこのへんで。では、よい師走を。

4
1
2

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
4
1

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?