読みやすさを重視した Jest の書き方 - おいちゃんと呼ばれています

f:id:inouetakuya:20191208183742p:plain

この記事は STORES.jp Advent Calendar 2019 - Adventar の 8日目の記事。

先日（11/26）STORES.jp 社内で Jest の勉強会を開催したので、そのときに話した内容を書く。

なお、私たち STORES.jp のフロントエンドチームが

Jest
Vue Test Utils
Vue.js

を採用しているので、それらを用いたテストコードを掲載しているが、大半の内容は上記以外のスタックを用いたテストにも当てはまるものだと思う。

アジェンダ

なぜ読みやすさを重視するのか？
読みやすくするためのヒント
書きやすくするためのヒント
おまけ

なぜ読みやすさを重視するのか？

さて、今回は「読みやすさ」を重視した Jest の書き方をお伝えする。なぜ「読みやすさ」を重視するのか、その背景について説明する。

よいテストとは？

まずはじめに、よいテストとは何か？について、次の 3つを備えているものがそれにあたると考えている。

信頼できること
読みやすいこと
書きやすいこと

まず、テストを書くべき対象にテストが書かれていて、それが通っていれば安心してリリースできるものになっていること（= 信頼できる）

次に、どのようなことをテストしているのか簡単に理解することができ、テストから仕様を理解するのも容易であること（= 読みやすい）

そして、適度に DRY であり、テストの追加や変更に、心が折れるほどの労力を必要としないこと（= 書きやすさ）

ただし、これらの 3つが等しく重要だとは考えていない。信頼できるものであることが最重要だと考えている。まずはこれを目指すべきで、それから読みやすさ、書きやすさの順で重要になると考える（後述）

テストの信頼性

幸いにも STORES.jp のフロントエンドチームでは、何をテストするか？について既に吟味されており、テストガイドラインという名のドキュメントが存在している。

そして、チームメンバー全員の認識が揃っている（ここがすごく重要で、認識が揃っているとコードレビューで「ここはテストすべきでは？」とかいうコメントをしなくて済む）

したがって、開催した勉強会ではこのあたりの話は割愛した。

簡単に紹介すると、テストガイドラインには、

複雑度、結合度、ビジネスとの関連性を基準にして、コンポーネントやストアのこの部分についてテストを書くとか
再利用するコンポーネントについてはビジュアルリグレッションテストを書いて UI 崩れに気付けるようにしようとか
複雑度の低いミューテーションとかはテストを書かなくてよいよねとか
みんな、このあたりのドキュメントは読んでおこうね

といったようなことがまとめられている。

読みやすさ vs 書きやすさ

テストの読みやすさと書きやすさのどちらがより重要かについては、実装のコードと同様に「書く回数よりも読まれる回数のほうが圧倒的に多いから」という理由で、読みやすさのほうが重要だと考える。

またテストコードは、実装のコードとは異なり、厳密に DRY でなくても、経験上、それほど困らない。例えば仕様の変更などがあって、それをコードに反映させる場合、実装のコードでは反映漏れがあれば、そこがすなわちバグとなるが、テストコードに反映漏れがあったところで、そのテストがこけるだけなので、漏れに気付いて修正するだけで済む。

一方で、テストコードを過度に DRY にすると、急激にテストが読みにくくなる。例えばテストの前提条件の記述がすごくスクロールしなければならないほど離れていたり、別ファイルに書かれていたりすると、test メソッドの中だけを見てもテストの意図と内容を把握できず、理解に余計な時間がかかったりする。

したがって、読みやすさと書きやすさが競合する場合には、読みやすさのほうを重視すべきであり、タイトルに「読みやすさを重視した」と入れているのはそのためである。

テストの実行速度は？

ところで、テストの実行速度について、これがあまりにも遅くて時間がかかると、開発速度に影響してくる。

しかしながら、現状では最新の master ブランチに対する CI のテスト実行時間は 39秒であり、まだテストの実行時間が問題になっているフェーズではない。

したがって、テストの実行速度については重視しない。

以上のような背景があり、今回、書きやすさや実行速度よりも、読みやすさを重視したテストの書き方をお伝えする。

読みやすくするためのヒント

テストを読みやすくするためのヒントとして、次の 3つについて書く。

beforeEach 再考
テストの構造化
差分を際立たせる

