【レビュー指摘】phpunitで書いたテストコードのレビューで受けた指摘をまとめてみる。

最近は、Laravelで作られたアプリのテストコードをphpunitで書いています。

今回はphpunitのテストコードについてレビューを受けた際の指摘内容について、二度と同じ指摘をされないようにするためにまとめておこうと思います。中にはphpunitの書き方への指摘というよりはプログラムの書き方自体への指摘や、プロジェクト固有の指摘もあるので、そこら辺は参考程度に見てもらえると嬉しいです。

指摘1 expectとactualの順序は正しく

第一引数がexpect、第二がactual。

$this->assertSame($expect, $actual);

指摘2 プロパティはprotectedに。型の説明も記載しましょう

テストクラスのプロパティにはprotectedを使う。
読みやすいように変数の型も記載する。

■Before

private $UserProfileModel;

■After

protected UserProfileModel $UserProfileModel; // protected 型 変数

指摘3 テストケース内でconfig設定しましょう

ログ取得のテストではConfigでログレベルの設定をしましょう。
setup(), tearDown()で設定を元に戻すことも忘れずに。

■Configによるログレベルの設定

/**
 * @test
 * 【正常系】メッセージがNullなら空文字がログ出力される
 */
public function messageIsNullSuccess()
{
    Config::set('logging.channels.stderr.level', 'debug'); // ←ここで設定

■setup(), tearDown()でログレベル設定の取得＆復元

/**
* setup()
* backupに現在のログレベルの設定を入れておく
*/
protected function setUp(): void
{
    parent::setUp();
    $this->backup = Config::get('logging.channels.stderr.level');
}

/**
* tearDown()
* 現在のログレベルの設定をbackupを使って設定する
*/
protected function tearDown(): void
{
    parent::tearDown();
    Config::set('logging.channels.stderr.level', $this->backup);
}

指摘4 namespaceのタイプミスに注意しましょう

namespaceが合っているかどうか確認する。
ディレクトリの先頭は大文字。

■Before

// ↓app, httpなど先頭が小文字になっている箇所がある
namespace Tests\Feature\app\http\controllers\Mypage\Profile;

■After

namespace Tests\Feature\App\Http\Controllers\Mypage\Profile;

指摘5 メソッドにはタイプヒンティングを書いておきましょう

テストケースのメソッドにもタイプヒンティングを書いておく。
基本はvoidだが、@dataProviderの関数はarrayになる。

■Before

public function registerMemberSuccess():

■After

public function registerMemberSuccess(): void

指摘6 assertEquals()ではなく、assertSame()を使いましょう

assertEquals()は「==」、assertSame()は「===」。
assertEquals()を使う場合は意図を記載しておく。

■Before

$this->assertEquals($expect, $actual);

■After

$this->assertSame($expect, $actual);

指摘7 メソッドを使いましょう

プロパティのテストには対象クラスで定義されているメソッドが使えることがある。
(例)👇alertFlagというプロパティを返すメソッドisAlertFlag()がある場合

// プロパティ alertFlag のテストケース
$this->assertSame(true, $this->target->isAlertFlag());

指摘8 元ソースのチェックもしましょう

テストを書いていて、テスト対象のソースに間違いがあればメンバーに伝える。

/**
 * ユーザモデルを返却する
 * @return UserModel
 */
public function getUserModel(): object // ここではオブジェクトとなっている
{
    return $this->settingsModel->getUserModel();
}

/**
 * ユーザモデルを返却する
 * @param void
 * @return UserModel
 */
public function getUserModel(): UserMode // ここではUserModeとなっている
{
    return $this->getUserModel();
}

指摘9 変数は展開しましょう

PHPではダブルクオートで囲うことで変数展開ができます。

■Before

$this->assertTrue(strpos($content[0], $logMessage.' | Exception:') !== false);

■After

$this->assertTrue(strpos($content[0], "{$logMessage} | Exception:") !== false);

指摘10 クオートは統一しましょう

クラス内はダブル or シングルクォートで統一すること。
基本、シングルに揃える。
※一括置換する際、変数展開が必要なもの（ダブル必須）でないか要確認。

■Before

$this->name = ["users"]; // ダブル
$this->age = ['20'];     // シングル

■After

$this->name = ['users'];
$this->age = ['20'];

指摘11 見辛い処理は分けましょう

見辛い処理は分けて書く。

■Before

$actualUser = (new UserList($user, $index))->all();

■After

$userList = new UserList($user, $index);
$actualUser = $userList->all();

指摘11' 変数は減らそう

可能な限り変数は減らす。
見辛さとトレードオフな所があると思うので場合による。

■Before

$userRequest = new UserRequest(['name' => 'taro']);
$response = $target->handle($userRequest);

$this->assertEquals(
    new UserResponse($result),
    $response
);

■After

$this->assertEquals(
    new UserResponse($result),
    $target->handle(new UserRequest(['name' => 'taro']));
);

指摘12 Config::('~')は冒頭にまとめよう

コード中ではなく、冒頭に変数でまとめる。

■Before

$a = Config::get('const.text.a');
処理
$b = Config::get('const.text.b');
処理
$c = Config::get('const.text.c');

■After

$a = Config::get('const.text.a');
$b = Config::get('const.text.b');
$c = Config::get('const.text.c');

処理
処理

指摘13 変数にした方が分かりやすい

データプロバイダーで渡す値が数値や文字列の羅列にならないように適当な変数名をつける。そうすることで、テストケースについての内容が読みやすくなる。
また、Configから取得している値についてはコメントがあると分かりやすい。

■Before

return [
    'age int success' => [
        'name' => taro,
        'age' => 10,
        'isLogin' => true,
        'regist_no' => 1,
        'valid' => 1,
    ]
];

■After

$login = true;
$unLogin = false;
$regist_no = 1;
$valid = '1';   //Config::get('const.valid')
$inValid = '2'; //Config::get('const.inValid')

return [
    'age int success' => [
        'name' => taro,
        'age' => 10,
        'isLogin' => $login,
        'regist_no' => $regist_no,
        'valid' => $valid,
    ],
    'age int error' => [
        'name' => taro,
        'age' => 10,
        'isLogin' => $unLogin,
        'regist_no' => $regist_no,
        'valid' => $valid,
    ]
];

まとめ：二度同じことは繰り返さない

レビューで一度受けた指摘は繰り返さないよう、しっかりと心に留めておきたいと思います。
また他の指摘を受けた際にはこちらに追記していこうと思います。

「僕はこんな指摘を受けた！」「ここはこうした方が良い。」などあれば、コメントいただけると助かります

それでは！