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 の結果を appveyor に反映する際 XML 出力を使う #735

Closed

Conversation

m-tmatma
Copy link
Member

googletest の結果を appveyor に反映する際 XML 出力を使う

#580 の内容を完全に置き換えてしまうので心苦しいですが、
自前でやらなくていいし、明示的にドキュメント化 されているので たぶん appveyor の推奨だと思います。

仕組み

  • googletest の引数に --gtest_output=xml:filename.xml を渡すことにより XML でもテスト結果を出力する
  • https://ci.appveyor.com/api/testresults/junit/{jobId} の URL に生成された XML ファイルをアップロードする

意図的にビルドに失敗させたもの

m-tmatma@b3120e0 の修正を行って意図的にビルドを失敗させた場合でもテストを認識できています。
https://ci.appveyor.com/project/m-tmatma/sakura/builds/21288286

参考サイト

@m-tmatma m-tmatma added this to the next release milestone Dec 30, 2018
@m-tmatma m-tmatma added CI appveyor など CI 関連 【ChangeLog除外】 UnitTest labels Dec 30, 2018
@ds14050
Copy link
Contributor

ds14050 commented Dec 30, 2018

これをもって反対意見とするわけではありませんが、XML 出力を選ばなかったのは「Tell AppVeyor test results one-by-one.」というコミットメッセージが示すように、テスト実行中に随時結果を反映したかったからです。

@ds14050
Copy link
Contributor

ds14050 commented Dec 30, 2018

自前でやらなくていいし、明示的にドキュメント化 されているので たぶん appveyor の推奨だと思います。

不正確な言及です。Build Worker API を使った方法は XML をアップロードする方法より先に紹介されています。

@m-tmatma
Copy link
Member Author

自前でやらなくていいし、明示的にドキュメント化 されているので

こっちのコメントが重要です。
Build Worker API の方法は確かに見たのでたしかに推奨しているとは言えないです。

ただわかりやすさやメンテしやすさでは xml 型式のほうが優れていると思います。
また googletest のテストに長い時間がかかるわけではないので、途中でテスト結果を
表示できなくても問題はないんじゃないかと思います。

@berryzplus
Copy link
Contributor

https://ci.appveyor.com/api/testresults/junit/{jobId} の URL に生成された XML ファイルをアップロードする

googletestって junit 互換な結果xml出せたんですよね。
結果xmlを使った分析ツールがなんか色々あった気がします。
サクラエディタのプロジェクト的に、その辺のツールを活用する方向に進むならこれもありと思います。

逐次報告方式とxml一括出力形式で、どちらがいいかの判断は単純にはつけられないです。

@ds14050
Copy link
Contributor

ds14050 commented Dec 31, 2018

サンプルなのか誰かが書いたのかは知りませんが、test.PointerSize の出力が拾えなくなっています。

googletest のテストに長い時間がかかるわけではないので、途中でテスト結果を表示できなくても問題はないんじゃないかと思います。

テストは増え続けます。そしてあえて言うのも馬鹿馬鹿しいことですが、そうは思わないから手間をかけました。

@ds14050
Copy link
Contributor

ds14050 commented Dec 31, 2018

googletestって junit 互換な結果xml出せたんですよね。
結果xmlを使った分析ツールがなんか色々あった気がします。
サクラエディタのプロジェクト的に、その辺のツールを活用する方向に進むならこれもありと思います。

--gtest_output=xml オプションは標準出力に加えて XML ファイルを出力するオプションのようです。現状を維持したまま XML ファイルの活用が可能です。

@m-tmatma
Copy link
Member Author

サンプルなのか誰かが書いたのかは知りませんが、test.PointerSize の出力が拾えなくなっています。

どのログですか? ちゃんと出力されているように見えます。
https://ci.appveyor.com/project/sakuraeditor/sakura/builds/21288789/job/6pwod8imt3k56y4e?fullLog=true

