CClipboard の単体テストをさらに追加する #1843
Conversation
* EnumClipboardFormats, GlobalAlloc, GlobalLock の呼び出しを仮想メンバ関数でラップする。 * GetDataType を private で non-static なメンバ関数に変更する。
|
✅ Build sakura 1.0.4130 completed (commit 0f6ecc3ddb by @k-kagari) |
|
SonarCloud Quality Gate failed. |
API 関数の定義を踏襲したものですので対応しません。 カバレッジについては追加コード分が52%ということですね。マージ後のファイル全体の数値では80%になると思います。 |
|
✅ Build sakura 1.0.4131 completed (commit de9e391166 by @k-kagari) |
| } | ||
|
|
||
| LPVOID CClipboard::GlobalLock(HGLOBAL hMem) const { | ||
| return ::GlobalLock(hMem); |
There was a problem hiding this comment.
カバレッジが想定より低いのは、API呼出をモック化するために追加したこれらのメソッドがテストで実行されないためと考えられます。（テストで動くのはモックコードだから、ここは動かない）
テストではクリップボードのAPIを呼ばないようにして、クリップボードが勝手に書き換わる事態を回避しよう！で始めた対応なので、APIを呼ばないこと自体は気にしなくて良い認識です。
「CClipboardクラスのテストカバレッジ」を考えたときに、「テストで実行されないメソッドがある」の状態なので、何か対策したほうがいいような気はします。（というか、ちょっと自分でやりたいです:smiley:
There was a problem hiding this comment.
「CClipboardクラスのテストカバレッジ」を考えたときに、「テストで実行されないメソッドがある」の状態なので、何か対策したほうがいいような気はします。（というか、ちょっと自分でやりたいです😃
対策としては別のクラスに実装してコンストラクタで注入するといったところでしょうか？ そういう手法の方がモックの作り方としては一般的らしく、設計を再考してもいいかもしれませんね。何かこの件で案があればぜひお知らせください。
There was a problem hiding this comment.
対策としては別のクラスに実装してコンストラクタで注入するといったところでしょうか？
テスト導入のときに書いた気がするんですけど、
APIラッパー部分をCClipbordApiとかの別クラスに分割してあげれば、
CClipboardクラスのカバレッジは限りなく100%にできます。
実装コードの検証をしたいのは独自に組んだロジックだけなので、とりあえずソコが目標になるはず。
コンストラクタで注入（＝依存関係の逆転）は、やれたらカッコイイですね。
サクラエディタのWinMainはDIを実装できるほど簡潔でないように思います。
問題点「WinMainが簡潔でない」の対策は大変なので、当面は放置ですかね...。
|
レビューありがとうございます。マージします。 |
PR の目的
CClipboard のうち、単体テストが可能な部分を網羅したテスト群をリポジトリに追加します。
カテゴリ
PR のメリット
仕様・動作説明
static メンバ関数である CClipboard::GetDataType を non-static なメンバ関数に変更する差分を含みます。「static インターフェース」として宣言されていた関数ですが、CClipboard 外での使用例がなく、仕様を変更しても問題にならないものと判断しました。
関連 issue, PR
#1707, #1800