beforeEach 再考

beforeEach メソッドを使う典型的なケースは、下記のようにテストコードが重複しているときだと思う。

describe('handleSaveItem() が実行されたとき', () => {
  test('$notify.clear が実行されること', async () => {
    // --- (1) ここから ---
    const wrapper = shallowMount(MockComponent, {
      localVue,
      store: new Vuex.Store({ ... }),
      propsData: { ... },
      mocks: { $notify }
    });
    await wrapper.vm.handleSaveItem();
    // --- (1) ここまで ---

    expect($notify.clear).toHaveBeenCalled();
  });

  test('$notify.success が実行されること', async () => {
    // --- (2) ここから ---
    const wrapper = shallowMount(MockComponent, {
      localVue,
      store: new Vuex.Store({ ... }),
      propsData: { ... },
      mocks: { $notify }
    });
    await wrapper.vm.handleSaveItem();
    // --- (2) ここまで ---

    expect($notify.success).toHaveBeenCalledWith('アイテムを保存しました');
  });
});

(1) と (2) の箇所が重複している。

これを beforeEach に切り出すことで DRY にできる。

describe('handleSaveItem() が実行されたとき', () => {
  beforeEach(async () => {
    const wrapper = shallowMount(MockComponent, {
      localVue,
      store: new Vuex.Store({ ... }),
      propsData: { ... },
      mocks: { $notify }
    });
    await wrapper.vm.handleSaveItem();
  })
  
  test('$notify.clear が実行されること', () => {
    expect($notify.clear).toHaveBeenCalled();
  });

  test('$notify.success が実行されること', () => {
    expect($notify.success).toHaveBeenCalledWith('アイテムを保存しました');
  });
});

DRY にするためだけじゃない

しかしながら、beforeEach は DRY にするためだけのものではなく、テストの前提条件と、テストで確認したいこととを、明確に分けることができる。

describe('handleSaveItem() が実行されたとき', () => {
  beforeEach(async () => {
    const wrapper = shallowMount(MockComponent, { ... });
    await wrapper.vm.handleSaveItem();
  })
  
  test('$notify.clear が実行されること', () => {
    expect($notify.clear).toHaveBeenCalled();
  });
});

このことはテストで確認したいことがひとつだけの場合にも有効である。

テストの構造化

テストの前提条件と確認したいこととを明確に分けることができると、テストが構造化されて、スペック（仕様）のアウトラインが把握しやすくなる。

describe('handleSaveItem', () => {
  let wrapper;

  describe('saveItem が成功したとき', () => {
    beforeEach(async () => {
      saveItem = jest.fn().mockResolvedValue(data);
      wrapper = shallowMount(MockComponent, { ... });
      await wrapper.vm.handleSaveItem();
    })

    test('$notify.success が実行される', () => {
      expect($notify.success).toHaveBeenCalledWith('アイテムを保存しました');
    });
  });

  describe('saveItem が失敗したとき', () => {
    beforeEach(async () => {
      saveItem = jest.fn().mockRejectedValue(error);
      wrapper = shallowMount(MockComponent, { ... });
      await wrapper.vm.handleSaveItem();
    })

    test('レスポンスのエラーメッセージが $notify.error の引数として渡される', () => {
      expect($notify.error).toHaveBeenCalledWith(errorMessage);
    });
  });
});

テスト結果をドキュメント形式で出力している場合にも、アウトラインの把握のしやすさは顕著になる。

$ yarn test ItemEditForm.spec.js
...
handleSaveItem
  saveItem アクションが成功したとき
    ✓ $notify.success が実行されること
  saveItem アクションが失敗したとき
    ✓ レスポンスのエラーメッセージが $notify.error の引数として渡される
...

反対に、テストが構造化されていないと、確認したいことの記述が長くなって読みづらい。

$ yarn test ItemEditForm.spec.js
...
handleSaveItem
  ✓ saveItem アクションが成功したときには $notify.success が実行されること
  ✓ saveItem アクションが失敗したときには、レスポンスのエラーメッセージが $notify.error の引数として渡される
...

差分を際立たせる

対比させるような前提条件があった場合には、差分を際立たせるようにすると読みやすくなる。

まずは差分が際立っていない例から。

