LoginSignup
18
6
@moromi25in株式会社Works Human Intelligence

Mockito.mockStaticを使ったのにcloseしないのは時限爆弾を仕込むようなものだよ、というお話

Last updated at Posted at 2023-09-07

まず言いたいこと

Mockito.mockStaticを使ってstaticメソッドをMockした場合、そのクラスでのテスト終了時に以下のいずれかによってMockしたクラスを解放しましょう。

  • try-with-resourcesで囲む
  • close処理を書く

この記事を書いた経緯などをすっ飛ばしてそれぞれの書き方の例をご覧になりたい方はMockしたstaticメソッドをcloseする2つの方法のセクションをどうぞ。

はじめに

staticメソッドをMockしたのに解放漏れを起こして、他チームにご迷惑をおかけしてしまいました・・・
自戒と理解を深めるために記載します。

JUnit界ではConnectionのclose漏れレベルの基本的な話だと思います。
お恥ずかしい限りですが、お付き合いください。
また、誤っている箇所があればご指摘ください。

やらかしの経緯

事の発端：mockStaticをしたら実行時に怒られた

1つのテストクラスの中に複数のテストがあり、同じstaticメソッドを利用したいです。
Mock化の記載重複を避けるため、@BeforeEachを付与したメソッド内でで毎回Mockしようとしました。

@TestInstance(Lifecycle.PER_CLASS)
class OptionsConverterTest {
    private MockedStatic<OptionsProvider> mockedOptionsProvider;
    private OptionsConverter converter;

    @BeforeAll
    void setupOnce() {
        converter = new OptionsConverter();
    }

    @BeforeEach
    void setupEach() {
        mockedOptionsProvider = mockStatic(OptionsProvider.class);
        mockedOptionsProvider.when(() -> {
            OptionsProvider.getOption("optionKey1");
        }).thenReturn(1);
        mockedOptionsProvider.when(() -> {
            OptionsProvider.getOption("optionKey2");
        }).thenReturn(2);
    }

    @Test
    void test_of_keyOne() {
        assertThat(converter.canUseSomeService("optionKey1"), is(true));
    }

    @Test
    void test_of_keyTwo() {
        assertThat(cconverter.canUseSomeService("optionKey2"), is(false));
    }
}

いざ実行してみると、2つ目のテストが落ちました。
以下のエラーで怒られていました。

org.mockito.exceptions.base.MockitoException: 
For OptionsProvider, static mocking is already registered in the current thread

To create a new mock, the existing static mock registration must be deregistered
	at OptionsConverterTest.setupEach(OptionsConverterTest.java:18)
	...

@BeforeEachで毎回Mockするのがいけないのかなー
staticメソッドだから何回もMockすると怒られるんだなきっと（雑）。

どうせMockしたい内容は同じだから、@BeforeAllで1度だけmockStaticしてみよう！

class OptionsConverterTest {
    private MockedStatic<OptionsProvider> mockedOptionsProvider;
    private OptionsConverter converter;

    @BeforeAll
    void setupOnce() {
        converter = new OptionsConverter();
        mockedOptionsProvider = mockStatic(OptionsProvider.class);
        mockedOptionsProvider.when(() -> {
            OptionsProvider.getOption("optionKey1");
        }).thenReturn(1);
        mockedOptionsProvider.when(() -> {
            OptionsProvider.getOption("optionKey2");
        }).thenReturn(2);
    }

