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.

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

googletest から doctest へ乗り換え #1002

Closed
wants to merge 3 commits into from
Closed

googletest から doctest へ乗り換え #1002

wants to merge 3 commits into from

Conversation

beru
Copy link
Contributor

@beru beru commented Aug 18, 2019

PR の目的

使用するユニットテストフレームワークを googletest から doctest へ変更するのが目的です。

カテゴリ

  • その他

PR の背景

現時点では Google Test をユニットテストフレームワークとして使用していますが、その導入に付きまとって色々な手間が発生しつづけています。

引き続き Google Test を使い続ければ同じような手間が今後も発生する事が想像出来ます。

PR のメリット

googletest と違って導入に手間がかかりません。doctest はヘッダファイルを #include するだけで済むので構成がすっきりします。ライブラリをビルドしたりパッケージマネージャを使ってインストールする必要が生じません。

PR のデメリット (トレードオフとかあれば)

doctest の使い方を調べる必要が生じます。

PR の影響範囲

ユニットテスト関連

関連チケット

#433 #434

参考資料

https://github.com/onqtam/doctest

@beru beru added CI appveyor など CI 関連 【ChangeLog除外】 UnitTest labels Aug 18, 2019
@AppVeyorBot
Copy link

Build sakura 1.0.2163 completed (commit 9641ca7f08 by @)

Copy link
Contributor

@berryzplus berryzplus left a comment

Choose a reason for hiding this comment

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

以前もコメントしましたが、乗換の前に導入提案が要ると思います。

現時点では Google Test をユニットテストフレームワークとして使用していますが、
その導入に付きまとって色々な手間が発生しつづけています。

引き続き Google Test を使い続ければ同じような手間が今後も発生する事が想像出来ます。

対策は、原因を見極めて適切に行うべきだと思います。

「手間が発生しつづけている」の原因はなんでしょうか?

個人的な見解は以下のとおりです。

  • バッチ処理の作成技術が低い
  • メンバーのコミュニケーション能力が低い
  • googletestフレームワークについて、習熟度が低い

なんか色々怒られそうな見解を並べてみましたが「ああ、そうね。」と思える内容だと思います。
doctestへの乗換を行うことによって、原因に対処できるかというと微妙な気がします。

  • バッチ処理の作成技術が低くても、doctestならテストが作れる
  • メンバーのコミュニケーション能力が低くても、doctestなら一人でテストが作れる
  • フレームワークについて、習熟度が低くても、doctestなら十分なテストが作れる

というわけで、ツールを変えることは対策にならないような気がしています。

あと、導入提案であれば真面目に考えます。

@takke
Copy link
Member

takke commented Aug 18, 2019

逆にそこまでgoogletestにこだわる理由は何なんでしょう。

@beru
Copy link
Contributor Author

beru commented Aug 18, 2019

以前もコメントしましたが、乗換の前に導入提案が要ると思います。

導入提案というのは Issue を作成する事ですか?Issueを作成しても誰も反応しなかったらその導入提案は先に進みません。まぁそれなら他の人は乗り気ではないと判断が出来るのでそこで終了という以前と同じ結論になりそうですが…。

対策は、原因を見極めて適切に行うべきだと思います。

「手間が発生しつづけている」の原因はなんでしょうか?

個人的な見解は以下のとおりです。

  • バッチ処理の作成技術が低い
  • メンバーのコミュニケーション能力が低い
  • googletestフレームワークについて、習熟度が低い

自分が気にしているのは

  • ビルド周りのスクリプトに googletest にまつわる記述が多くなっている事
  • Google Test は中間生成物が多い事
  • Visual Studioのソリューションにテストプロジェクトが組み込まれたけれど、開発環境の画面を見てもテストフレームワークのファイルへの繋がりが分からない事
    • tests/unittests/tests1.vcxproj を読んで、そこから Import している tests/googletest.targets を見て、さらにそこから呼び出している tests/googletest.build.cmd を見てやっと仕組みが分かります。

等です。「それらは別に手間じゃないよ、手間だと感じるやつがカスなのが悪いんだよ。」って言われたら、「いやまぁそうだけどライブラリ変えて手間を減らせるならそっちの方が良いんで無いの?」と返します。

なんか色々怒られそうな見解を並べてみましたが「ああ、そうね。」と思える内容だと思います。
doctestへの乗換を行うことによって、原因に対処できるかというと微妙な気がします。

@berryzplus さんが記載した点に関しては、doctest に乗り換えても当然解消しないと思います。そんな魔法のようなものは存在しないでしょう。

  • バッチ処理の作成技術が低くても、doctestならテストが作れる
  • メンバーのコミュニケーション能力が低くても、doctestなら一人でテストが作れる
  • フレームワークについて、習熟度が低くても、doctestなら十分なテストが作れる

