インターンシップで学んだこと3:テストを整理する方法 - 2013-08-27 - ククログ

ククログ

株式会社クリアコード > ククログ > インターンシップで学んだこと3:テストを整理する方法

インターンシップで学んだこと3:テストを整理する方法

インターン募集を開始したのが半年前の2月で、6月に開催されたRubyKaigi 2013までは1件も応募がありませんでした。RubyKaigi 2013に参加したところ1件応募があり、6月後半から7月にかけて実施しました。ここ最近まとめているインターンシップで学んだことはこの時期に実施したインターンシップで学んだことです。

その後、さらに2件の応募がありました。クリアコードのインターンシップは学生に限定せず働いている人でも働いていない人でも対象としていますが、3件ともすべて学生の方です。学生の方からの応募は夏休みの時期の方が多いようです1

2件の応募のうち、1件は「インターンシップ実施企業と学生が話をできる場を提供するイベント」がきっかけでした。もう1件はリーダブルコードの解説を読んでクリアコードを知ったことがきっかけだということでした。解説を書いてよかったです2

なお、この2件の応募に対して9月にインターンシップを実施する予定です。9月のインターンシップに向けて、前回のインターンシップで学んだことを早くまとめて活かしたいところです。が、間に合わなそうです。17日あるうちのまだ3日目です。

さて、前回は3日目に3つ学んだことの中の1つ「1人で開発しているときにtypoとどうつきあっていくか」についてまとめました。今回は3日目に学んだことの2つめである「テストを整理する方法」についてまとめます。

冗長なテスト

3日目の時点でのテストは以下のように50行にも満たない小さなものでした。

# -*- coding: utf-8 -*-

require 'test-unit'

require 'epub/parser'
require 'epub-searcher/epub-document'

class TestEPUBDocument < Test::Unit::TestCase
  def setup
    # groonga_doc_all.epub ... spine を一つしか含まない EPUB ファイル
    # 本文は groonga ドキュメント 1 章 が全て入っている
    epub_book_1 = EPUB::Parser.parse(fixture_path('groonga_doc_all.epub'))
    @document_1 = EPUBSearcher::EPUBDocument.new(epub_book_1)

    # groonga_doc_11_12.epub ... spine を二つ含む EPUB ファイル
    # 本文は groonga ドキュメント 1.1 と 1.2 が入っている
    epub_book_2 = EPUB::Parser.parse(fixture_path('groonga_doc_11_12.epub'))
    @document_2 = EPUBSearcher::EPUBDocument.new(epub_book_2)
  end

  def test_extract_contributors
    assert_equal([], @document_1.extract_contributors)
    assert_equal(["groongaコミュニティ A", "groongaコミュニティ B", "groongaコミュニティ C"], @document_2.extract_contributors)
  end

  def test_extract_creators
    assert_equal(["groonga"], @document_1.extract_creators)
    assert_equal(["groongaプロジェクト"], @document_2.extract_creators)
  end

  def test_extract_title
    assert_equal("groongaについて", @document_1.extract_title)
    assert_equal("groongaについて", @document_2.extract_title)
  end

  def test_extract_xhtml_spine
    assert_equal(["OEBPS/item0001.xhtml"], @document_1.extract_xhtml_spine)
    assert_equal(["item0001.xhtml", "item0002.xhtml"], @document_2.extract_xhtml_spine)
  end

  private
  def fixture_path(basename)
    File.join(__dir__, 'fixtures', basename)
  end
end

しかし、いくつか冗長な点が見えてきています。例えば変数名です。

def setup
  # groonga_doc_all.epub ... spine を一つしか含まない EPUB ファイル
  # 本文は groonga ドキュメント 1 章 が全て入っている
  epub_book_1 = EPUB::Parser.parse(fixture_path('groonga_doc_all.epub'))
  @document_1 = EPUBSearcher::EPUBDocument.new(epub_book_1)

  # groonga_doc_11_12.epub ... spine を二つ含む EPUB ファイル
  # 本文は groonga ドキュメント 1.1 と 1.2 が入っている
  epub_book_2 = EPUB::Parser.parse(fixture_path('groonga_doc_11_12.epub'))
  @document_2 = EPUBSearcher::EPUBDocument.new(epub_book_2)
end

