読者です 読者をやめる 読者になる 読者になる

リファクタリングのための回帰テストの書き方

ソフトウェア開発

 リファクタリングで定番のテスト活用方法として、変更前と変更後で挙動が変わってないことをテストコードで検証する、というものがある。違う用例で使われることもあるが、ここではそのテストを回帰テストと定義する。

 そうしたリファクタリングでの回帰テストとしては、大きく以下の2パターンが挙げられる。

  • 新しいコードの出力と古いコードの出力を比較するテストを書く。
  • 満たすべき仕様を検証するテストを書く。そして古いコードと新しいコード両方がそれをパスすることを確認する。

 今回は前者のテストをどう作っていくか、について扱っていきたいと思う。

簡単な場合

 新しいコードの出力と、古いコードの出力を比較するテストというのは、満たすべき仕様を検証するテストよりも、一般的に実装が容易であることが多い。というのも、そのアプローチでは、テスト対象の仕様をよく考えなくとも、カバレッジなどを基準に網羅性を高めることで必要なテストを確保できる場合があるからだ。例えば以下の様な、if文の2分岐のみでフローを構成する単純な関数を考える。

int foo(int bar1, int bar2, int bar3)
{
    int baz;
    baz = -1;
    if (bar1 == 0) {
        baz = bar2;
        if (bar3 == bar2 || bar3 == 0) {
            return bar3;
        } else {
            return bar2;
        }
    } else if (bar1 == 1) {
        baz = 0;
        if (bar2 == 1) {
            return 1;
        }
    } else {
        return bar1;
    }
    return baz;
}

 こうしたコードでは、仕様や意図を良く考えずに、ブランチカバレッジやコンディションカバレッジの網羅性を基準にテストコードを書いても問題がない場合がある。具体的には、例えば以下のようなテストの作成方法が十分実用に足ることがある。

  1. 関数の入出力をすべて抽出する。
  2. 関数内の条件式をすべて抽出する。
  3. すべての条件式の真偽の組み合わせを網羅するように、入力の組み合わせを用意する。
  4. その入力の組み合わせを入力し、出力を検証するテストコードを書く。

難しい場合

 ただ新旧比較の回帰テストであっても、実際は上記のように楽にテストコードが書けない場合も多々ある。その理由として、大きく以下の要因が挙げられると感じる。

フローが複雑すぎる場合がある

 上記の例ではあくまでif文のみで分岐が行われていたため、「すべてのフローを抽出して入力組み合わせを決定する」という手順が実用的なものとなっていた。ただこれにfor文やwhile文といったループ制御や、複雑な条件式が関わってくると、フローが爆発し、入力とフローの組をすべて網羅するという作業が非現実的なものになる。
 すなわち、フローの網羅性を基準にテストを実装しようにも、検証するフローと検証しないフローの区分けに迫られる場合があるといえる。

バグが存在する場合がある

 例えば以下のような関数を考える。

unsigned char bar(unsigned char max)
{
    unsigned char i, cnt = 0;/*ここでのunsigned charは8bit*/
    for (i = 0; i <= max; i++) {
        cnt += i;
    }

    ...

    return cnt;
}

 この関数では、引数maxに255を指定すると無限ループが発生する。そうしたバグは、そもそもテストの比較対象にできないため、テストを設計する際に除外しなければならない。
 すなわちきちんとコードをレビューしてバグを抽出しておかないと、テストが正常に動作しない場合があるといえる。

出力を比較できない場合がある

 例えば以下のような関数を考える。

struct sfoo {
    int a;
    int b;
};

struct sfoo bar(int mode)
{
    struct sfoo foo;

    if (mode == 1) {
        foo.a = 0;
        foo.b = 0;
    } else {
        foo.a = -1;
    }
    return foo;
}

 この関数では、引数modeに1以外の数を指定すると、初期化していないメンバ変数を含む構造体変数を返してくる。そこでは不定値を取るということでsfoo::bの比較が無意味になり、テスト時はそれを検証対象から除外しなければならない。
 すなわち、コードをレビューしてテストコードが有効に動作するかどうか検証しておかないと、テストが正常に動作しないリスクが出てくる。


 上記のような要素がテスト対象に含まれていると、どうしてもテスト対象の分析が必要になる。結果、新旧比較の回帰テストであっても、それなりの手間や難しさが発生することになる。