テストは増え続けます。そしてあえて言うのも馬鹿馬鹿しいことですが、そうは思わないから手間をかけました。

どれぐらい時間がかかるテストを想定していますか?

テストの完了を待てないほど時間がかかる状況を想像できないのですが
どのような状況を想定していますか?

またテスト数が膨らんできて時間がかかるようになっても実行ファイルを分割すればいいと思います。

@ds14050
Copy link
Contributor

ds14050 commented Dec 31, 2018

どのログですか? ちゃんと出力されているように見えます。

Tests タブです。

テストの完了を待てないほど時間がかかる状況を想像できないのですが
どのような状況を想定していますか?

現状でも cppcheck の実行中にログ出力が止まるのをじりじりと待っている状態です。

またテスト数が膨らんできて時間がかかるようになっても実行ファイルを分割すればいいと思います。

なぜこの PR に固執して追加対策を弄するのですか。

@m-tmatma
Copy link
Member Author

どのログですか? ちゃんと出力されているように見えます。

Tests タブです。

確かにこの PR の方法では反映できていないですね。

テストの完了を待てないほど時間がかかる状況を想像できないのですが
どのような状況を想定していますか?

現状でも cppcheck の実行中にログ出力が止まるのをじりじりと待っている状態です。

cppcheck の件と単体テストの件は別問題だと思います。

またテスト数が膨らんできて時間がかかるようになっても実行ファイルを分割すればいいと思います。

なぜこの PR に固執して追加対策を弄するのですか。

苦労して対応された PR を置き換える PR を投げて腹を立てる気持ちは理解できます。

ただ現状の方法はバッチファイルを駆使して作成しているので複雑なので
なかなか理解しづらいものになっています。

これをもっと誰でも理解しやすくてメンテしやすいようにするリファクタリングの提案です。

現状と比較して、リファクタリング後の方が良ければリファクタリング案を採用しましょうとなりますし
現状の方がメリットが多ければ現状を維持したほうがいうことになる、ただそれだけのことです。

※ 完全に等価ではないのでリファクタリングではない、と突っ込まれそうですが。

@ds14050
Copy link
Contributor

ds14050 commented Dec 31, 2018

ただ現状の方法はバッチファイルを駆使して作成しているので複雑なので
なかなか理解しづらいものになっています。

これをもっと誰でも理解しやすくてメンテしやすいようにするリファクタリングの提案です。

このあたりが考えに溝がある部分ですね。

m-tmatma さんはドキュメントの整備にも熱心だから行動には一貫性があるといえるでしょう。

しかし自分はそこに価値を見出さないから、自分にとっての価値(随時反映)を無碍にされ、そのための過去の選択(xml ではなく標準出力を利用すること)を反古にされたとしか受け取れない。足を引っぱられているとしか思えない。

理解できない人間に理解してもらうつもりはないんです。置いていくだけです。嫌なら必死で付いていくしかないが、すべての分野を理解して付いていく必要もありません。得意分野があるでしょう。ドキュメントを読んだだけコメントを読んだだけで理解したつもりの半可通に口出しされても迷惑なだけで、自ら調べ学ぶつもりのない人間のための解説講義にも価値を置きません。指針や要点、落とし穴など当事者のみが知る理解の助けは有用だとしても、イチから理解してもらうための講義を開くつもりもそうする動機もありません。


ちょっと逸れますが、ファイルを散らかしておいてドキュメントを読めばわかる、というのはベストな状態ではありません。ドキュメントを読まなければわからないと言っているのに等しい。しかもファイルの海に溺れるのみならずドキュメントの海に溺れることにもなる。

ファイルを分割しても複雑さは減りません。むしろ煩雑さが増します。バッチファイルに関して言えばファイルを分割することでほとんどのバッチファイルが共通コードのコピーを持つことになり、非本質部分が水増しされる結果になっています。