単にepub_book@documentとするのではなく、epub_book_1@document_2としています。epub_book_@document_が重複しています。

同様に、テストメソッド名にも冗長な点があります。すべてのテストメソッド名がextract_を含んでいます。

def test_extract_contributors
  # ...
end

def test_extract_creators
  # ...
end

def test_extract_title
  # ...
end

def test_extract_xhtml_spine
  # ...
end

このように、重複した情報によりテストが冗長になってきています。

流れ的には「冗長なテストをすっきりさせるにはどうしたらよいか」という話にいくのですが、その前に、どうして冗長なテストをすっきりさせなければいけないかを考えてみましょう。

冗長なテストをすっきりさせなければいけない理由

実装のコードはすっきりさせないといけないと言われています。そのための技術としてリファクタリングという名前もついているぐらいです。では、テストのコードはどうでしょうか。テストのコードもすっきりさせないといけないのでしょうか。もし、テストのコードもすっきりさせないといけないなら、テストのコードにもリファクタリングという技術を使えるのでしょうか。

結論を先にいうと、テストのコードもすっきりさせなければいけません。しかし、リファクタリングという技術は使えません。テストのコードをすっきりさせるには別のやり方を使わなければいけません。

テストのコードをすっきりさせなければいけない理由は、実装のコードをすっきりさせなければいけない理由と同じです。コードがすっきりしていないと、新しくコードを追加したり、既存のコードを修正したりといったコードを変更することが難しくなるからです。

では、テストのコードを変更しやすくするのはどうしてでしょうか。それは、実装のコードを変更しやすくするためです。テストがあれば、実装のコードを整理しても整理したことによって実装が壊れていないことを簡単に確認できます。同様に、新しく機能を追加したときも既存の機能が壊れていないことを簡単に確認できます。テストのコードが変更しづらくなって、テストを追加しなくなっていくと、実装のコードが壊れてしまったかどうかを確認することが難しくなります。そうすると、こわくて実装のコードを変更しづらくなります。

テストのコードが変更しやすいと以下のように開発できます。

実装変更 + テスト追加 → テスト実行 → 問題なし →
実装変更 + テスト追加 → テスト実行 → 問題なし →
実装整理              → テスト実行 → 問題なし →
実装変更 + テスト追加 → テスト実行 → 問題なし →
…

しかし、テストのコードの変更がしづらいと以下のようになります。

実装変更(テスト追加なし) → テスト実行 → たぶん問題なし →
実装変更(テスト追加なし) → テスト実行 → たぶん問題なし →
実装整理                   → テスト実行 → たぶん問題なし →
実装変更(テスト追加なし) → テスト実行 → たぶん問題なし →
実装変更(テスト追加なし) →
テスト少ないし実行しなくてもいいよね →
問題ないといいなぁ →
実装変更(したくないなぁ) →
…

まとめると、テストのコードを変更しやすくしておくことは、実装を変更しやすくしておくことにつながるので重要です。

それでは、テストのコードをすっきりさせるにはどうしたらよいでしょうか。残念ながらリファクタリングという技術は使えません。なぜなら、リファクタリングをするにはテストが必要だからです。テストをリファクタリングするにはテストのテストが必要になります。これではいつまでたってもテストのリファクタリングができません。

テストをすっきりさせるにはどのようなやり方がよいかを考えてみましょう。

それぞれのテストを独立させる

テストのコードを整理するときは、変更が他のテストに影響を与えていないことを手動で確認する必要があります。テストのテストがないため自動で確認することができないからです。

手動で確認するので、多くのことを確認しなければいけない状況は避けましょう。ミスが多くなります。この状況を避けるためには、それぞれのテストの影響範囲を小さくしておくことが有効です。変更の影響を確認する範囲が小さくなります。

具体的には以下のようにしてそれぞれのテストを独立させます。

  • 1つのテストで1つのことを確認する。
  • テスト対象ごとにテストをグループ化する。(さまざまなテスト対象のテストを1つのグループにまとめない。)

1つのテストで1つのことを確認

1つのテストで1つのことを確認するというのは、ざっくりというと1つのテストの中で1つのアサーションを使うということです3

今回のケースでは、以下のようにtest_extract_contributorsという1つのテストの中で、「spineを1つしか含まないケース」と「spineを2つ含むケース」の2つのことを確認していました。