    @Test
    void test_of_keyOne() {
    ...

結果
image.png

@BeforeAllの中で1度だけmockStaticするようにしたら怒られなくなった！

やっぱり1つのテストクラスの中で複数回mockStaticを利用したらいけなかったんだな。
はい実装完了！マージ！めでたしめでたし。1

・・・本来ならここでもう少し思考を巡らせているべきでした。

翌日、全件テスト実行ジョブが日次実行され、全く手を入れていない箇所で突然テストがfailする事態が発生

特に製品コードにもテストコードにも手を入れていないテストが突然failする事態が複数個所で発生し、failしたテストを持つチームの方は「？？？」となりつつも対応をしてくださっていました。2
しかし、調査を続ける中でSlackに流れてきたエラークラスの名前に、昨日私がマージしたクラスの名前が・・・

「まだよくわからないけどタイミング的にどう考えても私何かやらかしたっぽい」ということだけは認識し、これまで問題が起きていなかったテストと今回私がマージしたテストを見比べてみました。
これまで問題のなかったテストは、以下のように書かれていました。

BrilliantClassTest
void test_howBrilliant() {
    try (MockedStatic<OptionsProvider> mockedOptionsProvider = mockStatic(OptionsProvider.class)) {
        mockedOptionsProvider.when(() -> {
            OptionsProvider.getOption("optionKey1");
        }).thenReturn(1);
        assertThat(brilliant.execute("optionKey1"), is(1));
    }    
}

・・・ん？
try-with-resourcesでMockedStaticクラスが括られている・・・！

調べた

簡易的にMockito mockedStatic closeなどをネットで調べてみると、staticメソッドをMockしたら、必ずcloseしないといけないとのこと。

ポイントはちゃんとclose()呼ぶことです
try-with-resources使えば勝手に呼ばれますが、test1の書き方でclose()を書かない場合、クラス単位でテスト実行すると以下のエラーが出るので要注意です

To create a new mock, the existing static mock registration must be deregistered Bash

なるほど理解しました・・・
一応一次ソースも確認します。

MockitoのJavadocにもちゃんと書いてあった

If this object is never closed, the static mock will remain active on the initiating thread. It is therefore recommended to create this object within a try-with-resources statement unless when managed explicitly, for example by using a JUnit rule or extension.

大体の意味としては

製品のテストコード全体でMockedStaticなクラスの扱いの統制がとれているならともかくとして、
そうでない場合は、ちゃんとcloseしないとスレッド内にMockされたstaticなオブジェクトが残り続けてしまうよ。

そしてその結果、再度同じMockStaticクラスを作ろうとすることになって怒られるんですね。
わかりました。ごめんなさい。

テストクラスをネストして複数クラスに分割してみたら同じ状況を作り出せた

ローカルでは@BeforeAllを使うことでPassできてしまっていたテストコードを、以下のように2つのクラスに分割し、それぞれでmockStaticをしてみたところ、同じようにstatic mocking is already registered in the current threadのエラーを起こすことができました。

class OptionsConverterTest {
    @Nested
    @TestInstance(Lifecycle.PER_CLASS)
    class canUseSomeServiceTest1 {
        private MockedStatic<OptionsProvider> mockedOptionsProvider;
        private OptionsConverter converter;

        @BeforeAll
        void setupOnce() {
            converter = new OptionsConverter();
            mockedOptionsProvider = mockStatic(OptionsProvider.class);
            mockedOptionsProvider.when(() -> {
                OptionsProvider.getOption("optionKey1");
            }).thenReturn(1);
            mockedOptionsProvider.when(() -> {
                OptionsProvider.getOption("optionKey2");
            }).thenReturn(2);
        }

        @Test
        void test_of_keyOne() {
            assertThat(converter.canUseSomeService("optionKey1"), is(true));
        }
    }

    @TestInstance(Lifecycle.PER_CLASS)
    class canUseSomeServiceTest2 {
        private MockedStatic<OptionsProvider> mockedOptionsProvider;

        @BeforeAll
        void setupOnce() {
            converter = new OptionsConverter();
            mockedOptionsProvider = mockStatic(OptionsProvider.class);
            mockedOptionsProvider.when(() -> {
                OptionsProvider.getOption("optionKey1");
            }).thenReturn(1);
            mockedOptionsProvider.when(() -> {
                OptionsProvider.getOption("optionKey2");
            }).thenReturn(2);
        }

        @Test
        void test_of_keyTwo() {
            assertThat(converter.canUseSomeService("optionKey2"), is(false));
        }
    }
}

ここまで調べたことと状況の咀嚼

ローカルで1クラスに対してのみテストを実行する分には@BeforeAllでもなんとかなってしまいました。
Mockするのがスレッド内で1回のみであれば、特にcloseをせずとも、テストクラス1個の実行が終了すればそのスレッドも解放され、MockStaticなクラスも解放されるためです。

しかし、1つのクラスの中で複数のネストしたクラスを持たせたり、複数のテストクラスをまとめて実行する場合は別です。3
特に今回mockStaticしたクラスは製品共通的に利用されているクラスだったこともあり、他チームがすでに同じクラスをmockStaticしてテストを書いていました。

テストの実行順序的に先にmockStaticし、なおかつcloseし漏れているクラスの実行時は、元凶にもかかわらずエラーは発生しません。
ちゃんとclose処理を書いている、でも後から実行されるテストクラス側でstatic mocking is already registered in the current threadのエラーが発生するという、時限爆弾を仕込んでしまったことになります。

Mockしたstaticメソッドをcloseする2つの方法

1つのテストメソッド内でMockedStaticクラスの利用が完結する場合

try-with-resorcesで書けばOKです。
closeの書き漏れもないので安心です。
MockitoのJavadocにもtry-with-resourcesを使ってね、と書いてあります。基本はこちらを使えばよさそうです。

class OptionsConverterTest {
    @Test
    void test_of_keyOne() {
        OptionsConverter converter = new OptionsConverter();
        try (MockedStatic<OptionsProvider> mockedOptionsProvider = mockStatic(OptionsProvider.class)) {
            mockedOptionsProvider.when(() -> {
                OptionsProvider.getOption("optionKey1");
            }).thenReturn(1);
            mockedOptionsProvider.when(() -> {
                OptionsProvider.getOption("optionKey2");
            }).thenReturn(2);
            assertThat(converter.canUseSomeService("optionKey1"), is(true));
            assertThat(converter.canUseSomeService("optionKey2"), is(false));
        }
    }
}

1つのテストクラス内で同じMockedStaticなクラスを使いまわしたい場合

@BeforeAllでmockStaticし、@AfterAllでcloseすればOKです。
@BeforeEachと@AfterEachの組み合わせでもOKです。
openしたものはちゃんとcloseする、が守れればOKですが、万が一ずれると時限爆弾化するので細心の注意を払いましょう。

最初にstatic mocking is already registered in the current threadで怒られたテストクラスに対して、@AfterEachでmockStaticしたクラスのclose処理を書くことでエラーが起こらなくなりました。

@TestInstance(Lifecycle.PER_CLASS)
class OptionsConverterTest {
    private MockedStatic<OptionsProvider> mockedOptionsProvider;
    private OptionsConverter converter;

    @BeforeAll
    void setupOnce() {
        converter = new OptionsConverter();
    }

    @BeforeEach
    void setupEach() {
        mockedOptionsProvider = mockStatic(OptionsProvider.class);
        mockedOptionsProvider.when(() -> {
            OptionsProvider.getOption("optionKey1");
        }).thenReturn(1);
        mockedOptionsProvider.when(() -> {
            OptionsProvider.getOption("optionKey2");
        }).thenReturn(2);
    }

    @Test
    void test_of_keyOne() {
        assertThat(converter.canUseSomeService("optionKey1"), is(true));
    }

    @Test
    void test_of_keyTwo() {
        assertThat(converter.canUseSomeService("optionKey2"), is(false));
    }

    @AfterEach
    void teardownEach() {
        mockedOptionsProvider.close();
    }

}

AutoClosableがSuperclassに含まれているかで解放漏れを回避

Mockito.mockStaticの返り値だったMockStaticの継承元を見てみると、AutoCloseableが含まれているのがわかります。

image.png

AutoClosableについての説明はAutoclosableのJavadocが大変わかりやすいのでここでは割愛しますが、今回のような解放漏れ対策としては、このインターフェースが継承されていたら基本的には解放が必要だな、と判断できます。

おわりに

ちゃんとドキュメント読めよ、に終始しますが、おかげでMockito.mockStaticメソッドやMockedStaticインターフェースと少し仲良くなれた気がします。
とにかく、もう二度とMockedStaicクラスのclose忘れを起こさないし、見逃さないようにします。

参考URL

  1. だいぶ省略して記載していますが、セルフマージしているわけではありません。ペアプロでナビゲーターしつつレビューしつつ進めていたため、PRが来た段階ではさっと見てマージをしました。 ↩

  2. 実際に私がマージしたパッケージもクラスもaから始まるため、原因である私はエラーを初期的に検知できず、しばらく他チームにご迷惑をおかけした後に気づく事態になりました・・・ ↩

  3. JUnit Jupiterはデフォルトではシングルスレッドで順番にテストが実行されます。設定することでマルチスレッドで並列実行が可能ですが、私たちの製品ではこの指定はされておらず、シングルスレッドで実行される状態でした。（参考） ↩

18
6
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
18
6