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

AppVeyor のテストタブを利用します。 #580

Merged
merged 1 commit into from
Nov 11, 2018

Conversation

ds14050
Copy link
Contributor

@ds14050 ds14050 commented Oct 21, 2018

せっかくなので利用しましょう。

ジョブ一覧画面で実行されたテストの数が確認できるので、少し前にあったようにテストが意図せずスキップされていた場合に気づきやすくなる効果があります。

tests/run-tests.bat Outdated Show resolved Hide resolved
@m-tmatma
Copy link
Member

tests/test_result_filter_tell_AppVeyor.bat の for の中の処理は何のドキュメントに基づいて作成されていますか?

@m-tmatma m-tmatma added this to the next release milestone Oct 21, 2018
@m-tmatma m-tmatma added the CI appveyor など CI 関連 【ChangeLog除外】 label Oct 21, 2018
@m-tmatma
Copy link
Member

tests/test_result_filter_tell_AppVeyor.bat の for の中の処理は何のドキュメントに基づいて作成されていますか?

https://www.appveyor.com/docs/build-worker-api/#add-tests
かな?

他の人がメンテできるように、コメントまたはドキュメントを作成してほしいです。

@berryzplus
Copy link
Contributor

PRありがとうございます。

いつかやりたいと思いつつ手がつけられなかった案件です。是非やりたいです。

Appveyorでテスト実行を拾えてるようなので、大筋このまま入れていいように思ってます。後々の保守を考えて少し改善しておきたいとこを後でコメントします。

保守向けドキュメントは後回し…

@berryzplus
Copy link
Contributor

なんかGitHub
重い

@ds14050
Copy link
Contributor Author

ds14050 commented Oct 22, 2018

  • 意図がわかりにくいと思われる2点にコメントを付けました。
    • set NL のあとの空行
    • findstr を実行する for 命令
  • appveyor コマンドのドキュメントへの参照をコメントで付けました。
  • test_result_filter_tell_AppVeyor.bat ファイルについて、
    1. %~dp0 変数にパス区切りが含まれていることを考慮した上で
    2. 拡張子 .bat を含む形で
    3. TEST_RESULT_FILTER 変数として定義しました。
  • (指摘がなかった点ですが) testMsg 変数にメッセージを連結するときの改行を自然な配置にしました。

以上が変更点のすべてです。対応しなかった点については意見を表明していますので、再度コメントもしくは [Resolve conversation] をお願いします。

Copy link
Contributor Author

@ds14050 ds14050 left a comment

Choose a reason for hiding this comment

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

コミットを対象にしてコメントを付けるのに失敗しました。

tests/test_result_filter_tell_AppVeyor.bat Outdated Show resolved Hide resolved
tests/test_result_filter_tell_AppVeyor.bat Show resolved Hide resolved
tests/test_result_filter_tell_AppVeyor.bat Outdated Show resolved Hide resolved
@ds14050
Copy link
Contributor Author

ds14050 commented Oct 23, 2018

コミット 3a604f59657b0d の変更まとめです。

  • コメントのつもりで定義していた有効なラベル(2か所)を、2つ以上のコロンで始まる無効なラベルとして定義するように。
    • そのうちの片方のコメント内容について、NL(New Line) に空白を入れて NL (New Line) としました。
  • start コマンドを使う意図をコメントで説明するように。

持ち越された Unresolved conversation は以下。

  • パイプに渡すバッチコマンドに call をつけるべきか。
  • if errorlevel に代えて %ERRORLEVEL% を使うべきか。
  • フィルタエラーによりテストの実行と出力を中止すべきか。

そうすべきであると考えていないために対応していません。

@m-tmatma
Copy link
Member

m-tmatma commented Nov 1, 2018

* if errorlevel に代えて %ERRORLEVEL% を使うべきか。

#586 で if errorlevel に統一しました。

@m-tmatma
Copy link
Member