というわけで、ツールを変えることは対策にならないような気がしています。

テストコードを増やす、という目的に注目するとしたらそれに対しての直接的な解にはなりえないと思います。

各メンバーのバッチ処理の作成技術が高くてコミュニケーションが十分に取れてさえいればテストが問題無く作れるのかというと、それはよくわかりません。コミュニケーション能力に洗脳スキルを入れるとしたらまぢそれだけ欲しいです。

フレームワークの習熟度に関しては、テスト書いていく中で自然とドキュメント読むだろうし、ユニットテストフレームワークって世の中に色々ありますけど共通している点は多いので、そんなに懸念はしていません。

テストの量が今少ないのは単にみんなにテスト書く事に対する意欲が無いからであって、それはPR1つで解決出来る話だとは思えません。

@beru
Copy link
Contributor Author

beru commented Aug 18, 2019

逆にそこまでgoogletestにこだわる理由は何なんでしょう。

googletest にこだわっていない自分が推測するにサンクコストじゃないかなと思います。

@takke
Copy link
Member

takke commented Aug 18, 2019

差分を改めて見ましたが、各バッチが予想以上にずいぶんとすっきりすると感じました。メンテナンスも楽になると思うので導入する方向で進めた方がいいと個人的には思います。

@beru
Copy link
Contributor Author

beru commented Aug 18, 2019

削除したファイルに関して補足します。

ファイルパス 内容 削除理由
tests/googletest.build.cmd tests/googletest.targets から呼び出される gtest を用意するコマンド群を実行 googletest を使わないように変更した為
tests/googletest.targets tests/unittests/tests1.vcxproj から Import されるGoogleTestのライブラリをリンクする為の記述があるファイル。統合開発環境からは見えない googletest を使わないように変更した為
tests/test_result_filter_tell_AppVeyor.bat tests/run-tests.bat から呼び出されるAppVeyor の場合にパイプでテストexeの標準出力を受け取って appveyor コマンドの呼び出しに変換 テストexeの出力形式が変更される可能性を考慮して。今後復活させるべきかも。
tests/unittests/test-parameterized.cpp Parameterized Test のサンプルコード doctestにもパラメータ指定によるテスト方法は存在しますが記述の方向性が違うかなと感じた為。doctest のが良い気がする。test-parameterized.cpp のサンプルコードはあえてパラメータテストで書かないでも実現出来る内容なのでサンプルとして良くない気が…。

@berryzplus
Copy link
Contributor

以前もコメントしましたが、乗換の前に導入提案が要ると思います。

導入提案というのは Issue を作成する事ですか?Issueを作成しても誰も反応しなかったらその導入提案は先に進みません。まぁそれなら他の人は乗り気ではないと判断が出来るのでそこで終了という以前と同じ結論になりそうですが…。

導入提案 = doctestを追加で使えるようにするPRです。

置き換えするかどうかの判断と別に、doctestでテストを書けることの証明が必要だと思います。

導入するかどうか? ← 単純に使えるかどうかの判断で足りる
置換するかどうか? ← 使えるかどうか+アルファの判断が要る

プラスアルファの判断には、 googletest を選択した理由を知る必要があります。
長くなりそうなのと @takke さんから個別に問い合わせ入ってるのでコメント分けます。

自分が気にしているのは
•ビルド周りのスクリプトに googletest にまつわる記述が多くなっている事
•Google Test は中間生成物が多い事
•Visual Studioのソリューションにテストプロジェクトが組み込まれたけれど、開発環境の画面を見てもテストフレームワークのファイルへの繋がりが分からない事 ◦tests/unittests/tests1.vcxproj を読んで、そこから Import している tests/googletest.targets を見て、さらにそこから呼び出している tests/googletest.build.cmd を見てやっと仕組みが分かります。

  • ビルド周りのスクリプトに googletest にまつわる記述が多くなっている
    • googletest に依存する記述が多いのはめんどくさいことだと思っています。
    • 将来的に googletest.build.cmd/googletest.targets に集約できたらいいなと思っています
  • Google Test は中間生成物が多い事
    • 中間生成物についてはそんなでもないように思います。
    • gtest.lib と gtest_main.lib の 2つだけ です。pdbを含めると4個ですが。
  • Visual Studioのソリューションにテストプロジェクトが組み込まれたけれど、開発環境の画面を見てもテストフレームワークのファイルへの繋がりが分からない事 ◦tests/unittests/tests1.vcxproj を読んで、そこから Import している tests/googletest.targets を見て、さらにそこから呼び出している tests/googletest.build.cmd を見てやっと仕組みが分かります。
    • 単にテストを書いたり実行したりするときに詳細が気にならないように視界から外れるように構成しています。
    • ビルドは、困ったときに対応できれば詳細にはこだわらなくてよいと思います。