describe('handleSaveItem', () => {
  describe('saveItem が成功したとき', () => {
    beforeEach(async () => {
      // ここにダラダラ書くと前提条件の差分が際立たない
      const wrapper = shallowMount(MockComponent, {
        localVue,
        store: new Vuex.Store({
          modules: {
            items: {
              namespaced: true,
              // ...
            }
          } }),
        mocks: {
          // 差分はここなのだが...
          saveItem: jest.fn().mockResolvedValue(data)
        }
      });

      await wrapper.vm.handleSaveItem();
    })

    test('...', () => { ... });
  });

  describe('saveItem が失敗したとき', () => {
    beforeEach(async () => {
      // ここにダラダラ書くと前提条件の差分が際立たない
      const wrapper = shallowMount(MockComponent, {
        localVue,
        store: new Vuex.Store({
          modules: {
            items: {
              namespaced: true,
              // ...
            }
          } }),
        mocks: {
          // 差分はここなのだが...
          saveItem: jest.fn().mockRejectedValue(error)
        }
      });

      await wrapper.vm.handleSaveItem();
    })

    test('...', () => { ... });
  });
});

メソッド切り出しなどを利用して際立たせる（下記）

describe('handleSaveItem', () => {
  describe('saveItem が成功したとき', () => {
    beforeEach(async () => {
      const wrapper = getWrapper({
        saveItemResult: jest.fn().mockResolvedValue(data)
      });
      await wrapper.vm.handleSaveItem();
    })

    test('...', () => { ... });
  });

  describe('saveItem が失敗したとき', () => {
    beforeEach(async () => {
      const wrapper = getWrapper({
        saveItemResult: jest.fn().mockRejectedValue(error)
      });
      await wrapper.vm.handleSaveItem();
    })

    test('...', () => { ... });
  });
});

書きやすくするためのヒント

読みやすさを損なわないようにしつつ、書きやすくするためのヒントについても触れておく。

これは beforeEach に書くべき？test に書くべき？
適度な DRY で OK
todo メソッドの活用

これは beforeEach に書くべき？test に書くべき？

下記のコメントした箇所のように、テストの前提条件とも確認したい内容とも解釈できるものがある。これは beforeEach の中に書くべきか test の中に書くべきか迷う。

describe('saveButton がクリックされたとき', () => {
  beforeEach(() => {
    wrapper = shallowMount(MockComponent, { ... });
  });

  test('saveItem が実行されること', () => {
    wrapper.find('.save-button').trigger('click'); // この行は beforeEach に入れるべき？
    expect(saveItem).toHaveBeenCalled();
  });

  test('$notify.success が実行されること', () => {
    wrapper.find('.save-button').trigger('click');  // この行は beforeEach に入れるべき？
    expect($notify.success).toHaveBeenCalledWith('アイテムを保存しました');
  });
});

どちらにも解釈できると思うし、どちらに解釈しても間違いではないと思うが、私の場合は DRY にするために beforeEach の中に含めるようにしている。

describe('saveButton がクリックされたとき', () => {
  beforeEach(() => {
    wrapper = shallowMount(MockComponent, { ... });
    wrapper.find('.save-button').trigger('click'); // DRY にするために beforeEach に入れた
  });

  test('saveItem が実行されること', () => {
    expect(saveItem).toHaveBeenCalled();
  });

  test('$notify.success が実行されること', () => {
    expect($notify.success).toHaveBeenCalledWith('アイテムを保存しました');
  });
});

ただしテストで確認したいことが、トリガーの前後の「変化」を確認したいものである場合は test に書く。

describe('greenButton がクリックされたとき', () => {
  beforeEach(() => {
    wrapper = shallowMount(MockComponent, { ... });
  });

  test('color が red から green へ変わること', () => {
    expect(wrapper.vm.color).toBe('red');
    wrapper.find('.green-button').trigger('click');
    expect(wrapper.vm.color).toBe('green');
  });
});

参考）RSpec の change-from-to

なお RSpec ではトリガーの前後の「変化」を確認する構文として下記のようなものがある（読みやすくて好きだ）

expect {
  click_button('.green-button')
}.to change { color }.from('red').to('green')

適度な DRY で OK

前述したように、テストコードについては厳密に DRY を保たなくてもよいと考えている。

例を示すために「差分を際立たせる」の箇所で用いた例を再掲する。