このあたりは理解しやすさに対する考え方の違いです。無駄は理解を妨げます。

@KENCHjp
Copy link
Member

KENCHjp commented Dec 31, 2018

置いていくだけです。

それだと将棋の千日手とか囲碁のコウ状態になってしまうので、

このあたりが考えに溝がある部分ですね。

こういう意図の積み重ねを歓迎します。

@m-tmatma
Copy link
Member Author

ファイルを分割しても複雑さは減りません。

ファイルの分割に関してはテストの実行ファイルを分割する話です。
バッチファイルの分割ではありません。

ここでは複雑さを減らすことではなく実行がすべて完了するまで
Tests タブで確認できないことに対するコメントです。

また、テストの実行ファイルを分割することに関しては設計段階から
想定していて単体テストのバイナリ名が tests1.exe という名前と
なっているのも tests2.exe , tests3.exe という実行ファイルが
増えてくることを考慮してのことです。

そのことは run-tests.bat で for ループでテストの実行ファイルを
検索する構造に反映されています。

設計思想もドキュメントに記載したほうがいいのかもしれません。

ここの議論とは外れますが、

一般論としてはファイルの分割をすることにより、一度に理解する必要のある量を
減らせるので複雑さ自体はそんなに減らないかもしれませんが、理解しやすさは
格段にあがります。もちろんやり方次第ですが。

理解できない人間に理解してもらうつもりはないんです。

この方針だと初心者が近づきたくないということに繋がるのだと思います。

初めての人が見たときに簡単に構造を理解できて問題があれば修正できる
というのを目指しています。そのコードを書いた人しかメンテできないという
状況を回避したいと思っています。

足を引っぱられているとしか思えない。

そのような考えだと、 @ds14050 さんの書いたコードは誰も変更できなくなってしまいます。

過去に行った方法と違う方法でもっといい方法があると思えば提案すればいいし、
前の方法の方がやっぱりよかったねということになれば変えなくてもいいと思います。

サンプルなのか誰かが書いたのかは知りませんが、test.PointerSize の出力が拾えなくなっています。

ちなみこれは私が書いたものです。

@ds14050
Copy link
Contributor

ds14050 commented Jan 1, 2019

置いていくだけです。

それだと将棋の千日手とか囲碁のコウ状態になってしまうので、

このあたりが考えに溝がある部分ですね。

こういう意図の積み重ねを歓迎します。

「(理解できないものを)置いていくだけ」という部分に認識の齟齬があると思いました。

「理解」というのはコードに書かれている内容に関することであって、コードそのものに対する理解です。m-tmatma さんが他人の行動にキャップをはめる理由にしようとしている部分のことです。コードに対する理解が足りない(そのつもりもなさそうだ)のを他人のせいにして他人の行動を制限する理由にするなど馬鹿げていると思うんです。

自分にとって不要だからと他人が必要とした処理を削除するなど言語道断の振る舞いだと誇張なしに感じています。m-tmatma さんがあちらこちらにばらまいたファイルだって目障りだと削除したりはしません。よりよい解決策でシンプルに置き換えられる場合に初めて PR として成立するのです。

自分の行いが時に賛同とは別の理由によって尊重されていることを知り、他人を尊重することを覚えるべきだと忠告したい。

@m-tmatma
Copy link
Member Author

m-tmatma commented Jan 1, 2019

これをもって反対意見とするわけではありませんが、
XML 出力を選ばなかったのは「Tell AppVeyor test results one-by-one.」
というコミットメッセージが示すように、テスト実行中に随時結果を反映したかったからです。

これが反対の理由だと思っています。

「テスト実行中に随時結果を反映」をしたいためにこの処理を実装した、
でもこの PR はその修正を削除するもので自分の苦労を無効にするものだから反対した。

だったら最初に本音を言えばいいんだと思います。
本当の反対の理由を隠そうとするからややこしくなるのだと思います。

