Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CClipboard の単体テストをさらに追加する #1843

Merged
merged 4 commits into from
May 8, 2022
Merged

CClipboard の単体テストをさらに追加する #1843

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e669c88
CClipboard 単体テストの準備。
kengoide May 8, 2022
011ff3b
CClipboard の単体テストを拡充する。
kengoide May 8, 2022
eed2fec
Windows API の呼び出しを伴うテストを削除する。
kengoide May 8, 2022
98652eb
CClipboard::GetDataType を const 関数にする。
kengoide May 8, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 27 additions & 19 deletions sakura_core/_os/CClipboard.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ bool CClipboard::SetText(
if( bColumnSelect ){
UINT uFormat = ::RegisterClipboardFormat( L"MSDEVColumnSelect" );
if( 0 != uFormat ){
hgClipMSDEVColumn = ::GlobalAlloc(
hgClipMSDEVColumn = GlobalAlloc(
GMEM_MOVEABLE | GMEM_DDESHARE,
1
);
Expand All @@ -170,7 +170,7 @@ bool CClipboard::SetText(
if( bLineSelect ){
UINT uFormat = ::RegisterClipboardFormat( L"MSDEVLineSelect" );
if( 0 != uFormat ){
hgClipMSDEVLine = ::GlobalAlloc(
hgClipMSDEVLine = GlobalAlloc(
GMEM_MOVEABLE | GMEM_DDESHARE,
1
);
Expand All @@ -186,7 +186,7 @@ bool CClipboard::SetText(
if( bLineSelect ){
UINT uFormat = ::RegisterClipboardFormat( L"VisualStudioEditorOperationsLineCutCopyClipboardTag" );
if( 0 != uFormat ){
hgClipMSDEVLine2 = ::GlobalAlloc(
hgClipMSDEVLine2 = GlobalAlloc(
GMEM_MOVEABLE | GMEM_DDESHARE,
1
);
Expand Down Expand Up @@ -276,7 +276,7 @@ bool CClipboard::GetText(CNativeW* cmemBuf, bool* pbColumnSelect, bool* pbLineSe
//矩形選択や行選択のデータがあれば取得
if( NULL != pbColumnSelect || NULL != pbLineSelect ){
UINT uFormat = 0;
while( ( uFormat = ::EnumClipboardFormats( uFormat ) ) != 0 ){
while( ( uFormat = EnumClipboardFormats( uFormat ) ) != 0 ){
// Jul. 2, 2005 genta : check return value of GetClipboardFormatName
WCHAR szFormatName[128];
if( ::GetClipboardFormatName( uFormat, szFormatName, _countof(szFormatName) - 1 ) ){
Expand Down Expand Up @@ -425,7 +425,7 @@ static CLIPFORMAT GetClipFormat(const wchar_t* pFormatName)
bool CClipboard::IsIncludeClipboradFormat(const wchar_t* pFormatName)
{
CLIPFORMAT uFormat = GetClipFormat(pFormatName);
if( ::IsClipboardFormatAvailable(uFormat) ){
if( IsClipboardFormatAvailable(uFormat) ){
return true;
}
return false;
Expand Down Expand Up @@ -508,7 +508,7 @@ bool CClipboard::SetClipboradByFormat(const CStringRef& cstr, const wchar_t* pFo
case 0: nulLen = 0; break;
default: nulLen = 0; break;
}
HGLOBAL hgClipText = ::GlobalAlloc(
HGLOBAL hgClipText = GlobalAlloc(
GMEM_MOVEABLE | GMEM_DDESHARE,
nTextByteLen + nulLen
);
Expand All @@ -521,7 +521,7 @@ bool CClipboard::SetClipboradByFormat(const CStringRef& cstr, const wchar_t* pFo
memset( &pszClip[nTextByteLen], 0, nulLen );
}
::GlobalUnlock( hgClipText );
::SetClipboardData( uFormat, hgClipText );
SetClipboardData( uFormat, hgClipText );

return true;
}
Expand Down Expand Up @@ -563,7 +563,7 @@ bool CClipboard::GetClipboradByFormat(CNativeW& mem, const wchar_t* pFormatName,
if( uFormat == (CLIPFORMAT)-1 ){
return false;
}
if( !::IsClipboardFormatAvailable(uFormat) ){
if( !IsClipboardFormatAvailable(uFormat) ){
return false;
}
if( nMode == -2 ){
Expand All @@ -576,10 +576,10 @@ bool CClipboard::GetClipboradByFormat(CNativeW& mem, const wchar_t* pFormatName,
}
return bret;
}
HGLOBAL hClipData = ::GetClipboardData( uFormat );
HGLOBAL hClipData = GetClipboardData( uFormat );
if( hClipData != NULL ){
bool retVal = true;
const BYTE* pData = (BYTE*)::GlobalLock( hClipData );
const BYTE* pData = (BYTE*)GlobalLock( hClipData );
if( pData == NULL ){
return false;
}
Expand Down Expand Up @@ -633,10 +633,6 @@ bool CClipboard::GetClipboradByFormat(CNativeW& mem, const wchar_t* pFormatName,
return false;
}

// -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- //
// staticインターフェース //
// -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- //

//! クリップボード内に、サクラエディタで扱えるデータがあればtrue
bool CClipboard::HasValidData()
{
Expand All @@ -663,14 +659,14 @@ CLIPFORMAT CClipboard::GetSakuraFormat()
}

//!< クリップボードデータ形式(CF_UNICODETEXT等)の取得
int CClipboard::GetDataType()
int CClipboard::GetDataType() const
{
//扱える形式が１つでもあればtrue
// 2013.06.11 GetTextの取得順に変更
if(::IsClipboardFormatAvailable(GetSakuraFormat()))return GetSakuraFormat();
if(::IsClipboardFormatAvailable(CF_UNICODETEXT))return CF_UNICODETEXT;
if(::IsClipboardFormatAvailable(CF_OEMTEXT))return CF_OEMTEXT;
if(::IsClipboardFormatAvailable(CF_HDROP))return CF_HDROP;
if(IsClipboardFormatAvailable(GetSakuraFormat()))return GetSakuraFormat();
if(IsClipboardFormatAvailable(CF_UNICODETEXT))return CF_UNICODETEXT;
if(IsClipboardFormatAvailable(CF_OEMTEXT))return CF_OEMTEXT;
if(IsClipboardFormatAvailable(CF_HDROP))return CF_HDROP;
return -1;
}

Expand All @@ -689,3 +685,15 @@ BOOL CClipboard::EmptyClipboard() const {
BOOL CClipboard::IsClipboardFormatAvailable(UINT format) const {
return ::IsClipboardFormatAvailable(format);
}

UINT CClipboard::EnumClipboardFormats(UINT format) const {
return ::EnumClipboardFormats(format);
}

HGLOBAL CClipboard::GlobalAlloc(UINT uFlags, SIZE_T dwBytes) const {
return ::GlobalAlloc(uFlags, dwBytes);
}

LPVOID CClipboard::GlobalLock(HGLOBAL hMem) const {
return ::GlobalLock(hMem);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

カバレッジが想定より低いのは、API呼出をモック化するために追加したこれらのメソッドがテストで実行されないためと考えられます。（テストで動くのはモックコードだから、ここは動かない）

テストではクリップボードのAPIを呼ばないようにして、クリップボードが勝手に書き換わる事態を回避しよう！で始めた対応なので、APIを呼ばないこと自体は気にしなくて良い認識です。

「CClipboardクラスのテストカバレッジ」を考えたときに、「テストで実行されないメソッドがある」の状態なので、何か対策したほうがいいような気はします。（というか、ちょっと自分でやりたいです:smiley:

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

「CClipboardクラスのテストカバレッジ」を考えたときに、「テストで実行されないメソッドがある」の状態なので、何か対策したほうがいいような気はします。（というか、ちょっと自分でやりたいです😃

対策としては別のクラスに実装してコンストラクタで注入するといったところでしょうか？ そういう手法の方がモックの作り方としては一般的らしく、設計を再考してもいいかもしれませんね。何かこの件で案があればぜひお知らせください。

Copy link
Contributor

@berryzplus berryzplus May 9, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

対策としては別のクラスに実装してコンストラクタで注入するといったところでしょうか？

テスト導入のときに書いた気がするんですけど、
APIラッパー部分をCClipbordApiとかの別クラスに分割してあげれば、
CClipboardクラスのカバレッジは限りなく100%にできます。
実装コードの検証をしたいのは独自に組んだロジックだけなので、とりあえずソコが目標になるはず。

コンストラクタで注入（＝依存関係の逆転）は、やれたらカッコイイですね。
サクラエディタのWinMainはDIを実装できるほど簡潔でないように思います。
問題点「WinMainが簡潔でない」の対策は大変なので、当面は放置ですかね...。

}
6 changes: 5 additions & 1 deletion sakura_core/_os/CClipboard.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ class CClipboard{
//演算子
operator bool() const{ return m_bOpenResult!=FALSE; } //!< クリップボードを開けたならtrue

int GetDataType() const; //!< クリップボードデータ形式(CF_UNICODETEXT等)の取得

private:
HWND m_hwnd;
BOOL m_bOpenResult;
Expand All @@ -65,7 +67,6 @@ class CClipboard{
public:
static bool HasValidData(); //!< クリップボード内に、サクラエディタで扱えるデータがあればtrue
static CLIPFORMAT GetSakuraFormat(); //!< サクラエディタ独自のクリップボードデータ形式
static int GetDataType(); //!< クリップボードデータ形式(CF_UNICODETEXT等)の取得

protected:
// 単体テスト用コンストラクタ
Expand All @@ -77,5 +78,8 @@ class CClipboard{
virtual HANDLE GetClipboardData(UINT uFormat) const;
virtual BOOL EmptyClipboard() const;
virtual BOOL IsClipboardFormatAvailable(UINT format) const;
virtual UINT EnumClipboardFormats(UINT format) const;
virtual HGLOBAL GlobalAlloc(UINT uFlags, SIZE_T dwBytes) const;
virtual LPVOID GlobalLock(HGLOBAL hMem) const;
};
#endif /* SAKURA_CCLIPBOARD_4E783022_214C_4E51_A2E0_54EC343500F6_H_ */
Loading