25
13

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?

More than 5 years have passed since last update.

PHPUnitで「例外が投げられないことを期待する」というテストケースをどのように書くか

Posted at

こんな「困った」時の話

「異常時に例外を投げる、正常に完了したらvoidとなる」というようなメソッドがあるとします。
こんな時に、PHPUnitでどのように単体テストを書けばいいでしょうか?

PHPUnitでは、1つもassertionがないテストを「Risky」としてマークします。これは賢いのですが、しかし「返り値をとれないメソッド」=assertionを置けないテストケースについてRiskyになってしまうのが困る・・・と悩んでいました。

結論: @doesNotPerformAssertions を使えそう

テストケースのアノテーションに @doesNotPerformAssertions を入れると、「これはリスキーじゃないぞ」という事を示すことができそうです。

以下、「こうやって対処してきた」「これからは@doesNotPerformAssertionsを使いたいかも」という話をします。

いろいろな書き方

例えば、「(内容をバリデーションして)引き渡されたデータに誤りがあれば例外を投げる、問題がなければ何もしない」というメソッド denyIfInvalidData() があったとします。

微妙い例①

例えば「AssertTrue(true)をして無理矢理やるなどしていました。

public function testDenyIfInvalidDataValidData()
{
    $data = ['requiredKey' => true];
    $this->subject->denyIfInvalidData($validData);

    // ここまで到達したらOK
    $this->assertTrue(true);
}

気持ち悪いな〜〜という思いがあります。

微妙い例②

次に思いつくのが、「catchブロックに到達しなかったらOKでは」という発想です。

public function testDenyIfInvalidDataValidData()
{
    $data = ['requiredKey' => true];
    try {
        $this->subject->denyIfInvalidData($validData);
    } catch (\InvalidArgumentException $e) {
        $this->fail('問題ないデータなのに拒絶されている');
    } finally {
        $this->assertFalse(!isset($e), '例外が発生している');
    }
}

これで、「例外が生じていなければOK」という流れは作ることができました。
しかしどうにも冗長だし、 Test as documentation の観点から「簡潔さ」が実現できていないように思います。(「例外が生じていないこと」に、なぜアサーションを2つも書かなければ行けないのでしょう?)

微妙い例③ -> 悪い例

「返り値がない」ので、 null 扱いができますね・・・PHPerのみなさん・・・

public function testDenyIfInvalidDataValidData()
{
    $data = ['requiredKey' => true];
    $this->assertNull(
        $this->subject->denyIfInvalidData($validData);
        '問題ないデータなのに拒絶されている'
    );
}

「動き上の問題はない」と思われるのですが、これはPHPStan・Phan・PhpStorm等のコードを解析ツールに掛けると警告されるやり方です。そのくらい「悪い」と言えるでしょう。

PHPStan: Result of function method (void) is used.
Phan: PhanTypeVoidAssignment
PhpStorm: 'void' method METHOD result used.

これでいいのでないか、という例

@doesNotPerformAssertions を利用すると、こうなります

/**
 * @doesNotPerformAssertions
 */
public function testDenyIfInvalidDataValidData()
{
    $data = ['requiredKey' => true];
    $this->subject->denyIfInvalidData($validData);
}

記述量的にももちろん冗長さがないし、これなら「何を言いたいのか」というのがちゃんと際立つのではないでしょうか。
実行結果は OK (1 test, 0 assertions) となり、問題なくグリーンとなります。
もしこのアノテーションを付けていなかった場合、 OK, but incomplete, skipped, or risky tests!sky tests! となりグリーンとはなりません。

@doesNotPerformAssertions とは何なのか

それでは、コイツは一体何者なのでしょうか?
今回話題に上げている内容の正にそのままなのですが、「テストケースがRiskyとして扱われる事を回避する」ためのものです。
https://phpunit.readthedocs.io/ja/latest/annotations.html?highlight=doesNotPerformAssertions#doesnotperformassertions
ズバリですね。「リスキーとみなさない」という表現は何とも消極的な気はしますが、「異常が無いことを保証する」という点で単体テストの本義は十分に果たせると思いますし、またSUTの実行も勿論走るのでカバレッジにも寄与することと思います。

関連するIssue

長らくPHPUnitに触れてきたのに今まで何で気づいてなかったん・・?という感はあるのですが、私がこの「ヒント」に辿り着けたのは以下の議論を発見できたからでした。
https://github.com/sebastianbergmann/phpunit-documentation/issues/171

おわり

そもそも「単体テストをする」という意味で言えば「返り値を見る」という方がシンプルそうだし、そのように予め設計する〜というのも1つの考え方なのかもしれません。
とはいえreturn voidが「絶対に利用すべきでない」というものでもないはずですし、プロダクトコードにせよテストコードにせよ「筆者の気持ちがわかる」ような記述を心懸けながらパワフルで意義深いコードを書いていければ良いかな〜と思いました。

25
13
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
25
13

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?