この PR のリビルドをかけた
https://ci.appveyor.com/project/sakuraeditor/sakura/builds/20196311

@ds14050
Copy link
Contributor Author

ds14050 commented Nov 10, 2018

berryzplus さんの指摘に対応し、変数名とスペースの対応を行いました。対応が必要な指摘点はもう残っていないはずです。AppVeyor の報告を待ちましょう。

@ds14050
Copy link
Contributor Author

ds14050 commented Nov 10, 2018

AppVeyor 完了しました。

  • Console タブに生ログが出ている。
  • Tests タブにテストのリストが出ている。
    失敗したテストがあれば赤でマークされリストの最上部に移動されます。
  • テストケースからのメッセージが表示できている。(例: test.PointerSize)
    例は StdOut だが ErrorMessage の場合もある。

@ds14050
Copy link
Contributor Author

ds14050 commented Nov 11, 2018

コメントはもらえても Approve がもらえず時間があるので最新の master に rebase しました。(あとデバッグのために消して戻し忘れていたであろう @echo off を戻しておきました)

MinGW ビルドが止まってしまうことの修正が master に入っているので、MinGW ビルドでのテスト結果がテストタブで確認できるようになったはずです。

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.

LGTMです。

AppVeyor 完了しました。

sakuraリポジトリのものはまだ完了してないですが、
コミットID 4824c89 についてのビルドが完了しているのを確認したのでOKと判断します。

これね↓
https://ci.appveyor.com/project/ds14050/sakura-clone/builds/20210967/job/ie0075hfemg1qg98/tests
https://ci.appveyor.com/project/ds14050/sakura-clone/builds/20210967/job/1it9lyokenqs0vcy/tests

Console タブに生ログが出ている。

Consoleタブに出すのはいずれやめたいと思っています。
何故かというと、テストの結果は今後増え続けていくはずで、
appveyorは2000行程度までしかログを表示できないからです。
(厳密には、表示できるけれどもやたら時間がかかるだけらしいですが。)

Tests タブにテストのリストが出ている。
失敗したテストがあれば赤でマークされリストの最上部に移動されます。

失敗テストの検証はまだですが、リストの出力を確認しました。

テストケースからのメッセージが表示できている。(例: test.PointerSize)
例は StdOut だが ErrorMessage の場合もある。

これについて、どう活用できるか落ちていませんが、出力は確認できました。

@m-tmatma さんの反応を待つかどうかは任せる 😄

@ds14050 ds14050 merged commit 4d5522c into sakura-editor:master Nov 11, 2018
@ds14050
Copy link
Contributor Author

ds14050 commented Nov 11, 2018

@m-tmatma さんの反応を待つかどうかは任せる 😄

無視するわけではなく、確認が必要な障害が残っていないと思ったので(※昨日そう書きました)、マージしました。

@ds14050
Copy link
Contributor Author

ds14050 commented Nov 12, 2018

Consoleタブに出すのはいずれやめたいと思っています。

その場合にはフィルタの出力をテキストファイルなり虚空なりへリダイレクトすることになると思います。%%i | "%FILTER_BAT%" > test_result.txt || set ERROR_RESULT=1

テストケースからのメッセージが表示できている。(例: test.PointerSize)
例は StdOut だが ErrorMessage の場合もある。

これについて、どう活用できるか落ちていませんが、出力は確認できました。

典型的な ErrorMessage は失敗したテストの期待値と実際の値です。

@@ -12,8 +14,8 @@ for /r %%i in (tests*.exe) do (
@echo %%i --gtest_list_tests
%%i --gtest_list_tests || set ERROR_RESULT=1

@echo %%i
%%i || set ERROR_RESULT=1
@echo %%i | "%FILTER_BAT%"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

パイプをエスケープしていないからエコーメッセージがフィルタに流れていた。影響はパイプ以降のメッセージが表示されないことだけ。

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.

5 participants