テストコードを増やす、という目的に注目するとしたらそれに対しての直接的な解にはなりえないと思います。

このあたりは難しいですが、テストコードを増やすこと自体は目的じゃないつもりです。

テストコードを整備する目的は、以下のような感じです。

  • 開発者の想定がx86/x64環境下で期待通りに成立することを証明する
  • あるコード変更が既存処理に影響を与えないことを証明する
  • 新設コードの仕様をドキュメントなしで簡潔に説明するための手段を提供する

コミュニケーション能力に洗脳スキルを入れるとしたらまぢそれだけ欲しいです。

そうね...

@beru
Copy link
Contributor Author

beru commented Aug 18, 2019

導入提案 = doctestを追加で使えるようにするPRです。

置き換えするかどうかの判断と別に、doctestでテストを書けることの証明が必要だと思います。

導入するかどうか? ← 単純に使えるかどうかの判断で足りる
置換するかどうか? ← 使えるかどうか+アルファの判断が要る

プラスアルファの判断には、 googletest を選択した理由を知る必要があります。

なんか前と流れが同じな気がします。

既存の Google Test に関連するところには手を付けないで、別のユニットテストフレームワークへの対応を追加すると複雑度を下げる方向では無く上げる方向になってしまうのが嫌なところですね。

証明って書かれるとなんだか表現というか形式が大げさな気がします。置換では無くて導入(オプションで追加対応的な?)というやり方でないと doctest でテストが書けることが判断出来ないという事でしょうか?

まぁソフトな言い方にしたら順を追って導入したい、的な事なのかな。。

そういう進め方じゃないと考慮もしたくないという事は分かりました。

  • Google Test は中間生成物が多い事

    • 中間生成物についてはそんなでもないように思います。
    • gtest.lib と gtest_main.lib の 2つだけ です。pdbを含めると4個ですが。

もっとあると思いますが、まぁ別に気にならないなら話は噛み合わないですね。

  • 単にテストを書いたり実行したりするときに詳細が気にならないように視界から外れるように構成しています。
  • ビルドは、困ったときに対応できれば詳細にはこだわらなくてよいと思います。

まぁ自分の環境で問題が確認出来なかったらその問題は存在しないと認識しますしね。

このあたりは難しいですが、テストコードを増やすこと自体は目的じゃないつもりです。

#1002 (review) で書かれていた テストが作れる という記述はテストを増やす事だと思っていたのですがそうではないようですね。どうもテストを回す仕組みを作る事が目的のようですね。

テストコードを整備する目的は、以下のような感じです。

  • 開発者の想定がx86/x64環境下で期待通りに成立することを証明する
  • あるコード変更が既存処理に影響を与えないことを証明する
  • 新設コードの仕様をドキュメントなしで簡潔に説明するための手段を提供する

それって一般論ですが、増やす事自体は目的じゃないよ、って事に対する説明なんですね。自分が 増やす って書いたから、単に増やせば良いってもんじゃないよという返しになったわけだ。。

@beru
Copy link
Contributor Author

beru commented Aug 18, 2019

まぁ「ユニットテスト周りの整備を楽にしたいから別のユニットテストフレームワークに切り替えましょう」って言ってもメンテしてる本人はこのままで良いって判断なんだから好きにしてもらうのが一番ですね。。

@beru beru closed this Aug 18, 2019
@beru beru deleted the doctest branch August 18, 2019 12:49
@berryzplus
Copy link
Contributor

まぁソフトな言い方にしたら順を追って導入したい、的な事なのかな。。

そういう進め方じゃないと考慮もしたくないという事は分かりました。

ぬ?と思ったので、改めて考慮してみました。