仕様や意図を読み取ってテストを書くということ

 上記の問題を考慮してテストを書くというのは、コード以外の、仕様や実装者の意図を読み取ってテストを書く、と言い換えることもできる。具体的にいえば「バグは仕様ではないためテストに含めない」「仕様上の機能を提供する制御フローのみテストする」といった考え方で、テストを作っていくイメージを指す。

 これはある意味合理的な考え方だと思う。というのも、仕様や実装者の意図を読まずコードの挙動のみを着眼点にしてテストを書くと、テストがコードの構造に強く依存してしまい、バグを引き継いだり、テストの再利用性や可読性を大きく貶めたりするリスクを増大させてしまうためだ。
 例えば、コンディションカバレッジ100%を基準にテストを書いた場合、以下のようなリスクが出てくる。

  • バグや比較不能な出力まで回帰テストで検証してしまう。
  • 修正などでテスト対象の条件式やフローが変化してしまうと、テストの設計意義が崩れてしまう。結果、何のためにその入力組み合わせを使っているか、といったものが、後から読み取りにくくなる。

 その欠点を考慮に入れると、実は冒頭のカバレッジベースのテスト実装方法は、実用に足るといっても、使える機会が限られているというのが分かる。カバレッジベースの実装が使えるほどテスト対象が明快なら、仕様や意図を読み取るのも容易だろうし、一方で仕様や意図が読み取れないほどテスト対象が難解なら、バグ等の恐れからカバレッジ基準による網羅的なテストなんて恐ろしくて手が出せなくなるためだ。


 結論として、リファクタリングでは、すべての挙動の等価性を完璧に検証する必要はない。
 変えてはいけないコードの挙動と、変わってしまってもよいコードの挙動を区別し、後者をテスト対象から切り捨てる作業は、テストコードの再利用性等を考慮するとどうしても避けることはできない。デベロッパーテストにおいて、カバレッジ最優のテスト設計や、全通りの入力組み合わせテスト等が良くないとよくいわれているのも、このような背景があるためなのだろう。

結局どうテストを書けばいいのか

 ではどうすればいいかというと、やはり変更前と変更後のコードの出力を比較するような簡単なテストであっても、意図する仕様をよく分析してテストを書いた方が良い、ということになる。具体的には、例えば以下のような手順が有効になるだろう。 

  1. コードレビューによってリファクタリング対象をよく分析し、分析結果をテストコードとして蓄積していく。具体的には、リファクタリング対象のフローや、リファクタリング対象の使われ方、異常系処理などを見ていく。そしてキャラクタライゼーションテストなどを使って、テストコードにその結果を記録していく。
  2. キャラクタライゼーションテストといった、検証対象を表現したテストコードが蓄積してきたら、それをリファクタリングのための回帰テストとしてまとめる。
  3. まとめた回帰テストを使ってリファクタリングを実施する。
  4. リファクタリング後のコードに、検証対象から除外した入力をassertなどを使って可視化する。

 最後の項目は、前述の関数を例に取ると、以下のようにassertを挿入する対処が該当する。

unsigned char bar(unsigned char max)
{

    assert(max < 255);/*リファクタリング後に追加。ありえない入力をassertで可視化する*/

    unsigned char i, cnt = 0;/*ここでのunsigned charは8bit*/
    for (i = 0; i <= max; i++) {
        cnt += i;
    }

    ...(リファクタリング後のコード)...

    return cnt;
}


 これはあくまで一例だが、テストコードの再利用性や可読性を重視したいのなら、何かしらこれと似たようなアプローチを取ることになると思う。結局のところ、テスト対象への理解なしには、良いテストコードは書けないということなのだろう。