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

tests1.exe の依存ライブラリに gmock.lib を追加する #1798

Merged
merged 1 commit into from Feb 12, 2022

Conversation

kengoide
Copy link
Member

@kengoide kengoide commented Feb 12, 2022

PR の目的

test1.exe で Google Mock を使用できるようにします。

カテゴリ

  • ビルド関連
    • ビルド手順
    • Azure Pipelines
    • AppVeyor
    • GitHub Actions
    • ローカルビルド
  • その他の問題

PR の背景

#1707 (CClipboard のテスト) の検討中、実行するとクリップボードを書き換えてしまうテストは不便であるとの指摘がありました。対策として OS のクリップボード関数を模擬するモックの導入を検討しています。

現在のサクラエディタのビルドシステムでは Google Test と同時に Google Mock のビルドを行っているため、成果物である gmock.lib と gmock_main.lib をリンクするように指定するだけで使えるようになる…ということではないかと思っていますが、個人的に経験の薄い分野ですので正しい方策が判断できません。紆余曲折 (#894, #899) もあったように見受けられますし、詳しい方の見解を伺いたいと考えています。

「このやり方だとあれが動きません」といったご指摘をお待ちしています。

PR のメリット

  • 自動テストで Google Mock が使えるようになります。

PR のデメリット

@kengoide kengoide marked this pull request as draft February 12, 2022 14:08
@AppVeyorBot
Copy link

Build sakura 1.0.4032 completed (commit 4216b92298 by @k-kagari)

@berryzplus
Copy link
Contributor

gtest_main.libとgmock_main.libは排他関係にあるはずです。
記憶が確かなら、どちらも以下のような疑似コードが書かれているライブラリなはず。

int main(int argc, char* argv[]) {
return ライブラリ初期化関数(argc, argv);
}

上記の仕様的な部分からして、これらのライブラリは「独自に定義されたmain関数」とも競合します。
どれを使ったら効率がよいか?はプロジェクトによると思います。
サクラエディタの場合、既にgtest_main.libをリンクしてるので、gmock_main.libはリンクしなくていいんじゃないかと思います。。。

あとはこの辺を読んで分かった気になる感じだと思います。
http://opencv.jp/googlemockdocs/fordummies.html

@AppVeyorBot
Copy link

Build sakura 1.0.4034 failed (commit 09a35f5409 by @k-kagari)

* gmock.lib をリンクする指定を追加した。
* 不要だった gtest_main.lib のリンクをやめた。
@kengoide kengoide changed the title tests1.exe の依存ライブラリに gmock.lib と gmock_main.lib を追加する tests1.exe の依存ライブラリに gmock.lib を追加する Feb 12, 2022
@kengoide
Copy link
Member Author

サクラエディタの場合、既にgtest_main.libをリンクしてるので、gmock_main.libはリンクしなくていいんじゃないかと思います。。。

調べてみたところ gtest_main.lib すらも不要なのかもしれません。code-main.cpp で明示的に InitGoogleTest を呼んでいるため。

@AppVeyorBot
Copy link

Build sakura 1.0.4036 completed (commit 45a09bf473 by @k-kagari)

@ghost
Copy link

ghost commented Feb 12, 2022

サクラエディタの場合、既にgtest_main.libをリンクしてるので、gmock_main.libはリンクしなくていいんじゃないかと思います。。。

調べてみたところ gtest_main.lib すらも不要なのかもしれません。code-main.cpp で明示的に InitGoogleTest を呼んでいるため。

ファイル名に_mainが含まれるライブラリは、既定のmain関数を利用するときに使うものらしいので、独自にmain関数を書いているサクラエディタではgtest_main.libは不要だと思います。

よってリンクするのはgtest.libgmock.libの二つで良いです。

※最新のGoogleTestのドキュメント:https://google.github.io/googletest/primer.html#writing-the-main-function

@sonarcloud
Copy link

sonarcloud bot commented Feb 12, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

@kengoide kengoide marked this pull request as ready for review February 12, 2022 16:50
@kengoide
Copy link
Member Author

kengoide commented Feb 12, 2022

CI が通ったのでドラフト外します。本 PR の変更点は以下の通りです。

  • tests1.exe に gmock.lib をリンクする指定を追加した。
  • 不要だった gtest_main.lib のリンクをやめた。
  • InitGoogleTest の呼び出しを InitGoogleMock で置き換えた。

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

CIがまだ終わっていませんが、LGTMです。

@ghost
Copy link

ghost commented Feb 12, 2022

InitGoogleTest の呼び出しを InitGoogleMock で置き換えた。

GoogleMockのソースを見たところ、InitGoogleMockの中のこのあたりでInitGoogleTestが呼ばれているので大丈夫だと思いました。
それでも「個別に呼び出すべきだ」という人はいるかもしれません。

@AppVeyorBot
Copy link

Build sakura 1.0.4037 completed (commit 4893f484b2 by @k-kagari)

@kengoide
Copy link
Member Author

それでも「個別に呼び出すべきだ」という人はいるかもしれません。

  // Since Google Mock depends on Google Test, InitGoogleMock() is
  // also responsible for initializing Google Test.  Therefore there's
  // no need for calling testing::InitGoogleTest() separately.
  testing::InitGoogleMock(&argc, argv);

gmock_main.cc のコメントで no need だと明言されているので問題にならないと判断しました。もし納得しない人がいたとしてもこのコメントを盾にして説得します。😎

@kengoide
Copy link
Member Author

レビューありがとうございました。マージしてみて問題があれば適宜修正するか revert して対処します。

@kengoide kengoide merged commit a16ac2b into sakura-editor:master Feb 12, 2022
@kengoide kengoide deleted the feature/enable-gmock branch February 12, 2022 17:18
@AppVeyorBot
Copy link

Build sakura 1.0.4038 failed (commit d87641c744 by @k-kagari)

@berryzplus
Copy link
Contributor

InitGoogleTest の呼び出しを InitGoogleMock で置き換えた。

GoogleMockのソースを見たところ、InitGoogleMockの中のこのあたりでInitGoogleTestが呼ばれているので大丈夫だと思いました。 それでも「個別に呼び出すべきだ」という人はいるかもしれません。

ここが少し不安でしたが、ソースで確認できたなら安心です。

あとは CClipboard で使ってるクリップボード関数をモック化けるしてテスト書くぞ~!ですが、これまでやったことのない種類の変更なので、どうすすめるか考えないとあかん気がします。。。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants