sakura-editor / management-forum

管理・運用向けフォーラム(Issues をフォーラム代わりに使う)
2 stars 0 forks source link

UI に依存しない機能実装、仕様変更の際に単体テストを導入したい #10

Open m-tmatma opened 6 years ago

m-tmatma commented 6 years ago

UI に依存しない機能実装、仕様変更の際に単体テストを導入したい

googletest を使うのが、いいと思ってます。

KENCHjp commented 6 years ago

UIに依存しない実装部分に対してgoogletestで得られる回帰テストの工数削減って、googletestを仕込む工数と見合った場合にどれぐらい効果的なんしょう。 ほぼUIの機能なサクラエディタでUIに依存しない部分のデグレ確認の効果があまりイメージつかなくて。 例えばメモリリークとか余計なファイルやレジストリを更新しないかとかそういうAPI周りも含めてチェックしてる暴走を止めるチェックとかしてくれるとうれしいなぁとgoogletestを仕込む工数と効果がわからずしゃべってます。

JavaのjUnitとかホワイトボックスなライブラリのテストとかのイメージはある程度つくんですが、テストコードとかも関数の引数からある程度自動生成してくれたりとか、仕込む工数もなんとなく見えてたりするんですが。

kobake commented 6 years ago

@m-tmatma さんはそのあたり組んでみた経験はありますか?

自分は経験ないので何かサンプル等あれば見たいです。理想としてはサクラエディタのちっちゃい機能のテストサンプルを目で見れると実感がわいて意見や感想も出しやすいです。

m-tmatma commented 6 years ago

小さいサンプルは以下で作っています。 https://github.com/m-tmatma/googletest-sample-cmake

以下がテスト対象コードです。 https://github.com/m-tmatma/googletest-sample-cmake/tree/master/src

以下がテストコードです。 https://github.com/m-tmatma/googletest-sample-cmake/tree/master/tests

kobake commented 6 years ago

お、ちょっとだけ触ってみたい気持ち湧きました。

例えば https://github.com/sakura-editor/sakura/issues/160 で実装予定のフォーマット付き Append 関数なんかは、テストコード書いておくと動作検証になる上に関数の利用サンプルにもなって良さそうだなーとか思いました。

「テストコードの実装を強制する」のではなく「テスト書きたい人はテストも書けるようにしておく」程度のスタンスであれば自分は導入歓迎です。

(PR が上がってきたときに、「この PR にはテストコードが含まれていないから approve しません」みたいな厳しいスタンスは避けたい。まぁテストコードがあったほうが好ましいケースというのはあるにはありそうなので、そういうものだけ限定的にテストコード実装を促すのはアリといえばアリという気持ちもあります)

このサンプルでは Travis CI でテスト回してる感じですね。 AppVeyor のバッジはありましたけどパーミッションがないみたいなエラーで内容見れませんでした。(そもそも appveyor.yml も存在しないようだったのでこのバッジは過去の名残か何かですかね??)

m-tmatma commented 6 years ago

AppVeyor のバッジはありましたけどパーミッションがないみたいなエラーで内容見れませんでした。(そもそも appveyor.yml も存在しないようだったのでこのバッジは過去の名残か何かですかね??)

アカウント名変えたのに対応してませんでした。 修正しました。

kobake commented 6 years ago

修正確認しました。ありがとうございます。 AppVeyor 側でもテスト走ってるの確認できました。

サクラエディタにテスト組み込むとしたら AppVeyor に追記する感じですかね?

m-tmatma commented 6 years ago

(そもそも appveyor.yml も存在しないようだったのでこのバッジは過去の名残か何かですかね??)

UI で設定してました。UI で appveyor.yml をエクスポートして登録しました。

m-tmatma commented 6 years ago

サクラエディタにテスト組み込むとしたら AppVeyor に追記する感じですかね?

Windows 固有の機能を使うのであればそうですね。

kobake commented 6 years ago

あー、なるほど、今判断基準が定まりました。 Windows 固有の機能は使い得ると考えているので、やるとしたら AppVeyor が良さそうです。

KENCHjp commented 6 years ago

