テストでexpectExceptionを使用している箇所のチェック

[seto1]

それ以降のテストが実行されないため対応が必要

https://www.engilaboo.com/laravel-php-exception-test/

また、このような例外のテストを書く際のポイントは、テストしたいメソッドを呼び出すよりも先にexpectExceptionを書くことです。
なぜなら、例外が発生するとそこで処理が終了するため、テストがそれ以上実行されないからです。

https://teratail.com/questions/362232

$this->expectException() を使わない
テストメソッドを try catch してcatch内でエラーアサートする

[ryuring]

@seto1 例外が発生するとそこで処理が終了というのはありますが、その前提で、expectException は使ってもいいと思いますけどね。テストを分ければよいので。
結構使ってるはずです。

[seto1]

@ryuring

例外が発生するとそこで処理が終了というのはありますが、その前提で、expectException は使ってもいいと思いますけどね。テストを分ければよいので。

その前提での使用は問題ないと思います。
ただ、現状 例外が発生する処理以降にもテストが書かれているケースがいくつかあります。

テストを分けることで現在作成済みのテストの対応はできます。
しかし、今後例外を発生させたいテストを作成する際に、前提を知らずに既存のコードを参考にしてしまう可能性が出てきます。

そうであれば、問題が起きなさそうな書き方をしておいたほうが安全な気がします。
コード量は増えてしまいますけどね..

$this->expectException('Cake\Datasource\Exception\RecordNotFoundException');
$this->ContentsService->getTrash(16);

try {
    $this->ContentsService->getTrash(16);
    $this->fail();
} catch (\Exception $e) {
    $this->assertInstanceOf(\Cake\Datasource\Exception\RecordNotFoundException::class, $e);
}

この場合も、既存のコードを参考にせずexpectExceptionを使うケースも考えられるので、問題解決とはいきませんが..

[ryuring]

@seto1

この場合も、既存のコードを参考にせずexpectExceptionを使うケースも考えられるので、問題解決とはいきませんが..

ですよね。
基本的にプルリクベースなので、レビュワー側で認識をあわせていれば問題ないと思いますがいかがでしょう？

[seto1]

内容を再確認

問題

それ以降のテストが実行されないため対応が必要

例

#648
https://github.com/baserproject/ucmitz/pull/648/files#diff-fdfa30dcde75d58ad4d44f372e41ca69c5038ddceb556f8601ce618d96740a56

やること

expectException を呼び出している箇所をチェックして、それ以降にテストが書かれていないかチェック

[seto1]

plugins配下でexpectExceptionで検索してヒットしたのは70箇所

要チェックの箇所は以下

https://github.com/baserproject/basercms/blob/dev-5/plugins/bc-favorite/tests/TestCase/Service/FavoritesServiceTest.php#L77

    public function testGet(): void
    {
        $this->expectException("Cake\Datasource\Exception\RecordNotFoundException");
        $result = $this->FavoritesService->get(0);
        $this->assertEmpty($result);

        $result = $this->FavoritesService->get(1);
        $this->assertEquals("固定ページ管理", $result->name);
    }

https://github.com/baserproject/basercms/blob/dev-5/plugins/baser-core/tests/TestCase/Service/ContentsServiceTest.php#L273

    public function testHardDelete(): void
    {
        // treeBehavior falseの場合
        $this->assertTrue($this->ContentsService->hardDelete(15));
        $this->expectException('Cake\Datasource\Exception\RecordNotFoundException');
        $this->ContentsService->getTrash(15);
        // treeBehavior trueの場合
        $this->assertTrue($this->ContentsService->hardDelete(16, true));
        $this->expectException('Cake\Datasource\Exception\RecordNotFoundException');
        $this->ContentsService->getTrash(16); // 親要素
        $this->expectException('Cake\Datasource\Exception\RecordNotFoundException');
        $this->ContentsService->getTrash(17); // 子要素
    }