自分にとって不要だからと他人が必要とした処理を削除するなど言語道断の振る舞いだと誇張なしに感じています。

随時結果 が必要な理由を教えてただけますか?

m-tmatma さんがあちらこちらにばらまいたファイルだって目障りだと削除したりはしません。

もしファイルが増えることに反対であるのなら、該当の PR に対して反対意見を書けばいいだけの話です。

あるいはトップディレクトリにあるファイルが多くなってきたので整理しませんか? というチケットを
作るなりして問題提起して、必要なら PR を投げればいいだけの話です。

よりよい解決策でシンプルに置き換えられる場合に初めて PR として成立するのです。

xml 出力をすることでシンプルな解決策になっていると思っています。
googletest の実行時間は現在 1 秒もかかってません。

つまり 1秒も待てないことがあるか? ということから、随時結果を反映する
必要はないんじゃないかと思ってこの PR を投げています。

#580 に関しては私もレビューしたので、この PR で 随時結果を反映できることは
理解していますし、標準出力のログを解析して、 appveyor コマンドを使って
結果を反映しているのは理解しています。

@ds14050
Copy link
Contributor

ds14050 commented Jan 1, 2019

これをもって反対意見とするわけではありませんが、
XML 出力を選ばなかったのは「Tell AppVeyor test results one-by-one.」
というコミットメッセージが示すように、テスト実行中に随時結果を反映したかったからです。

これが反対の理由だと思っています。

それを書いた時点ではこの PR の全容を把握していませんでした。なので気がついていない場合を想定して PR とは関係なく自分の持っている知識を書きました。内容から反対意見に取られると思ったので、まだニュートラルだという意思表示をしました。

その後わかったのは、この PR が(自分にとっての)新しい価値をもたらすものではなく、既存の価値を毀損するだけのものだということです。

@berryzplus さんが XML ファイルのさらなる活用を示唆して新しい価値を模索しましたが、それはこの PR とは無関係に可能だということがわかりました。

もはやはっきりしましたが自分が反対するのは、m-tmatma さんが他者にとっての価値を認めず、自分にとっての価値だけを大事にしているということが理由です。

@m-tmatma
Copy link
Member Author

m-tmatma commented Jan 1, 2019

その後わかったのは、この PR が(自分にとっての)新しい価値をもたらすものではなく、既存の価値を毀損するだけのものだということです。

もはやはっきりしましたが自分が反対するのは、m-tmatma さんが他者にとっての価値を認めず、自分にとっての価値だけを大事にしているということが理由です。

既存の価値 というのを教えていただけますか?
それを理解するために質問をしているにそれに回答いただけないので、理解しようがないです。

@m-tmatma
Copy link
Member Author

m-tmatma commented Jan 1, 2019

それを書いた時点ではこの PR の全容を把握していませんでした。なので気がついていない場合を想定して PR とは関係なく自分の持っている知識を書きました。内容から反対意見に取られると思ったので、まだニュートラルだという意思表示をしました。

別に反対意見を言っていただくのは全然問題ないですし、
よく理解できていない点があれば質問していただいたらいいと思います。

いろいろ議論した上でどの方法がいいか考えて、現状の方法がいいのか? あるいは
新しい方法がいいのか、あるいは全く想定していなかった新しい方法にするのか
考えればいいことです。

重要なのは何を重視するのかの意見を議論の過程の中で検討するかということだと思っています。

@ds14050
Copy link
Contributor

ds14050 commented Jan 1, 2019

価値を付け加えるなら判断に迷うことはありません。共通の価値が見出せるなら天秤にかけられます。引いて足して自分色に塗り直すだけなら失礼な話です。

いろいろ議論した上でどの方法がいいか考えて、現状の方法がいいのか? あるいは新しい方法がいいのか、あるいは全く想定していなかった新しい方法にするのか考えればいいことです。

重要なのは何を重視するのかの意見を議論の過程の中で検討するかということだと思っています。

