LoginSignup
24
24
@_mi(み ")in株式会社Works Human Intelligence

レガシーコードにおける単体テストのハードルを乗り越える思考フロー（初心者向け）

Last updated at Posted at 2024-05-27

はじめに

ついこの前、経験の浅いメンバー向けに単体テストを書きまくってもらう研修を実施したのですが、経験が浅いと「どうすれば単体テストを書ききれるか？」の見通しがうまく立てられないんだろうな～という印象を受けました。

なので、できるだけ体系立てて説明ができるように記事に残そうと思います。

想定読者

  • 単体テスト経験の浅いメンバー（入社1~2年目くらい）
  • レガシーコードの単体テストに苦手意識を持っている人

大前提

  • 前提１：とってもレガシーなコードである
  • 前提２：「これでだいたいのテストは書ける、しかし良いテストかどうかは分かりません」

20年物のレガシーコードなので、正直、難易度めっちゃ高いと思います。レガシーコードへの単体テストは特殊技能と言っても過言じゃないです。

また、最初から良いテストを目指さなくていいと考えています。良いテストなのかどうかを考えるのは、まず「なんでもテスト書けるぜ！」の状態を満たした後でよくて、というのはテストをいろいろ書いた経験がないとそのあとの議論が腹落ちしにくいと思うからです。

もくじ

基本の思考フロー

初心者や慣れていない人がやりがちなのは、とりあえず書いて試しているんだけど途中でよくわからなくなるパターン。「全体の中のどこでハマっているのか？」が理解できればやることは決まってくるし、詰んでも人に質問しやすいです。全体と個別をイメージしましょう。

以下、６ステップでざっくり解説します。

step１．テスト対象メソッドの振る舞いを把握する

ここがちゃんと認識できていない場合も多いです。『このメソッドの振る舞いって何？』をしっかりと見極めれば、自然とテスト観点が分かります。

単体テストで検証したい対象としては、だいたい２つのパターンしかないと思っています。

  • １．戻り値を確認するテスト
    • メソッド引数を渡して返される結果を検証する（引数がない場合も当然ある）
    • 検証対象は「戻り値」
  • ２．状態を確認するテスト
    • 状態とは以下のようなもののこと
      • テスト対象が持つ変数やオブジェクトなど
      • テスト対象クラスに関係するオブジェクト
      • データベースやファイルシステムなど外部への依存
    • 検証の対象は「テスト対象オブジェクトの変更された状態」

※ たまに１メソッド内でどっちもやっちゃっている犯罪メソッドもあるので注意

ところで『振る舞いってなに？』については以下読んでみてください。

また、振る舞いを正しく理解しておかないと加えたコードの変更が意図しない仕様変更につながるので要注意です。（リファクタリングは振る舞いを変えずに内部実装を変えること！）

step２．１つのテストケースを通すためのイメージを持つ

1単位の振る舞いを網羅した仕様化テスト（テストスイート）は結局1つ1つのテストケースの集合体です。なのでまず1本テストケースを Green にするところを目指しましょう。

１つのテストケースを作成するうえで必ず覚えておいてほしいのが、単体テストの3A（Arrange-Act-Assert）になります。

  • Arrange：準備（対象クラスをインスタンス化できるか）
  • Act：実行（対象メソッドを実行できるか）
  • Assert：検証（検証できるか）

なので、1つのテストケースが最終的にこの形になるように意識します。

step３．まずはコンパイラを通す

さて、テストを書いていくのですが、経験が浅いとコンパイルエラーで詰みます。IDE は Quick Fix としていくつか提案してくれますが、その正しさが分からないことも多いと思うからです。なのですが、１つ１つの目的に向かって書き進めていく意識があればなんてことはありません。単体テストの3Aパターンで考えると思考が整理されると思います。

  • Arrange：テスト対象クラスのインスタンス化
  • Act：テスト対象メソッドの実行
  • Assert：実測値と期待値の比較
MyClassTest.java
public class MyClassTest {

    /**
    * AAAパターンで書けるようにAAAを１つ１つクリアしていく意識を持つ
    */
	@Test
	void test() {
        // Arrange：準備（テスト対象クラスのインスタンス化）
		MyClass myClass = new MyClass();

        // Act：実行（テスト対象メソッドの実行）
		String actual = myClass.doSomething();

        // Assert：検証（実測値と期待値の比較）
		assertThat(actual, is("hoge"));
	}
 }

また、詰みポイントへの対処は後述します。（参照：コンパイラを通すフローチャート）

step４．テストを実行してみてNPEを解消する

コンパイラは許してくれたけど、実行してみたら容赦なく Null Pointer Exception がやってくる。たぶんこの辺で気持ちが折れます。

  • 必要なオブジェクトが初期化されておらず NPE
  • staticメソッドでシングルトンインスタンスを呼んでて NPE
  • 外部の共有依存（DBやファイルシステム）を呼ぼうとして NPE

だいたいこんなものかと思いますが、ここでまず考えるべき問いは『テストクラスからテスト対象クラスを呼び出したときにXXで落ちる、その原因は？』です。

  • アクセスしたインスタンスが初期化されていないことが原因ならば、そのインスタンスを初期化しているメソッドを呼び出す
  • アクセスしたフィールド変数がnullであることが原因ならば、変数に値をセットしているメソッドを呼びだす

すごい抽象的な話で恐縮なのですが（説明難しすぎる）、どこかでオブジェクトが使われているならば必ずどこかに初期化している処理が存在します。getter があるなら必ず setter がある、みたいな（setter の形とは限らない）イメージです。まずはプロダクトコードやスタックトレースを確認して、どこで値の初期化ができるのかを確認しましょう。

次点で考えるべきは、『テストクラスから任意の値を差し込めないか？』です。上記を試してみても初期化ができない（シングルトンインスタンスやデータベースなどに依存している場合、突破しにくい）、初期化がうまくいったとしてもそこに至るまでのコード量が多くなるなどのケースも多いです。なので思考を切り替えて、テスト対象メソッドの中の 依存したくない処理をテストクラスで書き換えてしまおう、という考えにシフトします。

ここで大切なのは「接合部」の考え方になります。（以下記事読んでください）

「いやいや、これプロダクトコード汚してますやん」という意見もあるかと思いますが、デグレードの責任は誰もとってはくれません。レガシーなコードにおいて、プロダクトコードの変更はそのコードの複雑さから影響範囲調査のコストはかなりのものになります。フィードバックのスピードを速め、安全にコードに変更を加えるには頑張って単体テストを書くのが最適だと思っています。（E2Eテストじゃフィードバックが遅すぎる）

また、単体テストはあくまでプロダクトの品質を向上させていくための最初のステップです。単体テストを書いて終わりではなく、それを使って安全に変更を加えていくという点で書いていくべき派です（コードをあまりにも汚している場合はチームのレビューではじけますし）根本の原因（つまり設計レベル）を解消するためにも結局単体テストは必要不可欠なので。

そんなこんなで長くなりましたが、Null Pointer Exception は以下の2点でほとんど解消できるはずです。

  • NPEの原因になっているオブジェクトを初期化する処理を探して呼び出す
  • NPEの原因になっているオブジェクトをテストクラスで置き換える

step５．振る舞いを網羅する

NPEの解消まで済んで1本テストケースが Green になったら、他のテストケースを作成してテスト観点を網羅していきます。

１メソッドの１単位の振る舞いが網羅できたら仕様化テストの完成です。

step６．テストコードのリファクタリング

重複するコードを共通化したり、仕様化テストの可読性を高める作業をします。

最後にではなく、step5とstep６は往復していってもいいですね（テスト駆動開発ではそうしてるし）。テスト対象クラスのコンストラクタ引数の作成（Arrangeの実行のためのsetup）やメソッド実行のための引数の作成（Actの実行のためのsetup）なんかもまとめられるところがあれば、@BeforeEach アノテーションによる共通化や、テストクラスのprivateメソッドとして処理を切り出してもいいでしょう。

共通化しない方がいいケースもあったりするので、そこは臨機応変に対応してください。

コンパイラを通すフローチャート

これですべてを網羅できるわけではありませんが、基本的な思考のフローは辿れていると思います。

Arrange-Act-Assert １つずつクリアしていきましょう。

Arrange：絶対にインスタンス化するフローチャート

Arrangeの部分.png

テスト対象メソッドのメソッド単位が大きい

モンスターメソッドからテストしたい部分の処理を抽出して対応する。

テスト対象がstaticメソッド

クラス名.メソッド名でActできるので、このステップはスキップしてOKです。

コンストラクタ引数がある

null を渡しておいて大丈夫そうならそれでOK。

コンストラクタに渡した変数をテストしたいメソッドが使っている場合も、コンパイラに怒られずにテストケース１本（3Aパターン）書ききってしまってから修正したらいいです。

テスト対象クラスが抽象クラスである

抽象クラスを継承するテスト用の具象クラスを新たに作成して単体テストを書くことをオススメします。そういう意味で、新たに１つクラスを作成するでもいいし、抽象クラスのテストクラス内にインナークラスを作成するでもいいと思います。

Act：絶対にメソッド実行するフローチャート

Actの部分.png

テスト対象メソッドの可視性がprivateである

単体テストの考え方/使い方 において、private メソッドのテストをすることは「単体テストのアンチ・パターン」として紹介されていますが、個人的には必要に応じて書くべき派です。理由は簡単で、private なメソッドもそれを呼び出す public なメソッドも巨大で複雑なため、分岐の網羅が困難、かつ、そのためテストを書いてからコードの変更をしないとデグレードを引き起こすからです。

弊組織では可視性を上げたメソッドに対して @VisibleForTesting アノテーションを付与することで、「テストコードのために可視性を上げている」ことを第三者に示しています。（@VisibleForTestingがもしプロジェクトに組み込まれていない場合コンパイルエラーが発生する可能性があるかもです。）

独り言：単体テストの考え方/使い方、分かるんだけどレガシーコードにはきつい...。（たぶんレガシーコード前提にしてないでしょこの本）

Assert：絶対に検証するフローチャート

Assertの部分 .png

期待値がエラーである

assertThrows を利用します。

引数で渡したオブジェクトの状態変化が検証の対象である

例えば引数で渡した StringBuilder 型のオブジェクトの状態の変化をテストしたい場合は以下のようできます。

MyClass.java
public class MyClass {

    // テスト対象メソッド
	void setMoji(StringBuilder moji) {
		moji.append("hoge");
  		moji.append("fuga");
    	moji.append("piyo");
	}
 }
MyClassTest.java
public class MyClass {

    @DisplayName("引数で渡したオブジェクトの変更を検証する例")
    @Test
	void setMojiTest() {
		// Arrange
        MyClass myClass = new MyClass();
        StringBuilder actual = new StringBuilder();
        
        // Act
        myClass.setMoji(actual);
        
        // Assert
        assertThat(actual.toString(), is("hogefugapiyo"));
	}
 }

テストクラスで生成したオブジェクトを引数として渡せる場合は、こんな感じで検証可能です。

ログのテストである

そもそもログのテストについてですが、テスト対象とするログ出力が、アプリケーションの観察可能な振る舞い（ユーザーや非開発者が見ることを想定）の一部なのか、それとも実装の詳細（開発者しか見ない）なのかによって変わってきます。

  • 観察可能な振る舞い：必ずテストする
  • 実装の詳細：個人的にはどっちでもいい

また、テスト方法についてはセンパイに相談してください（各プロジェクト・各チームで違いそうだし）

メソッド内外でインスタンス化されたオブジェクトの状態をいじっている

ケース１：メソッド外部でインスタンスが生成されており、かつ、setterがない場合

MyClass.java
public class MyClass {

    private List<String> list;

    private Object doSomething(){
        // いろんな処理があって呼びたくない
        list = new ArrayList<>();
        // いろんな処理があって呼びたくない
    }

    // テスト対象メソッド
    public void setHoge() {
        list.add("hoge");
    }
}

こうします。（それ以外だと setter をつくるかリフレクションしかない？）

MyClass.java
import com.google.common.annotations.VisibleForTesting;

public class MyClass {

    private List<String> list;

    private doSomething(){
        // いろんな処理があって呼びたくない
        generateList(); // メソッド抽出
        // いろんな処理があって呼びたくない
    }

    // メソッド抽出
    @VisibleFortesting
    void generateList(){
        list = createList();
    }

    // 接合部をつくる
    @VisibleFortesting
    List<String> createList(){
        return new ArrayList<>();
    }

    // テスト対象メソッド
    public void setHoge() {
        list.add("hoge");
    }
}

テストを書きます。

MyClassTest.java
public class MyClassTest {

    private List<String> listForTest; 

    @Test
    void test() {
        // Arrange
        listForTest = new ArrayList<>() // 差し込む用にListをインスタンス化
        MyClass myClass = new MyClass(){
            @Override
             List<String> createList(){ // 匿名クラスをつくって値を差し込む
                return listForTest;
            }
        }
        
        // Act
        // createList()の振る舞いをテストクラスで変えているので
        // テスト対象インスタンスのフィールドにはlistForTestがセットされる
        myClass.generateList(); 
        myClass.setHoge();
        
        // Assert
        assertThat(listForTest.get(0), is("hoge"));
    }
}

今回のケースは2段階にわたって新規メソッドとして抽出しているのであんまりない例ではあるものの、巨大メソッドから責務を分離するためには同様のことをしなくてはならないこともありますね。

ケース２：内部でインスタンスを生成しており、かつ、getterがない場合

MyClass.java
public class MyClass {

    private List<String> list;

    // テスト対象メソッド
    public void setFuga() {
        list = new ArrayList<>();
        list.add("fuga");
    }
}

こうします。（それ以外だと getter をつくるかリフレクションしかない？）

MyClass.java
import com.google.common.annotations.VisibleForTesting;

public class MyClass {

    private List<String> list;

    // テスト対象メソッド
    public void setFuga() {
        list = createList();
        list.add("fuga");
    }

    // 接合部をつくる
    @VisibleFortesting
    List<String> createList(){
        return new ArrayList<>();
    }
}

テストを書きます。

MyClassTest.java
public class MyClassTest {

    private List<String> listForTest; 

    @Test
    void test() {
        // Arrange
        listForTest = new ArrayList<>() // 差し込む用にListをインスタンス化
        MyClass myClass = new MyClass(){
            @Override
             List<String> createList(){ // 匿名クラスをつくって値を差し込む
                return listForTest;
            }
        }
        
        // Act
        myClass.setFuga();
        
        // Assert
        assertThat(listForTest.get(0), is("fuga"));
    }
}

プロダクトコードの振る舞いを変えずにテストが書けました。

NPEの解消へ

ここ読んでみてください：step４．テストを実行してみてNPEを解消する

まとめ

結論、単体テストの3A（Arrange-Act-Assert）だけ覚えてたらOKです。（まとめオワリ）

レガシーコードの単体テスト、すごく書きにくいのは分かりますがやっぱりやらないと身につきませんね。単体テストを書くことで、『テストしにくすぎる』『改善しないとだめだー』『改善するとすればどういう風に作り変えるべきだ？』という気持ちが芽生えるのでおすすめです。

また、元も子もない発言ですが単体テストではなくE2Eテストでテストしてもいいとは思いますので使い分けですね。ただ、単体テストが書けると確実に生産性が上がる（フィードバックがすぐに得られるため）ので、レガシーコードに単体テストを書けるという特殊技能は身に着けて損ないです。

自分もまだまだなので、一緒に頑張りましょう。

おわりに

唐突に書きはじめて1日で書ききったので、おかしいこと言ってるかもしれませんがそのときは指摘してください。

24
24
0

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
Sign upLogin
24
24