進め方に関することは何にも決めていないです。
やれるときに、やれる人が進める、で全体方針はありません。
で、実際導入するものとして変更内容を見てみました。

  • visual studio の test explorer が使えませんでした。
    • test adapter が存在するならインストールすれば対応可能と思いますが、導入提案としてはgoogletest相当の機能が使える状態にするか、特定機能が使えないことをデメリットに上げてほしいです。
  • テストの実行ログに実行したテスト名が表示されませんでした。
    • オプションを指定すれば同等のことができると思いますが、導入提案としてはgoogletest相当の機能が使える状態にするか、特定機能が使えないことをデメリットに上げてほしいです。
  • 一部で名前が変更されたテストがあるようです。
    • StdControl Wnd_GetText ⇒ Wnd_GetText
    • テスト名を表示するオプションを調べてないので同等の名前になっているかも知れません。
  • tests/unittests/test-is_mailaddress.cpp のうんこマクロがしれっと削除されてる気がします。
  • DEATHテストが使えないようです。
    • 導入提案としてはgoogletest相当の機能が使える状態にするか、特定機能が使えないことをデメリットに上げてほしいです。
  • disabled testの仕様が異なるようです。
    • 導入提案としてはgoogletest相当の機能が使える状態にするか、特定機能が使えないことをデメリットに上げてほしいです。
    • あと、導入提案と関係なさそうなコメントの削除が発生しています。
  • テスト結果のXML出力が使えないようです。
    • 導入提案としてはgoogletest相当の機能が使える状態にするか、特定機能が使えないことをデメリットに上げてほしいです。

まず、何ができるようになって、何ができなくなるのか、それを把握することが第一歩だと思います。導入提案を先にしてくれといったのは、「似た感じにできるんだから変えてもいいよね?」というのが分かりやすいと考えたからです。ざっと見た感じ、doctestにはgoogletestよりも機能が少ない印象を持ちました。つまり、似た感じにはできない。ただ、機能が減るから絶対ダメかというとそうじゃなくて、メリットとデメリットの受け取り方は立場によって違うと思います。

こんだけメリデメがあって、このメリットを重視して変えたいと思う。デメリットのうち、これは本当は困るんだけど、きっとそのうち機能追加されるから当面は我慢しようぜ。

導入提案のイメージはそんな感じです。

@berryzplus
Copy link
Contributor

閉じられてしまったのですがコメント付けると予告したので。

逆にそこまでgoogletestにこだわる理由は何なんでしょう。