これは自分に欠けているかもしれません(だから短絡的に「失礼な話」と受け止めてしまう)。自分と他人をアンタッチャブルなものにして合意とそれに基づく服従を諦めています。

@m-tmatma
Copy link
Member Author

m-tmatma commented Jan 1, 2019

価値を付け加えるなら判断に迷うことはありません。

私の考えているこの PR の価値は、わかりやすさです。

動作ロジックは説明欄の 仕組み に記載していますが、シンプルです。
xml 出力して、それを appveyor にアップするというものです。

jnit という形式で xml が出力されるので多分 gooogletest がメジャーバージョンアップ
しても動きつづけてくれるだろうという希望もあります。

ただ、現状では標準出力への出力を appveyor に反映できないという欠点もあります。
だから現状でなんからの解決策が見つからない限り、適用は難しいとは思っています。

@ds14050 さんの考える価値とは、「テスト実行中に随時結果を反映」だと思います。

しかし、私は googletest の実行時間はそれほどかからないので、
「テスト実行中に随時結果を反映」する必要はないと思ったので、
xml 出力してappveyor に上げる PR を作成しました。

ところが、 @ds14050 さんは、「テスト実行中に随時結果を反映」が重要と考えているが
ロジックをシンプルにすることに対して、「テスト実行中に随時結果を反映」よりも重要視
していないということになるかと思います。

この PR の背景にある目的の認識に相違があるのだと思います。

それで、なぜ「テスト実行中に随時結果を反映」を重視するのか理由を知りたいので
質問しました。

@m-tmatma
Copy link
Member Author

m-tmatma commented Jan 1, 2019

Agree to disagree という言葉があります。
意見が一致しない事自体は悪いことではないです。

@KENCHjp
Copy link
Member

KENCHjp commented Jan 1, 2019

意見が一致しない事自体は悪いことではないです。

はい、「どのように」に対する「なぜ」のやり取りを積み重ね、唯一無二の解決方法や同意点が見いだせないところで、落としどころを模索しているのであれば、時間はかかってしまいますが、短絡的に妥協するよりも有意義だと思います。

お互いにどの点について問題や齟齬を感じその問題が誤解でなければそこから積み上げていけてればいいのですが、文字だけのやり取りなので殺伐としがちなにが否めないかと。

すいませんPR汚しだけの書き込みで。

@ds14050
Copy link
Contributor

ds14050 commented Jan 2, 2019

なぜ「テスト実行中に随時結果を反映」を重視するのか

それがなければ Console ログを見る代わりとして Tests タブが利用できない理由になるからです。Console ログに対して不足があるなら Tests タブそのものの利用価値が問われます。仕組み自体が不要だという判断に傾きます。

この PR に価値を見出せないのはなぜか

すでに存在し、現在も稼働しているシステムを(不完全に)置き換えるのに「わかりやすさ」は不十分だと考えますし、そもそも「わかりやすさ」を理由にして行動を縛られるのが苦痛です。

わからない人が内部の仕組みを理解する必要はありません。test_result_filter_tell_AppVeyor.bat という名前を読んで、「ああ、テスト結果を AppVeyor に通知するんだな」と理解するにとどめておけばいいのです。そのためにファイルを分けて名前を付けています。

そしてすでに書きましたが、シンプルさ、わかりやすさという点において m-tmatma さんとは見解の相違があります。

ファイルが増えるということは自分にとって、シンプルさにもわかりやすさにも反することです。ファイルが増えれば全体像がつかみにくくなりますし、目的のファイルを探し当てることが難しくなります。ドキュメントがあっても同じです。ドキュメントを漁るなら同じ手間でファイルを漁った方が目標に確実に近づいています。

すいませんPR汚しだけの書き込みで。

KENCHjp さんはこのプロジェクトの良心であり、「きれいなリーナス・トーバルズ」なのですから、どーんと構えていてください。