class TestEPUBDocument < Test::Unit::TestCase
  def setup
    # groonga_doc_all.epub ... spine を一つしか含まない EPUB ファイル
    # 本文は groonga ドキュメント 1 章 が全て入っている
    epub_book_1 = EPUB::Parser.parse(fixture_path('groonga_doc_all.epub'))
    @document_1 = EPUBSearcher::EPUBDocument.new(epub_book_1)

    # groonga_doc_11_12.epub ... spine を二つ含む EPUB ファイル
    # 本文は groonga ドキュメント 1.1 と 1.2 が入っている
    epub_book_2 = EPUB::Parser.parse(fixture_path('groonga_doc_11_12.epub'))
    @document_2 = EPUBSearcher::EPUBDocument.new(epub_book_2)
  end

  def test_extract_contributors
    assert_equal([], @document_1.extract_contributors)
    assert_equal(["groongaコミュニティ A", "groongaコミュニティ B", "groongaコミュニティ C"], @document_2.extract_contributors)
  end
end

これは、以下のように別のテストにわけると1つのテストで1つのことを確認するようになります。

class TestEPUBDocument < Test::Unit::TestCase
  def setup
    # ...
  end

  def test_extract_contributors_with_spine
    assert_equal([], @document_1.extract_contributors)
  end

  def test_extract_contributors_with_spines
    assert_equal(["groongaコミュニティ A", "groongaコミュニティ B", "groongaコミュニティ C"], @document_2.extract_contributors)
  end
end

こうすると、@document_1を変更しても2つめのテスト(..._spinesの方)には影響がありません。影響範囲が狭くなりましたね。

なお、1つのテストで1つのことを確認することはテストが失敗したときのデバッグのしやすさにもつながるので、テストの変更のしやすさ以外の観点からも有用です。

テスト対象ごとにテストをグループ化

個々のテストではなくもう少し大きな単位で考えてみましょう。影響範囲を小さくする別の方法があります。具体的に言うと、「テストをグループ化した単位」です。xUnitの場合はテストケースです。

具体的なコードで考えてみましょう。以下のコードには1つのテストケースの中に2つのテストがあります。それぞれのテストではsetupで用意したのに使っていない変数があります。

class TestEPUBDocument < Test::Unit::TestCase
  def setup
    # groonga_doc_all.epub ... spine を一つしか含まない EPUB ファイル
    # 本文は groonga ドキュメント 1 章 が全て入っている
    epub_book_1 = EPUB::Parser.parse(fixture_path('groonga_doc_all.epub'))
    @document_1 = EPUBSearcher::EPUBDocument.new(epub_book_1)

    # groonga_doc_11_12.epub ... spine を二つ含む EPUB ファイル
    # 本文は groonga ドキュメント 1.1 と 1.2 が入っている
    epub_book_2 = EPUB::Parser.parse(fixture_path('groonga_doc_11_12.epub'))
    @document_2 = EPUBSearcher::EPUBDocument.new(epub_book_2)
  end

  def test_extract_contributors_with_spine
    assert_equal([], @document_1.extract_contributors)
  end

  def test_extract_contributors_with_spines
    assert_equal(["groongaコミュニティ A", "groongaコミュニティ B", "groongaコミュニティ C"], @document_2.extract_contributors)
  end
end

@document_1は1つめのテストでしか使われていません。@document_2は2つめのテストでしか使われていません。setupで準備したのに使われていない変数があるということは、そのテストケースでは様々な種類のテストをグループ化しているということです。必要な変数を使うテスト毎にグループを細かくすることで影響範囲を小さくすることができます。

「spineの数」でさらにグループ化しましょう。

class TestEPUBDocument < Test::Unit::TestCase
  class TestSingleSpine < self
    def setup
      epub_book = EPUB::Parser.parse(fixture_path('groonga_doc_all.epub'))
      @document = EPUBSearcher::EPUBDocument.new(epub_book)
    end

    def test_extract_contributors
      assert_equal([], @document.extract_contributors)
    end
  end

  class TestMultipleSpines < self
    def setup
      epub_book = EPUB::Parser.parse(fixture_path('groonga_doc_11_12.epub'))
      @document = EPUBSearcher::EPUBDocument.new(epub_book)
    end

    def test_extract_contributors
      assert_equal(["groongaコミュニティ A", "groongaコミュニティ B", "groongaコミュニティ C"], @document.extract_contributors)
    end
  end