describe('handleSaveItem', () => {
  describe('saveItem が成功したとき', () => {
    beforeEach(async () => {
      const wrapper = getWrapper({
        saveItemResult: jest.fn().mockResolvedValue({})
      });
      await wrapper.vm.handleSaveItem();
    })

    test('...', () => { ... });
  });

  describe('saveItem が失敗したとき', () => {
    beforeEach(async () => {
      const wrapper = getWrapper({
        saveItemResult: jest.fn().mockRejectedValue({})
      });
      await wrapper.vm.handleSaveItem();
    })

    test('...', () => { ... });
  });
});

ここで用いた getWrapper メソッドをより広い範囲で再利用しようとすると、より多くのユースケースに応えるために引数は多くなり、メソッドの行数も増える。

そのようなメソッドは内容を把握しづらく、メンテナンスもしづらいので適度な DRY で OK だと考える。

describe('ItemEditForm', () => {
  const getWrapper = ({ ... }) => {
    // より多くのユースケースに応えるために引数は多くなりメソッドの行数も増える
  }
  
  describe('handleSaveItem', () => {
    describe('...', () => {
      beforeEach(async () => {
        wrapper = getWrapper({ ... });
      })
    });

    describe('...', () => {
      beforeEach(async () => {
        wrapper = getWrapper({ ... });
      })
    });
  });

  describe('handleDeleteItem', () => {
    describe('...', () => {
      beforeEach(async () => {
        wrapper = getWrapper({ ... });
      })
    });

    describe('...', () => {
      beforeEach(async () => {
        wrapper = getWrapper({ ... });
      })
    });
  });
})

todo メソッドの活用

Jest には書く予定のテストをメモしておくための todo というメソッドが用意されていて、私の場合は実装前にこれを使って仕様を書き出してしまうことが多い。

describe('handleSaveItem', () => {
  describe('saveItem アクションが成功したとき', () => {
    test.todo('$notify.success が実行される');
  });

  describe('saveItem アクションが失敗したとき', () => {
    describe('axios のエラーのとき', () => {
      test.todo('Sentry にエラーが通知されない');
      test.todo('エラーメッセージが $notify.error の引数として渡される');
    });

    describe('axios のエラー以外のとき', () => {
      test.todo('Sentry にエラーが通知される');
      test.todo('エラーメッセージが $notify.error の引数として渡される');
    });
  });
});

テスト結果に todo の数が表示されるので、実装して todo を減らしていく。

$ yarn test ItemEditForm.spec.js
...
Tests: 5 todo, 10 passed, 15 total
...

このように進めると、実装の途中で「あれ？いまどこを実装しているんだっけ？」と迷子にならなくて済むという効用があるのでオススメ。

おまけ

beforeEach vs beforeAll
「時」と「とき」
「事」と「こと」

beforeEach と beforeAll

Jest には beforeEach とは別に beforeAll メソッドも用意されていて、テスト全体の実行時間を短くするのに役立つ。

しかしながら、現状、テストの実行時間が問題になっていないことと、テストの独立性を保つために、原則として beforeEach のほうを使っていこう。

describe('handleSaveItem', () => {
  beforeAll(() => {
    // 一度だけ実行される
    wrapper = getWrapper({ ... });
  })

  test('...', () => { ... });
  test('...', () => { ... });
});

なお describe 内の beforeAll が実行されるタイミングについては注意が必要。下記参照。

https://github.com/tomoyukikashiro/jest-sample/blob/master/__tests__/before_after.test.js

「時」と「とき」

時間そのものを表すときには「時」、場合を表すときには「とき」

（共同通信社記者ハンドブック）

describe('〇〇のとき', () => { ... });

「事」と「こと」

具体的な事象を指す場合は「事」、抽象的なものを指す場合は「こと」

（共同通信社記者ハンドブック）

test('〇〇であること', () => { ... });

まとめ

読みやすくするために
- beforeEach でテストの前提条件とテストの内容を分けよう
- テストを構造化して、仕様のアウトラインが把握しやすくしよう
書きやすくするために
- テストコードは適度な DRY で OK
- todo メソッドを活用しよう

Inspired by

なお、今回の内容は下記の本に強く影響を受けている。少し前に読み直す機会があって、あらためて良い本だと感銘を受けた。

Everyday Rails - RSpec による Rails テスト入門