テストの可読性と保守性を改善したいよねって話

この記事は [NIJIBOX Advent Calendar2019](https://qiita.com/advent-calendar/2019/nijibox) の20日目の投稿です。

#背景

この記事は[「仕様の変更に強いコードを書きたいよねって話」](https://qiita.com/JSON_HardCoder/items/09b7c3136bdad36c4830)のテストについて掘り下げたお話になります。

題材は「[ページネーションにおける関数](https://qiita.com/JSON_HardCoder/items/09b7c3136bdad36c4830#%E4%BB%8A%E5%9B%9E%E3%83%95%E3%82%A9%E3%83%BC%E3%82%AB%E3%82%B9%E3%81%99%E3%82%8B%E3%81%A8%E3%81%93%E3%82%8D)」です。
※ 以下```current```は現在いるページ、```total```は総ページ数、```size```はページネーションの表示するページサイズを指します。

#書くこと
- ページネーションのロジック部分のgetPageNums関数のテストコードがわかりにくかったのでクラス設計を導入し、修正した。
- テストコードを書くときに気をつけたいぞい！ってこと

#参考にしたもの
- テスト駆動開発(TDD)の第一人者のtwadaさんにアドバイスをいただきました、ありがとうございました！
- 書籍[「リーダブルコード ―より良いコードを書くためのシンプルで実践的なテクニック」](https://www.oreilly.co.jp/books/9784873115658/)


#手順

テストには、Facebook社がOSSとして開発を進めている [Jest](https://jestjs.io/docs/en/getting-started.html) を使っています。

##今回ターゲットにしている関数について

ページネーションを構成するのは前に戻るボタンと 次へ進むボタンに、分割されたページへのリンクで構成されています。今回は分割されたページへのリンクを作るところに焦点を当てています。

以下のページネーションは、現在いるページが6ページ目でリンク表示数が10つ、かつ総ページ数が10ページ以上の時のものです。
現在いるページ、リンク表示数、総ページ数という3つの条件から1~10の数字の配列を取得できれば、ページネーションは実装できます。この配列を取得する関数(以下、getPageNums関数)を用いたテストに関するお話です。

![googlePN.png](https://qiita-image-store.s3.ap-northeast-1.amazonaws.com/0/528613/86ff2a28-4b18-e9d7-0152-07bfb80d90b9.png)
[Google](https://www.google.com/)より引用


##既存のテストコード

```ts:getPageNums.test.ts

import assert from "assert";
import { getPageNums } from "../pagination";

describe("pagination", () => {
  test("current = 3, total = 5, size = 5 の時", () => {
    const expected = [1, 2, 3, 4, 5];
    const result = getPageNums(3, 5, 5);
    assert.deepStrictEqual(result, expected);
  });

  test("current = 4, total = 6, size = 5 の時", () => {
    const expected = [2, 3, 4, 5, 6];
    const result = getPageNums(4, 6, 5);
    assert.deepStrictEqual(result, expected);
  });

  test("current = 3, total = 8, size = 6 の時", () => {
    const expected = [1, 2, 3, 4, 5, 6];
    const result = getPageNums(3, 8, 6);
    assert.deepStrictEqual(result, expected);
  });

  test("current = 4, total = 8, size = 6 の時", () => {
    const expected = [2, 3, 4, 5, 6, 7];
    const result = getPageNums(4, 8, 6);
    assert.deepStrictEqual(result, expected);
  });

  test("current = 4, total = 8, size = 3　の時", () => {
    const expected = [3, 4, 5];
    const result = getPageNums(4, 8, 3);
    assert.deepStrictEqual(result, expected);
  });
});
```

以下、テスト結果

```
jsonHardCorder$ npx jest -c jest.config.js src/server/services/util/__tests__/getPageNums.test.ts
 PASS  src/server/services/util/__tests__/getPageNums.test.ts
  util::pagination
    ✓ current = 3, total = 5, size = 5 の時 (2ms)
    ✓ current = 4, total = 6, size = 5 の時
    ✓ current = 3, total = 8, size = 6 の時
    ✓ current = 4, total = 8, size = 6 の時 (1ms)
    ✓ current = 4, total = 8, size = 3 の時

Test Suites: 1 passed, 1 total
Tests:       5 passed, 5 total
Snapshots:   0 total
Time:        1.854s, estimated 8s
Ran all test suites matching /src\/server\/services\/util\/__tests__\/getPageNums.test.ts/i.
```
テスト自体は通っていることはわかると思いますが、テストの実行結果から何をしているのかが汲み取りづらいと思います。

まず、満たすべき条件が5つありますが`これがなぜ満たさないといけない条件なのか`がわかりづらい。また、条件が同じレベル感になっているのでどこに重要性があるかもわかりづらいです。

次に、テスト結果だけを見た人はテストしている関数に精通しているとは限らないため、引数や返り値がどういったものを指しているのかが不明瞭です。


##このテストコードのどこがダメなのか？

###肝となる条件がわかりづらい

このgetPageNums関数がどういう条件を満たせば良いのか、作成した自分自身はわかりますが、テストをした他の人（3ヶ月後の自分も）はなぜこの条件を満たせば良いとなったのかがわからないです。

まず条件を整理して、言語化する必要がある。
上記5件のパターンがどういう条件かを洗い出していく。

※ prev は ページネーションの「前へ」、next は「次へ」
<img width="358" alt="スクリーンショット 2019-12-20 19.10.19.png" src="https://qiita-image-store.s3.ap-northeast-1.amazonaws.com/0/528613/7b6ae55a-68b8-b63a-15aa-f0118f66d369.png">
[Google](https://www.google.com/)より引用


**・current = 3, total = 5, size = 5 の時**
currentが3で、size幅が奇数のときは prev と next が均等になる。
``` [1, 2, 3, 4, 5] ```になると良い。

**・current = 4, total = 6, size = 5 の時**
currentが4で、size幅が奇数のときは prev と next が均等になる。
``` [2, 3, 4, 5, 6] ```になると良い。

**・current = 3, total = 8, size = 6 の時**
currentが3で、size幅が偶数のときは prev より next が多くなる。
``` [1, 2, 3, 4, 5, 6] ```になると良い。

**・current = 4, total = 8, size = 6 の時** 
currentが4で、size幅が偶数のときは prev より next が多くなる。
``` [2, 3, 4, 5, 6, 7] ```になると良い。

**・current = 4, total = 8, size = 3　の時**
currentが4で、size幅が奇数のときは prev と next が均等になる。
``` [3, 4, 5] ```になると良い。

条件を言語化すると、以下のことがわかりました。

・size幅が奇数のときは current を中心にして同じ数の配列が存在するので、条件としてはわかりやすいです。
・**size幅が偶数 のときは current を中心に prev 方向より next 方向 が多くなる**、といった条件はわかりづらい、言語的なアシストがないとテスト実行者は意図を汲み取りづらい可能性があります。



###テスト結果が簡潔すぎて見てもわからない

実行結果が、``` ✓ current = 3, total = 5, size = 5 の時 (2ms)
```だけだと、テストが通ったことはわかるが、説明が皆無のため、読み手側はソースコードを読みに行かなければならなくなります。結果、時間が無駄にかかってしまいます。コメントがテスト結果に出力されていれば、理解を助けることができます。

なので、コメントを入れて、わかりやすいテスト結果になるように変更していきたいと思います。


##クラス設計を導入する
テストコードの修正にあたり、言語化が難しかったり、関数の処理の責務とタイミングが異なることに気づき、クラス化という形にリファクタした方が可読性が上がるのでは？と思い、書き換える判断に至りました。

以下では、条件の中でもテストコードを読む人が一番わかりづらい`size幅が偶数のとき`の2つの条件にフォーカスを当てて、テストコードを書いていきます。

###1. class化する。
今回は、引数に使われている部分を共通部分をくくり出して、まとめます。

###2. わかりやすくコンストラクタを定義する
ページネーションの表示するページサイズは```size```でしたが、もう少しわかりやすい変数名に変えたいです。今回は```windowSize```にリネームし、```total```は```totalPages```にリネームしています。

```
this.windowSize = windowSize;
this.totalPages = totalPages;
```
###3. 共通の処理であるヘルパー関数を作る。

``windowSize``と```totalPages```は各条件の```describe```で使いまわしたいです。共通部分をまとめる、ヘルパー関数を作成します。


```ts:getPageNums.test.ts
//ページネーションの表示をサポートする Paginator クラス
class Paginator {
  windowSize: number;
  totalPages: number;
  constructor(windowSize: number, totalPages: number) {
    this.windowSize = windowSize;
    this.totalPages = totalPages;
  }
```

###4. paginatorを定義する。
```ts:getPageNums.test.ts
describe("ページネーションの表示をサポートする Paginator クラス", () => {
  let paginator: Paginator;
}
```

###5. paginatorインスタンスをnewして、beforeEachする。
各テストの開始前に paginator のインスタンスを作ります。
以下に```test()```を書くのですが、その前に Paginator のインスタンスを new したいので、```beforeEach()```を使います。

```ts:getPageNums.test.ts
describe("総ページ数 8, ウィンドウ幅 6 の Paginator の場合", () => {
    beforeEach(() => {
      paginator = new Paginator(6, 8);
    });
}
```

###6. totalPagesとwindowSizeを説明的に記述する

`assert.deepStrictEqual（）`メソッドを使って第1引数の paginator と 第2引数の子要素が`===`かどうかを判定します。

```ts:getPageNums.test.ts

beforeEach(() => {
      paginator = new Paginator(6, 8);
    });
    test("#totalPages は 8", () => {
      assert.deepStrictEqual(paginator.totalPages, 8);
    });
    test("#windowSize は 6", () => {
      assert.deepStrictEqual(paginator.windowSize, 6);
    });
```

###7. getPageNums() → getWindow() をテストする

getPageNums()は、直感的に何をやっている関数かが、わかりづらいです。なのでリネームし、getWindow()にします。windowの方がページネーションの遷移先の配列であることがわかりやすいです。

この時に、describeにどういう条件でのテストかの説明を記述します。
resultに`getWindow`を代入して、変数化します。
`totalPages`と`windowSize`が等しいかをテストしたように、同じく、 result も等しいかテストします。

```ts:getPageNums.test.ts
describe("#getWindow は引数で渡されたページ番号のウィンドウを返す。幅が偶数のときは prev より next が多くなる", () => {
      test("3 ページ目は [1, 2, 3, 4, 5, 6] になる", () => {
        const result = paginator.getWindow(3);
        assert.deepStrictEqual(result, [1, 2, 3, 4, 5, 6]);
      });
      test("4 ページ目は [2, 3, 4, 5, 6, 7] になる", () => {
        const result = paginator.getWindow(4);
        assert.deepStrictEqual(result, [2, 3, 4, 5, 6, 7]);
      });
});
```

###8.全体コード

以下のコードを見ていただきたいのですが、「総ページ数 8, ウィンドウ幅 6 の Paginator の場合」と「getWindow は引数で渡されたページ番号のウィンドウを返す。幅が偶数のときは prev より next が多くなる」と2段にしたことにより、詳細度が分かれて、可読性が上がっています。

```ts:Paginator.test.ts

import assert from "assert";

​
//共通の処理であるヘルパー関数
class Paginator {
  windowSize: number;
  totalPages: number;
  constructor(windowSize: number, totalPages: number) {
    this.windowSize = windowSize;
    this.totalPages = totalPages;
  }
 getWindow(pageNum: number) {
    // 1 から total までの配列を作る
    const s = pageNum - this.windowSize / 2 < 0;
    const e = pageNum + this.windowSize / 2 > this.totalPages;
    const start = s
      ? 0
      : e
      ? this.totalPages - this.windowSize
      : pageNum - this.windowSize / 2;
    const end = s
      ? this.windowSize
      : e
      ? this.totalPages
      : pageNum + this.windowSize / 2;
    const ranges = this.range(start, end);
    return ranges;
  }
  range(start: number, end: number) {
    const r = [];
    for (let i = Math.floor(start); i < Math.floor(end); i++) {
      r.push(i + 1);
    }
    return r;
  }
}
​
describe("ページネーションの表示をサポートする Paginator クラス", () => {
  let paginator: Paginator;
​
  describe("総ページ数 8, ウィンドウ幅 6 の Paginator の場合", () => {
    beforeEach(() => {
      paginator = new Paginator(6, 8);
    });
    test("#totalPages は 8", () => {
      assert.deepStrictEqual(paginator.totalPages, 8);
    });
    test("#windowSize は 6", () => {
      assert.deepStrictEqual(paginator.windowSize, 6);
    });
    describe("#getWindow は引数で渡されたページ番号のウィンドウを返す。幅が偶数のときは prev より next が多くなる", () => {
      test("3 ページ目は [1, 2, 3, 4, 5, 6] になる", () => {
        const result = paginator.getWindow(3);
        assert.deepStrictEqual(result, [1, 2, 3, 4, 5, 6]);
      });
      test("4 ページ目は [2, 3, 4, 5, 6, 7] になる", () => {
        const result = paginator.getWindow(4);
        assert.deepStrictEqual(result, [2, 3, 4, 5, 6, 7]);
      });
    });
  });
});


```

##テストを実行

testをPASSした時。

``` 
HOGEHOGE:jsonHardCorder$ npx jest -c jest.config.js src/server/services/util/__tests__/pagination.test.ts
 PASS  src/server/services/util/__tests__/pagination.test.ts (6.866s)
  ページネーションの表示をサポートする Paginator クラス
    総ページ数 8, ウィンドウ幅 6 の Paginator の場合
      ✓ #totalPages は 8 (2ms)
      ✓ #windowSize は 6
      #getWindow は引数で渡されたページ番号のウィンドウを返す。幅が偶数のときは prev より next が多くなる
        ✓ 3 ページ目は [1, 2, 3, 4, 5, 6] になる (1ms)
        ✓ 4 ページ目は [2, 3, 4, 5, 6, 7] になる

```

以下は、testがエラーを起こした時。
総ページ数が 8 /ウインドウ幅が ６ /現在いるページが 3 の時の条件の時にテストが落ちていることが一目でわかります。
(わざと期待される値をいじってエラーさせています。) 

``` 
 FAIL  src/server/services/util/__tests__/pagination.test.ts (6.418s)
  ページネーションの表示をサポートする Paginator クラス
    総ページ数 8, ウィンドウ幅 6 の Paginator の場合
      ✓ #totalPages は 8 (1ms)
      ✓ #windowSize は 6 (1ms)
      #getWindow は引数で渡されたページ番号のウィンドウを返す。幅が偶数のときは prev より next が多くなる
        ✕ 3 ページ目は [1, 2, 3, 4, 5, 6] になる (6ms)
        ✓ 4 ページ目は [2, 3, 4, 5, 6, 7] になる (1ms)

  ● ページネーションの表示をサポートする Paginator クラス › 総ページ数 8, ウィンドウ幅 6 の Paginator の場合 › #getWindow は引数で渡されたページ番号のウィンドウを返す。幅が偶数のときは prev より next が多くなる › 3 ページ目は [1, 2, 3, 4, 5, 6] になる

    assert.deepStrictEqual(received, expected)

    Expected value to deeply and strictly equal to:
      [1, 2, 3, 4, 5, 6, 7]
    Received:
      [1, 2, 3, 4, 5, 6]

    Difference:

    - Expected
    + Received

      Array [
        1,
        2,
        3,
        4,
        5,
        6,
    -   7,
      ]

      46 |       test("3 ページ目は [1, 2, 3, 4, 5, 6] になる", () => {
      47 |         const result = paginator.getWindow(3);
    > 48 |         assert.deepStrictEqual(result, [1, 2, 3, 4, 5, 6, 7]);
         |                ^
      49 |       });
      50 |       test("4 ページ目は [2, 3, 4, 5, 6, 7] になる", () => {
      51 |         const result = paginator.getWindow(4);

      at Object.<anonymous> (src/server/services/util/__tests__/pagination.test.ts:48:16)

Test Suites: 1 failed, 1 total
Tests:       1 failed, 8 passed, 9 total
Snapshots:   0 total
Time:        8.429s
```

コードを言語化することで、関数を実際に読んでいない人でも、どういうケースのテストをしているのかが、わかりやすいです。テストコードが読みやすくなれば、テストも書きやすくなるし、チームのメンバーがテストを追加しやすくなります。結果、コード設計が全体的に改善できます。


#展望
・登場する数字が多い。
・同じ型の引数が多い。
・total - sizeといったものが三項演算子に入ってくるとわかりづらい。
・このtotal - sizeを説明変数にする。
・変数は多くて良いのでもっと変数化していく。
・上記を考慮して、関数自体のリファクタが必要である。

#今回の学びの背景にあるもの
https://www.amazon.co.jp/dp/4274217884
https://www.amazon.co.jp/dp/4274224546

これらの書籍からテストについてもっと学びたいぞい。