end

「spineの数」でグループ化したことにより、テストケース名(TestSingleSpine)で「どのようなspineに注目しているか」ということが表現できるようになりました。そのため、もともとは以下のように「spineの数は1つだよ」と書いていたコメントを削除しました。

def setup
   # groonga_doc_all.epub ... spine を一つしか含まない EPUB ファイル
   #                          本文は groonga ドキュメント 1 章 が全て入っている
   epub_book_1 = EPUB::Parser.parse(fixture_path('groonga_doc_all.epub'))
   @document_1 = EPUBSearcher::EPUBDocument.new(epub_book_1)
end

使っている変数に注目してテストケースをわけたため、それぞれのテストは独立するようになりました。TestSingleSpineの中を変更しても、もう一方のテストケースには影響を与えません。

今は変数に注目してテストケースをわけましたが、メソッド名に注目してわける方法もあります。メソッド名の一部のtest_extract_contributorsに注目するとこうなります。

class TestEPUBDocument < Test::Unit::TestCase
  class TestExtractContributors < self
    def test_empty
      assert_equal([], extract("groonga_doc_empty.epub"))
    end

    def test_multiple
      assert_equal(["groongaコミュニティ A", "groongaコミュニティ B", "groongaコミュニティ C"],
                   extract("groonga_doc_multiple_contributors.epub"))
    end

    private
    def extract(path)
      epub_book = EPUB::Parser.parse(fixture_path(path))
      document = EPUBSearcher::EPUBDocument.new(epub_book)
      document.extract_contributors
    end
  end

  def test_XXX
    # ...
  end

  # ...
end

extract_contributorsという情報をテストケース名にもっていって、テスト名からは抜きました。これで、このテストケース内の変更は他のテストケースには影響を与えません。

extract_contributorsがsnake_caseからExtractContributorsというCamelCaseになって気持ち悪いと感じる人もいるでしょう。その場合はsub_test_caseを使えば、snake_caseのまま書けます4

class TestEPUBDocument < Test::Unit::TestCase
  sub_test_case("extract_contributors") do
    def test_empty
      # ...
    end

    def test_multiple
      # ...
    end

    private
    def extract(path)
      # ...
    end
  end

  def test_XXX
    # ...
  end

  # ...
end

テストのグループ化についてはテストをすっきり書く方法として過去にまとめていて、インターンシップでもこのエントリーを読んでもらいました。

まとめ

インターンシップで説明した、テストを整理する方法についてまとめました。今回説明したやり方は以下の2つです。

  • 1つのテストで1つのことを確認する。
  • テスト対象ごとにテストをグループ化する。

過去にこれらのやり方についてエントリーにまとめていたので、インターンシップではまずエントリーを読んでもらい、概要を把握してもらったあとに説明しました。知見を文章としてまとめておくと有用であることがわかりました。過去に説明したものはまとめておいて、次に説明する機会があるときに参照しながら説明する、ということは今後も続けたいです。

作業をしていてなにかあったら相談して欲しいということは伝えていましたが、それだと曖昧すぎるようです。それよりも、「名前に困ったら相談するタイミング」というより具体的な基準を設定するほうが実行しやすいということがわかりました。今回のケースでは_1_2がでていますが、連番は適切な名前が思いつかなかったときに使われることが多いです5。このように、この基準はわかりやすいため、これからも使えそうです。

  1. そりゃそうだろうなぁという感じですね。

  2. リーダブルなコードに関する講演依頼はお問い合わせフォームからどうぞ。

  3. 複数のアサーションで1つのことを確認しているならそれでもOKです。厳密に1つのアサーションにしろということではなく、1つのことを確認しているかがわかりやすい表現なだけです。

  4. ただし、いつものRubyの書き方ではなくなります。この書き方を知っている人にはわかりやすいかもしれませんが、Rubyだけを知っている人には馴染みにくいでしょう。どちらを選ぶかはトレードオフを判断して決めてください。

  5. 連番が適切なときもありますが、配列などで表現することもできるのでそんなに多くはありません。