どうでもいいのかもしれないけど気になっちゃう【単体テスト編】
たいしたことではないですし、テスト結果に影響はありません。
趣味の問題かもしれませんし、私の感性がずれているのかもしれません。
なのでさしてどうということもないのですが、ふと皆さんどう思っているのか気になり、記事にしてみました。
※モックライブラリは Moq を例に記述します。
モックの Setup/Verify ラムダ引数名を m に統一している
ネット上のサンプルでも見かけます。
mock の m だと思うんですが、入力引数の型は Mock<T> でなく、中身の T なんですよね。
mで統一
customerServiceMock.Setup(m => m.IsMonster(4989)).Returns(true);中身に沿った命名
customerServiceMock.Setup(s => s.IsMonster(4989)).Returns(true);m 派の意見も聞いてみたいです。
x 派もいますね。意味があるのか、命名に困ったのか。こちらも興味があります。
（いきなり）余談
上の例では bool IsMonster(int customerId) にリテラルを渡していますが、リテラルか定数かという考察も面白いですね。
リテラル
// Arrange
:
customerServiceMock.Setup(s => s.IsMonster(4989)).Returns(true);
// Act
service.DoSomething(4989);定数
// Arrange
const int CustomerId = 4989;
:
customerServiceMock.Setup(s => s.IsMonster(CustomerId)).Returns(true);
// Act
service.DoSomething(CustomerId);どちらがわかりやすいでしょうか。
リテラル方式の場合、文字列型にはインターンプール（別記事参照）があるとして、日付型などは都度 new することになる。インスタンス重複生成の無駄は許容できるか？
たとえば Guid 文字列の同一性を判断するのは人の目には酷ではないか？
定数化（組み込み型以外の値型では変数化）方式の場合、引数が多くても全部定数化するのか？
bool 型も定数にするのか？
増分されたことを検証する場合は定数を使った計算式で表現するのか？
定数にすることで意味を明示できる
リテラルを基本としつつ、必要に応じて定数／変数化も検討する、といった柔軟な運用でいいのかもしれません。
戻り値を収める変数名を result に統一している
単体テストにはプログラマの意図した使用法が記述されており、メソッドの仕様書的な側面もあります。
プロダクションコードの実装時に参考にすることもあるでしょう。
であれば、使用例となるような中身に沿った命名がいいな、と私は思います。
戻り値の中身や Assert で何を表明しているのか、ぱっと見でわかりやすくなりますしね。
resultで統一
var result = service.GetCustomers();
Assert.AreEqual(2, result.Count);
:中身に沿った命名
var customers = service.GetCustomers();
Assert.AreEqual(2, customers.Count);
:テスト対象オブジェクトの変数名を target に統一している
上と同じ理由で、プロダクションコードで実際使うような命名にするのが好みです。
ただし、クラスリネームの影響を受けないよう、customerService ではなく service のような抽象的な名前にします（ここは異論もあるでしょう）。
でも、テストクラス内で自明なので、あえて意味のない名前 target でもいいのかなと、これはプロジェクト規約次第でしょうか。
targetで統一
var customers = target.GetCustomers();中身に沿った命名
var customers = service.GetCustomers();Mock インスタンス名に "mock" プレフィックスをつける
以前、開発チームで議論になったことがありました。
プレフィックス派
var mockCustomerService = new Mock<CustomerService>();・縦に並んだときなど、ほかと識別しやすい。
・大文字／小文字を区別しない検索・置換で検出しやすい。（プロダクションコードではないということも考慮して実利をとる）
サフィックス派
var customerServiceMock = new Mock<CustomerService>();・CustomerService の Mock だから。
独自にモッククラスを定義する場合の名前は MockCustomerService でしょうが、これは同一のインターフェイスを実装、またはクラスを継承していますから、同列には考えられませんね。
ちなみに私はサフィックス派ですが、統一することが最優先等という立場なので、プロジェクトでプレフィックスと決まれば抵抗はやめて従います。
参考までに、GitHub の検索にかけたところ、2020年7月12日現在で以下のような結果でした。
・mockCustomerService：1,290 code results
・customerServiceMock：1,439 code results
拮抗していますね。
余談
これに近いことで、ディレクトリ内で同じ分類のものが固まるよう、プレフィックス風に命名されたクラス群を目にすることがあります。
狙いや効用を否定するつもりはありませんが、本来それらは名前空間の構成で考えるべきことではないでしょうか。
やりすぎると言葉として破綻してしまいますし、それならと For や Of でつなぐ例もありますが、
・長くなる（識別に寄与しない単語に字数を使うのはもったいない）
・複数形にしづらい
・DbDriverForSqlServer の Mock は DbDriverForSqlServerMock となり、Mock がどこにかかるのかわからない
といった理由で扱いにくいと感じてしまいます。
テストダブル（Mock, Stub, Fake）の命名が実体と異なる
『単体テストを記述するためのベスト プラクティス - .NET Core | Microsoft Docs』より、やや不安定な日本語ですが、引用させていただきます。
フェイク - フェイクは、スタブまたはモック オブジェクトのいずれかを示すのに使用できる汎用的な用語です。 スタブとモックのどちらであるかは、使用されているコンテキストによって決まります。 つまり、フェイクはスタブとモックのどちらにもなりえます。
モック - モック オブジェクトは、単体テストに合格したか失敗したかを判断する、システム内のフェイク オブジェクトです。 モックは、アサートされるまでフェイクとして開始します。
スタブ - スタブは、システム内の既存の依存関係 (コラボレーター) の制御可能な置換です。 スタブを使用すると、依存関係を直接処理することなく、コードをテストできます。 既定では、フェイクはスタブとして開始します。
簡単に言うと「アサートするのがモック」ということですね。
"Mock" の誤用を "Fake" に訂正するコード例も載っています。
フェイクの定義については異論があるかもしれませんね。
一般的な定義と大きく乖離していない限り、プロジェクト内で統一がとれていればよいでしょう。
Assert の後に Verify がある
Verify は通常、Assert の前提です。
Verify に失敗するなら Assert に成功しても動作の保証にはなりません。
前提をクリアしていない状態で結果の表明を行うことになってしまいます。
と言いつつ、ある程度の規模になれば実装効率と可読性を優先し、共通の仕組みとして後でまとめて VerifyAll してしまうことが多いです（以下記事参照）。
[.NET] 単体テストがさくっと書ける！モック化の枠組み（Moq + Unity）
[.NET] 単体テストがさくっと書ける！モック化の枠組み（Moq + Autofac）
Stack Overflow では Verify は Assert の後に来るべきという見解 が支持を得ていますので、一般的にはこちらの考えが優勢なのかもしれません。
争うつもりはありませんが、別の見解にも興味はあります。
私が大事な観点を見落としている可能性もありますので。
Dispose する前に Verify/Assert している
using ステートメントや finally ブロック内にあれば別ですが、Verify や Assert に失敗すると例外が発生して、その後の Dispose が実行されません。
単体テストではパフォーマンスへの意識が薄くなりがちですが、通しで実行するとリソース解放漏れが累積して、メモリ不足やハンドル枯渇を引き起こすこともあります。
このケースに限って言うと、そんなにたくさん失敗するとしたらそれ自体が問題なわけですが。
リスト要素の複数プロパティ値を検証するのに毎回添え字やインデクサでアクセスしている
うまく言えてませんので、コードを見た方が早いでしょう。
添え字でアクセス
Assert.AreEqual("河内 土成", customers[0].JapaneseName);
Assert.AreEqual("Donaru Kawanai", customers[0].EnglishName);
Assert.IsTrue(customers[0].IsMonster);
Assert.AreEqual("沢山 加宇代", customers[1].JapaneseName);
:この方が個別のステートメントで見たときに判別しやすい、という意図はわからなくもありません。
が、毎回要素を探させる無駄は、要素数や参照プロパティ数（の累計）が増えると無視できません。
コピペで添え字がずれたのに気づかず、本当は不正なのにテストが成功していた、というようなバグも混入しやすいんですよね。
私はこのように書きます。
変数導入
Customer customer;
customer = customers[0];
Assert.AreEqual("河内 土成", customer.JapaneseName);
Assert.AreEqual("Donaru Kawanai", customer.EnglishName);
Assert.IsTrue(customer.IsMonster);
customer = customers[1];
Assert.AreEqual("沢山 加宇代", customer.JapaneseName);
:長い単体テスト実行はスピーディな開発の流れを阻害します。
単体テストが短く終わるのは継続的インテグレーションにおいて大きなメリットです。
テストのためだけに public にする
ビルド時間が長くなります。
リファクタリング機能のついたIDEであれば、リネーム時の反映など、コーディング中の待ちも長くなります。
public は「どうぞ外からお使いください」というメッセージです。
影響箇所（不正に操作される可能性／障害原因の調査個所）が爆発的に増えます。
一度開いたら閉じるのはたいへんです。
開くにしても internal にしましょう。
C# であれば、テスト対象プロジェクトの AssemblyInfo.cs に
[assembly: InternalsVisibleTo("TestProjectName")]のような宣言を記述することで、テストメソッドから internal メンバにアクセスできるようになります。
さらに
[assembly: InternalsVisibleTo("DynamicProxyGenAssembly2")]とすれば、Moq の Setup でも使えます。
テストコードの実装が簡易になるというメリットはありますので、プロジェクトの方針次第で総合的に検討すべき事項かと思います。
注意深く読まないと何のテストかわからない
メソッド名に明示するか。
メソッド名が連番ルールならヘッダ部か条件設定箇所にコメントをつけるか。
そうしないと、書いたとき、書いた人はわかるかもしれませんが、次に手を入れるとき苦労することになります。
改修の影響するテストメソッドを特定するのがたいへんだったり、失敗したテストの原因を追うのに時間を奪われたり、すでにあるケースのテストを重複作成してしまったり。
単体テストでは、プロダクションコード以上に実装効率の優先度、というかビジネス観点からの省コスト要請が高いと思います。
それ自体は製品価値を生み出さないわけですから。
そこでつい書きっ放しになりがちですが、そうすると改修時の実装効率を下げることになってしまいます。
単体テストを通すのは次に手を入れるときのためでもあります。
頭がホットなうちはたいした手間ではありません。
ケースは素早く判別できるよう、明示しておきたいところです。
ということを自分に言い聞かせつつ、ひとまず、もやもやを形にできたので作業に戻りたいと思います。