googletest にこだわっていない自分が推測するにサンクコストじゃないかなと思います。

  • テストアダプターが visual studio に標準インストールされるからです。
    • 個別にアダプターをインストールしなくても、テストエクスプローラーを使えます。
    • チェックは外せるので、外しちゃってたら意味無いっすけど(^^;
  • 一応、最も有名なテストフレームワークの1つだからです。
    • 他のテストフレームワークを使うよりも学習コストを削減できると考えました。あまり効果は出てないようですが 😢

実は サンクコスト に関してはあんまり気にしてないです。

@beru
Copy link
Contributor Author

beru commented Aug 18, 2019

まぁソフトな言い方にしたら順を追って導入したい、的な事なのかな。。
そういう進め方じゃないと考慮もしたくないという事は分かりました。

ぬ?と思ったので、改めて考慮してみました。

ちょっと言い方がきつかったですがちゃんと見てくれて有り難いです。

進め方に関することは何にも決めていないです。
やれるときに、やれる人が進める、で全体方針はありません。
で、実際導入するものとして変更内容を見てみました。

欲を言うと変更内容に関する確認を最初からしてほしかったですね。

  • visual studio の test explorer が使えませんでした。

    • test adapter が存在するならインストールすれば対応可能と思いますが、導入提案としてはgoogletest相当の機能が使える状態にするか、特定機能が使えないことをデメリットに上げてほしいです。

自分がその機能を使っていない事もあってデメリットと感じていませんでしたが、愛用している人からしたら「ダイヤ改正で最寄り駅に快速が止まらなくなった」ぐらいの衝撃を受けるのかもしれないですね。

  • テストの実行ログに実行したテスト名が表示されませんでした。

    • オプションを指定すれば同等のことができると思いますが、導入提案としてはgoogletest相当の機能が使える状態にするか、特定機能が使えないことをデメリットに上げてほしいです。

これについて気にしていませんでしたが、コマンドラインオプション--success=1 で成功したテストに関しても出るようなので、このオプションが使えるかもしれないとは思います。

  • 一部で名前が変更されたテストがあるようです。

    • StdControl Wnd_GetText ⇒ Wnd_GetText
    • テスト名を表示するオプションを調べてないので同等の名前になっているかも知れません。

Google Test では TEST マクロの引数が2つ有って、1つ目がテストケース名、2つ目がテスト名ですが、記載が面倒だったので2つ目のテスト名だけをコピペしました。その為、doctest の TEST_CASE マクロの引数の文字列の内容が重複したりとかあるかもしれませんが、その場合に問題無いのかどうかは一切確認してません。

  • tests/unittests/test-is_mailaddress.cpp のうんこマクロがしれっと削除されてる気がします。

結構削除するのが大変でした。

  • DEATHテストが使えないようです。

    • 導入提案としてはgoogletest相当の機能が使える状態にするか、特定機能が使えないことをデメリットに上げてほしいです。

tests/unittests/test-mydevmode.cpp 内で使われていましたね。ロードマップには載せてないけど検討中 みたいです。

  • disabled testの仕様が異なるようです。

    • 導入提案としてはgoogletest相当の機能が使える状態にするか、特定機能が使えないことをデメリットに上げてほしいです。

tests/unittests/test-main.cppcontext.addFilter("test-case-exclude", "DISABLED_*"); する事で特定のテストを無効化しています。方法は変わっていますが効果としては同等かなと…。

  • あと、導入提案と関係なさそうなコメントの削除が発生しています。

どこだろう?tests/unittests/test-is_mailaddress.cppIsEqualResult で受けているラムダ式の中の記述の話ですか?これは対応が面倒に感じて…。

  • テスト結果のXML出力が使えないようです。

    • 導入提案としてはgoogletest相当の機能が使える状態にするか、特定機能が使えないことをデメリットに上げてほしいです。

tests/run-tests.bat の記述を変更して出力を切っていますね。XML出力は出来るようですが、形式が今までと同じか未確認なので切っておきました。

まず、何ができるようになって、何ができなくなるのか、それを把握することが第一歩だと思います。導入提案を先にしてくれといったのは、「似た感じにできるんだから変えてもいいよね?」というのが分かりやすいと考えたからです。ざっと見た感じ、doctestにはgoogletestよりも機能が少ない印象を持ちました。つまり、似た感じにはできない。ただ、機能が減るから絶対ダメかというとそうじゃなくて、メリットとデメリットの受け取り方は立場によって違うと思います。

こんだけメリデメがあって、このメリットを重視して変えたいと思う。デメリットのうち、これは本当は困るんだけど、きっとそのうち機能追加されるから当面は我慢しようぜ。

導入提案のイメージはそんな感じです。

あれ?当初と導入提案に関する定義が変わってるような…。。

PRの内容の説明が不十分なのはおっしゃる通りであと不備も多いと思います。

PRを導入提案として捉えてはくれなかったのは、やはり提案というものはいらすとやの素材で作ったポンチ絵を大量に載せたパワポじゃないと駄目だったという事でしょうか…。

自分も他の人が出したPRは全て導入提案とみなしてまずはパワポが添付されているかで判断したいと思います。

@berryzplus
Copy link
Contributor

欲を言うと変更内容に関する確認を最初からしてほしかったですね。

では次回からは、できる限りそんな感じで 😄

これについて気にしていませんでしたが、コマンドラインオプション の --success=1 で成功したテストに関しても出るようなので、このオプションが使えるかもしれないとは思います。

実はそれほど気にしていません。
googletestでも、detail 表示にしなければ実行したテスト名は表示されないので。
現状、テスト件数があまりにも少な過ぎるから「欲しい」という人の意見を通しているだけで、テスト件数が正常化したら「消して派」が勝つかも知れません。サクラエディタのコードは10万ステップあるので、業界標準でいったら数十万件のテストが必要なはずです。このあたりも認識のズレが大きなところだなぁと思っています。

•あと、導入提案と関係なさそうなコメントの削除が発生しています。

どこだろう?tests/unittests/test-is_mailaddress.cpp の IsEqualResult で受けているラムダ式の中の記述の話ですか?これは対応が面倒に感じて…。

tests/unittests/test-sample-disabled.cpp のコメントが大量に削除されてるように見えました。

XML出力は出来るようですが、形式が今までと同じか未確認なので切っておきました。

junit互換の確認が取れればそれ以上の確認は不要と思ってます。
コンソール出力を拾ってappveyorに連携するバッチを使うか、結果xmlをアップロードしないとテスト結果が反映されない仕組みだと理解しとります。 ← 人任せなので実はよく分ってない

あれ?当初と導入提案に関する定義が変わってるような…。。

そりゃ、中身見てしれっと変えてますから (マテ

PRを導入提案として捉えてはくれなかったのは、やはり提案というものはいらすとやの素材で作ったポンチ絵を大量に載せたパワポじゃないと駄目だったという事でしょうか…。

ぼくの主観では、絵じゃなくて表で十分だと思います。

既存がある状況で新しいものを提案されるときに気になるのは、
新しくできるようになることと、今後はできなくなること、の2点なので、
それが分かるカタチであればなんでもいいと思います。

ちなみにオイラぱわぽ嫌い...orz

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI appveyor など CI 関連 【ChangeLog除外】 UnitTest
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants