🙄

怪奇な NullPointerException はどこから来た？

2022/12/16に公開

Java

JUnit

tech

同僚から「CI のみで JUnit による単体テストが失敗するので診てほしい」と頼まれたので調べた話。良い設計とか良いテスティングを考えるために敢えてちょっとストーリー仕立てにしています。興味があればお付き合いください。

起きるはずのない `NullPointerException`

実際のコードはもっと込み入っていたのですが、簡単に書くと、失敗して問題となっていたのは以下のようなテストコードでした。

  @Test
  void testListWeekends() {
    Weekends result = DateUtils.listWeekends("20221201", "20221215"); // ★
    assertThat(result).isEqualTo(...); // ★★
  }

CI プラットフォームの出力やローカルでの動作確認を踏まえると、以下のような状況でした。

ローカルで IDE を通して当該テストを実行した場合には成功する。
Pull Request をトリガーとした CI プラットフォーム上でのテスト実行で必ず失敗する。
- ビルドキャッシュを利用しているプロジェクトだったので、試しに Invalidate Cache をした上で再度実行したが、結果は変わらずだった。
テストの失敗は、★★ の行から発生していた NullPointerException が原因だった。
ローカルでビルドツールを通じて全テストを実行したときには成功する。

不思議だったのはテスト失敗の原因となった NullPointerException の出どころです。どう見ても null たり得るのは result 変数のみ。つまりは DateUtils#listWeekends が null を返しているに違いないのです。当然のように DateUtils の実装を確認します。

class DateUtils {
  /** 期間中の週末にあたる日付を抽出します */
  public static Weekends listWeekends(String since, String until) {
    List<LocalDate> weekends = Lists.newArrayList();
    
    for (...) { // 詳細のロジックは省略します。
      weekends.add(...);
    }
    
    return Weekends(weekends);
  }
}

record Weekends(List<LocalDate> list) { ... }

見ての通り、return している weekends 変数に null が代入される可能性がどこにもないわけです。
調査のために DayUtils#listWeekends で return している weekends も、テストコードの ★ の行の代入文の右辺も Objects#requireNonNull で囲いました。すると、プロダクションコードの null check は成功しているように見えるのに、例外はテストコード側に仕掛けた requireNonNull から送出されていたことが分かりました。^[1]

怪奇です。調査が行き詰まりかけました。

`static` を嫌った調査リファクタリングで見つけた原因

このように行き詰まったとき、「とりあえず」の勢いで自分なりのリファクタリングを試みます。そのひとつ DateUtils に定義されたメソッドの非 static 化がありました。実装を見たところ static である必然性もなかったので、 new DateUtils().listWeekends(...) と呼び出すようにリファクタリングしようと、IDE の機能を触っていたときでした。この DateUtils の呼び出し元に以下のようなテストコードを見つけました。

class CalcTermEndUseCaseTest {

  private MockedStatic<DateUtils> dateUtilsMock = null
  
  // ...
  
  @BeforeEach
  void setupMocks() {
    MockedStatic<DateUtils> mocked = Mockito.mockStatic(DateUtils.class)
    mocked.when(() -> 
            DateUtils::anotherMethod(any()))
	.thenAnswer(...);
  }
  
  @Test
  public void test...() {
    // ...略
}

かつて static メソッドのモッキング^[2]は PowerMock を利用する必要がありましたが、Mockito の 3.4.0 にて static メソッドへのモッキングを行う機能が追加されていたのです^[3]^[4]。この static メソッドへのモックがクローズがされていなかったのが、この問題の直接の原因でした。テスト終了時までに MockedStatic#close を呼び出すようにして、無事テストが成功するようになりました。

調査時に遭遇した不思議な挙動も説明が付きます。

ローカルで IDE から単一のテストクラスを実行したときには、static メソッドのモックを設定するテストを実行していないので成功した。
Jenkins で実施したときに失敗したのは、テスト順がたまたまモック設定されたあとに当該テストが実行されたため。
- 本来は成功したりしなかったりするテスト^[5]だった可能性が高い。
ローカルでビルドツールを通じて全件実施した際にテストが成功したのも実行順依存だった可能性が高い。
プロダクションコードの null check は成功していたのではなく、モックされていて動作していなかった。

学び

今回遭遇した問題からは、細かいですが様々な教訓や知見が得られるのではないかと思います。

Mockito の static メソッドの挙動

Mockito のドキュメントにありますが、この static メソッドのモックされた振る舞いは、Mockito が内部で static に保持する MockMaker にスレッドローカルで保持しています。それにより、マルチスレッドでテストを並列実行しても互いに干渉しないように出来ているようです。

一方で、一度登録されたモックの振る舞いは MockedStatic の close メソッドを呼び出すことで明示的に取り除かないかぎり残り続けます。そのため、公式ドキュメントでもモックの解放忘れを防ぐために try-with-resources を用いた利用を推奨しています。^[6]

xUnit のテスト実行順は不定

JUnit を始めとする xUnit 系のテスティングフレームワークでは、テストは原則としてランダムに実行されます。こうすることで、実行順に依存して成否が決まるテストが実装されづらくなります。「1つのテストの失敗が1つの問題と対応する」ようにすることで、テストの失敗から問題点の特定を効率よくできるようになります。^[7]

今回のケースでは、それぞれのテストは独立性を持つようにする意図をもって記述されていました。しかし、static メソッドのモックと close 漏れによってテストの独立性が下がってしまったということになります。

static メソッドをモックしたいのならインスタンスメソッド化を検討する

今回、テストの独立性が下がってしまったのはテストの問題というよりプロダクションコードの設計の問題のほうが大きいかもしれません。static なメソッドの必要性や妥当性は様々な文脈に依るので一概には言えませんが、static メソッドを何らかの理由でモックしなければならないのなら、それは static メソッドではなくてインスタンスのコンポジションを選んだほうが好ましいという兆候かもしれません。

例えば、件のモックを行っていたテストコードのテスト対象は以下のように書かれていたとしましょう。

class CalculateTermUseCase {
  void run() {
    // ...略
    if(DateUtils.listWeekends(...).contains(...)) {
      // ...
    }
    // ...略
  }
}

日付が週末であるかどうかの判定ロジックをモックしたいということは、その判定ロジックは可変である可能性があるということです。^[8] 例えば、 DateUtils#listWeekends をインスタンスメソッドとした上で、以下のような修正をすれば、トリッキーなモックをせずにテストをすることができそうです。

class CalculateTermUseCase {
  private DateUtils dateUtils;
  
  CalculateTermUseCase(DateUtils dateUtils) {
    this.dateUtils = dateUtils;
  }
  
  void run() {
    // ...略
    if(this.dateUtils.listWeekends(...).contains(...)) {
      // ...
    }
    // ...略
  }
}

当然、そのコードベースの設計や思想次第ではありますが、static なメソッドをモックする必要に出会ったら、それは設計を見直す合図だと捉えても良いかもしれません。

それでは、引き続き良いテスティングライフを！

脚注

このコードベースは Java 11 を利用していました。Java 14 以降ならば JEP 358 Helpful NullPointerException でこの苦労はせずに済みますね。 ↩︎
厳密には mock ではなく stub ですが、本稿の趣旨を鑑みて重要ではないので区別していません。 ↩︎
実際、mockito の当該 issue の最初のコメントが「個人的にはこの機能を入れるべきかは悩ましい。これができるようにすることで、開発者が悪いプラクティスを (コードベースに) 持ち込ませてしまうことにならないだろうか。」と懸念を表明するものだったようです。 ↩︎
。コードベースが依存しているライブラリに PowerMock がいなかったので油断していました。個人的には Java を触るのが数年ぶりだったので、単純に知識のアップデート遅れです。 ↩︎
このようなテストを「フレーキーテスト」といいます。 ↩︎
IntelliJ IDEA 上では AutoClosable が実装されているクラスの close が呼ばれていない旨の警告が出るので、Qodana なら検出できたかもしれません。未確認。 ↩︎
書籍『テスト駆動開発』にて「独立したテスト」というパターン名で紹介されています。 ↩︎
イスラム圏には金曜と土曜を週末とする国があります。 ↩︎