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

単体テストで起きたassert失敗を捕捉できるように、コンソールモードではメッセージボックスを表示しないようにしたい #1362

Conversation

berryzplus
Copy link
Contributor

PR の目的

コンソールモードではメッセージボックスを表示しないようにします。

カテゴリ

  • その他の問題

PR の背景

このPRは #1351 デバッグ用のメッセージ出力を改善する をベースに、修正箇所を最小に抑えて変更内容を分かりやすくしたものです。

前提知識

  • サクラエディタはエラー情報の表示にメッセージボックスを利用しています。
    • メッセージボックスを使うのは、サクラエディタがGUIプログラムだからです。
    • メッセージボックスを表示するコードは続行するのにユーザー操作が必要になります。
      • ユーザー操作が必要ということは 自動テストできない ということです。

問題点

  • メッセージボックスを表示するコードは自動テストができません。

考えられる対策

  1. 自動テストからメッセージボックスを表示するコードを呼ばないようにする。
    これは、メッセージボックスを表示するコードはテストしない、になるので良くない気がします。
  2. 自動テストではメッセージボックスを表示しないようにする。
    メッセージボックス表示関数の呼び出しをラップして、呼ばれても表示しない、を実施する作戦です。
    サクラエディタには既にラッパー関数が存在しているため、多少のカスタムで実現可能です。
    この方式ならば、メッセージボックスを表示するコードの自動テストを書くことができます。

経緯

  1. 単体テストで文字列リソースを利用できるようにする #1275 単体テストで文字列リソースを利用できるようにする
    sakura.sln でビルドした単体テストにリソースが埋め込まれるような修正を実施。
  2. プロファイルマネージャを表示するかどうかの判定をテスト可能にする #1307 プロファイルマネージャを表示するかどうかの判定をテスト可能にする
    CDlgProfileMgrのテストを導入して、ダイアログ表示の要否判定をテスト可能に。
  3. 単体テストで x64 Debug で CDlgProfileMgr.TrySelectProfile_001 で assert になる #1334 単体テストで x64 Debug で CDlgProfileMgr.TrySelectProfile_001 で assert になる
    バッチから単体テストを実行した場合、MSVC版であってもCMakeでビルドされた単体テストが実行されるようになっていたことにより発生した問題。CMakeLists.txtのMSVC版設定をsakura_rc.resを取り込むように修正して、CMake×MSVCでビルドした単体テストでassertが出る問題は解決済み。単体テストでのassert時にテスト実行が中断されてしまう問題が残課題。
  4. 単体テストで assert が発生したときに MessageBox が表示されないようにする #1342 単体テストで assert が発生したときに MessageBox が表示されないようにする
    単体テストで x64 Debug で CDlgProfileMgr.TrySelectProfile_001 で assert になる #1334 の残課題対策PR。迷走中。
  5. デバッグ用のメッセージ出力を改善する #1351 デバッグ用のメッセージ出力を改善する
    単体テストで x64 Debug で CDlgProfileMgr.TrySelectProfile_001 で assert になる #1334 の残課題対策PR。単体テストで assert が発生したときに MessageBox が表示されないようにする #1342 のカウンターPRとして作成。push先ブランチが間違っていたため一旦破棄。
    このPRの元ネタ(完全版)。

PR のメリット

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

仕様・動作説明

ファイル シンボル - 説明
debug/Debug1.h TRACE(...) 追加 トレース出力(トレース箇所のファイルパスと行番号を出力してエラー解析を容易にする目的)を追加。
debug/Debug2.h debug_output(...) 削除 TRACEで置換。
debug/Debug2.h debug_exit() 変更 処理内容を終了のみに変更。
debug/Debug2.h debug_exit2(...) 削除 debug_exitに統合。
debug/Debug2.h warning_point() 変更 処理内容をデバッグブレーク発動に変更。
debug/Debug2.h assert() 変更 デバッガ出力でTRACEを使うように変更。エラー表示と分かるように赤○に×のアイコンを付加するように変更。
debug/Debug2.h assert_warning() 変更 デバッガ出力でTRACEを使うように変更。
util/MessageBoxF.cpp Wrap_MessageBox(...) 変更 コンソールモードではメッセージボックスを表示する代わりに標準エラー出力にメッセージを書き込むように変更。コンソールモードではメッセージボックスの表示結果に0(≒ありえない値)を返すように変更。

テスト内容

テスト1

既存メッセージボックスが表示できることの確認手順です。

手順

  1. このPRをfetchしてcheckoutし、ビルドします。(ビルド環境は何でも可)
  2. visual studioからビルドしたexeを実行します。
  3. aaaa と打って改行、行ごとコピーして3行以上にします。
  4. 全選択して 連続した重複行の削除(Ctrl + M) します。
    N行マージしました。のメッセージボックスが表示されたらOKです。

テスト2

変更後仕様のassertが設計通りに動作することの確認手順です。

手順

  1. このPRをfetchしてcheckoutし、Debug設定でビルドします。
  2. WinMain関数の先頭付近に assert(false): を挿入します。
  3. visual studioからビルドしたexeを実行します。
  4. 以下のように赤丸に×のエラーダイアログが表示されることを確認します。
    assertダイアログ
  5. エラーダイアログを閉じるとプログラムが終了することを確認します。
  6. 出力ペインにASSERTION FAILEDが出力されていることを確認します。
  7. 出力ペインのASSERTION FAILEDの行をダブルクリックして、該当箇所にジャンプできることを確認します。
    出力ペイン

PR の影響範囲

  • メッセージボックスを表示するコードについて自動テストを書けるようになります。
  • assert失敗時のメッセージボックスにアイコンが付くのでエラーメッセージだと分かりやすくなります。
  • デバッガで実行しているときにassertが失敗した場合、問題発生個所のコードに容易にアクセスできるようになります。

関連 issue, PR

resolves #1334 単体テストで x64 Debug で CDlgProfileMgr.TrySelectProfile_001 で assert になる
#1342 単体テストで assert が発生したときに MessageBox が表示されないようにする
#1351 デバッグ用のメッセージ出力を改善する

参考資料

https://docs.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-messagebox
https://docs.microsoft.com/en-us/windows/console/getstdhandle
http://eternalwindows.jp/windevelop/console/console02.html
https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/assert-macro-assert-wassert?view=vs-2019
https://c.keicode.com/lib/assert-when-you-should-use.php

@AppVeyorBot
Copy link

@berryzplus berryzplus force-pushed the feature/suppress_messagebox_for_console branch from 0c139e9 to 8ba9681 Compare August 9, 2020 08:51
@AppVeyorBot
Copy link

Build sakura 1.0.2972 failed (commit 41cc2b4b30 by @berryzplus)

@berryzplus berryzplus force-pushed the feature/suppress_messagebox_for_console branch from 8ba9681 to c36c7c1 Compare August 9, 2020 09:57
@AppVeyorBot
Copy link

@AppVeyorBot
Copy link

@berryzplus
Copy link
Contributor Author

レビューありがとうございます。マージしちゃいます。

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.

単体テストで x64 Debug で CDlgProfileMgr.TrySelectProfile_001 で assert になる
3 participants