いいすね。サンプルあるとわかりやすくてイメージもつきやすいですね。 いいだしっぺがサンプルも提示するって文化大好きです。

kobake commented 6 years ago

「やる」って方向性で良いと思ってます。適当なタイミングで PR 飛ばしてもらえれば僕か誰かが見ると思います。

m-tmatma commented 6 years ago

https://github.com/sakura-editor/sakura/issues/181 を登録しました。

berryzplus commented 6 years ago

単体テスト導入には賛成。

google test の利用にも、もちろん賛成。 windows API に依存するプログラムを単体テストするにはモック機構がほぼ必須で、 google test は モック機構 とセットで開発がすすめられているプロジェクトだから。

テストファイル管理が cmake なところが少し疑問。 何故かというと、visual studio 2017 は google test プロジェクトをサポートしているから。 現時点で test adaptor for google test には困ったバグがあるので仕方ないか、とも思います。 https://github.com/Microsoft/TestAdapterForGoogleTest/issues/121

vs2017 は cmake プロジェクトを取り込めるけど、 cmake プロジェクトを取り込むことで visual studio で出来る本来の機能が損なわれないかが心配。 現状、準備ができてるのは @m-tmatma さんだけなので、 提案を運用に載せていくためのお手伝いはできるかぎりしていきたい所存です。

m-tmatma commented 6 years ago

テストファイル管理が cmake なところが少し疑問。

https://github.com/google/googletest/blob/master/googletest/README.md

で以下と書いてあります。

Before settling on CMake, we have been providing hand-maintained build projects/scripts 
for Visual Studio, Xcode, and Autotools. While we continue to provide them for convenience,
they are not actively maintained any more. We highly recommend that you follow the instructions
in the above sections to integrate Test with your existing build system.

もうメンテされないものをこれから使うのはよろしくないと思います。

m-tmatma commented 6 years ago

一応書いておくと cmake はビルド用の generator なので、Visual Studio 用のソリューション、プロジェクトを動的に生成できます。

berryzplus commented 6 years ago

もうメンテされないものをこれから使うのはよろしくないと思います。

google test 自体をビルドする方法としては、使われなくなったという認識です。

google test はそれでいいと思います。 でも、サクラエディタは当面、windowsだけをターゲットにすると考えています。

一応書いておくと cmake はビルド用の generator なので、Visual Studio 用のソリューション、プロジェクトを動的に生成できます。

動的に生成されたプロジェクトを通常の vs2017 プロジェクトと同じように触れるなら文句はないのですが、実際そうではないことが気になっています。

究極的な話、vs2017 → cmake の移行は、sed一発でもできないことはない認識です。

m-tmatma commented 6 years ago

cmake は単体テストに使うだけで、既存のプロジェクトは 変えないです。

berryzplus commented 6 years ago

cmake は単体テストに使うだけで、既存のプロジェクトは 変えないです。

理解してると思います。

そのうえで「いいんだっけ?」というコメントを残そうとしています。

基本的には「賛同してる」と思っていただいて構いません。

berryzplus commented 6 years ago

現状整理

googletestについて

方法 実現性 説明
googletestをsubmoduleする × vs2017でビルドできない
googletestをforkする 上記対策版パッチを当てたものを使う
googletestをビルドしない vs2017同梱の「ほぼ最新版」を使う

まぁ、2択。

vs2017からgoogletestを使う方法について

方法 実現性 説明
普通のexeとして作る googletestバイナリのビルドが必要。CMakeで作成可。
microsoftが用意したNugetを使う vcxprojのExtensionsに記述すれば使える。CMakeでの実現方法不明。

テストをどう構成したいかによって変わってきそう。

CMakeでテスト構成を組む場合のメリット

普通のプロジェクトで作る場合との比較

方法 説明
CMakeプロジェクト exeを作るサブプロジェクトを追加できる
普通のプロジェクト exeを作るサブプロジェクトは追加できない
berryzplus commented 6 years ago

自分で書いた

究極的な話、vs2017 → cmake の移行は、sed一発でもできないことはない認識です。

事実誤認だった、ということを書いておきます。

ちょっとでも細かいことをしようとすると、 途端にcmakeの混沌の闇に落ちていく仕様のようです。