@ds14050
Copy link
Contributor

ds14050 commented Jan 2, 2019

jnit という形式で xml が出力されるので多分 gooogletest がメジャーバージョンアップしても動きつづけてくれるだろうという希望もあります。

見落としていました。これは標準出力を利用することに対する明確な優位点です。バッチやスクリプトなんて実行可能な設定ファイルくらいな位置づけなので、頻頻に変更を迫られてから考えてもいいと思いますが。

@berryzplus
Copy link
Contributor

自分は逐次報告の方がいいんじゃないかと思っています。
明確な理由はありません。ただ「なんとなく」です。

2つのやり方を排他関係で考える理由が分からなくなってきました。
http://opencv.jp/googletestdocs/advancedguide.html#adv-generating-an-xml-report

Google Test では,通常のテキスト出力に加えて,XML 形式のレポートをファイルに出力できます.

できるなら「いいとこ取り」したいです 😄

@ds14050
Copy link
Contributor

ds14050 commented Jan 2, 2019

#735 (comment)@berryzplus さんに向けて書きました。補足すると「現状」というのはこの PR を抜きにした現状です。

できるなら「いいとこ取り」したいです

残念ながら test_result_filter_tell_AppVeyor.bat を使う方法は「いいところ」がなかったようですよ。

@KageShiron
Copy link
Member

CIの活用方法は人それぞれなので、アクティブに活動できている方が活用しているものは残した方が良いと思います。逆に、実は誰も活用していない出力とかがあれば消していきたい。

#702 でCIのdoxygenを削除したら?と提案してますが、CIでのdoxygen結果を活用されている方がいれば残すべきだと思ってます。


個人的には、バッチファイルもドキュメントも数が多いと新しい人がわかりにくいので、減らす方向でPRを投げてます。

バッチファイルは他のバッチファイルを書き換えた影響が波及しそうで怖いというのもありますね 😓

@m-tmatma
Copy link
Member Author

m-tmatma commented Jan 2, 2019

Google Test では,通常のテキスト出力に加えて,XML 形式のレポートをファイルに出力できます.

できるなら「いいとこ取り」したいです 😄

はい、XML 形式のレポートと通常のテキストは同時に出力できます。
この PR でもログ上で通常のテキスト出力を行っています。

しかし、どちらの出力を appveyor の Tests 欄の入力に使うかに関しては仕組み上排他です。
両方を使うというのは意味がないです。テキスト出力を解析するのか、
あるいはXML 出力を使うのかしかないです。

@m-tmatma m-tmatma removed this from the next release milestone Jan 2, 2019
@m-tmatma
Copy link
Member Author

m-tmatma commented Jan 2, 2019

なぜ「テスト実行中に随時結果を反映」を重視するのか

それがなければ Console ログを見る代わりとして Tests タブが利用できない理由になるからです。Console ログに対して不足があるなら Tests タブそのものの利用価値が問われます。仕組み自体が不要だという判断に傾きます。

この PR を投げた時点では、気づきませんでしたが、
標準出力を Tests タブに反映できてないという問題があるのは事実です。

RecordProperty を呼ぶことで XML の出力に指定した値の結果を含めることは可能みたいですが
appveyor の Tests タブには反映はしてくれないようです。

標準出力への結果を xml 経由で appveyor に反映させる方法があるのかもしれませんが、
なかなか難しいものがあるのかもしれません。

それを実現するために単体テストを大きく変更する必要があるとすれば、
シンプルにしたいという目的とは反するので、この PR は閉じます。

@m-tmatma m-tmatma closed this Jan 2, 2019
@KageShiron
Copy link
Member

考えている最中にcloseとなってので余談ですが、最近のWindowsやAppVeyorにはcurl.exeがあるのでxmlのアップではそちらが使えそうです。

https://www.appveyor.com/docs/how-to/download-file/#curl

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