[WIP] vs2017ソリューションにテストを組み込みたい #796
Conversation
|
PR の説明欄に説明が欲しいです。 |
|
テストが実施された数が凄く減っています。 |
|
PR のタイトルに closes #793; と入っていて |
|
↑ 自動入力じゃないかも。コミットメッセージにそんなコメントなさそう。 |
|
PR に closes #nnn などと書いておけば、そのPRをマージすると同時に Issue #nnn が自動クローズされて便利ですが、タイトルよりは本文に書いておいた方がいい気もします。 |
|
↑ PR に書いてもいいんですか？ それなら言われる通り、タイトルではなく本文に書いて欲しいです。 |
#792(メールアドレス判定対応をrevertして暫定対処を入れる)をマージした結果です。 |
|
MinGW には StdControl.Wnd_GetText のテストがあるのに VC にはないのはなぜですか? |
すんません。入れ間違った、というか、作り始めた時点で存在してなかったので抜けていました。 |
berryzplus
changed the title
|
|
スルーはできませんでした。:smile: |
どのあたりを説明したらいいか分からんかったので、大前提部分「MsBuildのプロジェクトファイルは手で編集してもいい」を説明したブログのリンクを貼ってみます。 |
あとで更新のコミットを積んどきます。 MinGW側のgoogletest取得をpacman利用に変えたいです。
編集しやすいように・・・yesです。
手順はコミットコメントに書いています。
googletest本体から削られるまでは定義が必要です。
3.のコメント通りにやるとウィザードで「テスト対象を選べ」と訊かれます。 visual studioの「予期しないエラー」については、vs2019とかでVC++がPackageReferenceに対応したら直るかも知れません。（直らないかも知れません。 この挙動は認識していて「基本的に手作業でメンテする」とコミットコメントに書いてます。
当然、手動です。visual studioのGUIでは確認できません。 公式のマニュアル（日本誤訳）を参考に手書きしました。 MsBuildプロジェクトファイルに書かれたターゲット定義を表示できるGUIプラグインがあるかどうかは知らんです。 |
#799 に登録しました。 |
tests/create-project.bat
Outdated
| @@ -7,6 +7,10 @@ if "%platform%" == "MinGW" ( | |||
| set BUILD_EDITOR_BAT=build-gnu.bat | |||
| ) else ( | |||
| set BUILD_EDITOR_BAT=build-sln.bat | |||
|
|
|||
There was a problem hiding this comment.
tests/create-project.bat や tests/build-project.bat などは変更せずに
appveyor.yml 側で VC 構成だと tests\build-and-test.bat を呼ばないなどの
対応をしたほうがいいと思います。
ここで条件分岐するのは見通しが悪いと思います。
また VC の cmake のコードをテストしにくくなります。
There was a problem hiding this comment.
tests/create-project.bat や tests/build-project.bat などは変更せずに
appveyor.yml 側で VC 構成だと tests\build-and-test.bat を呼ばないなどの
対応をしたほうがいいと思います。ここで条件分岐するのは見通しが悪いと思います。
これは指摘（＝修正依頼）ですか？それとも感想ですか？
修正依頼だとすると、現時点では難しいと思っているので対応を見送りたいです。
感想なら同意で、今後見直していく必要があると思っています。
これを1行にまとめてbuild.batを作りたい・・・
Lines 20 to 24 in a722f9c
また VC の cmake のコードをテストしにくくなります。
VC向けCMakeのコードは、ローカルビルド用のNinja Generatorにしたいと思っています。
sln + vcxproj構成とのダブルメンテになりますが、細かいところはしょうがないっす。
何を言ってるかというと、VC向けCMakeのコードはたぶんバッチ向けにはしない、ってことです。
（というか、MinGW向けCMakeコードをMSVCのIDEからビルド＆テストできます。バッチなしで。）
There was a problem hiding this comment.
何が難しいですか？
今回修正した部分を取り消して、
appeyor.yml からのよびだしをかえたらいいだけのように思いますが。
There was a problem hiding this comment.
何が難しいですか？
今回修正した部分を取り消して、
appeyor.yml からのよびだしをかえたらいいだけのように思いますが。
There was a problem hiding this comment.
何が難しいですか？
まず、appveyor.ymlに条件分岐を書く必要があります。
appveyor.ymlのcmd文法はやや特殊なので、書いたものが動く保証はありません。
それをこれから実験して詰めるのが難しいです。
必要な対処だと思いますが、いまやるべきじゃない、と言ってます。
There was a problem hiding this comment.
まず、appveyor.ymlに条件分岐を書く必要があります。
ローカルで VC に対する cmake によるテストをこれまで通り使用できる
状態にしておくことが目的です。
appveyor.yml に直接書くのが難しければ、
tests\build-and-test.bat をラップする バッチファイルを作って
appveyor.yml から呼ぶのでもいいし、
バッチファイルが増えるのが嫌ならtests\build-and-test.bat の中で
分岐してもいいと思います。
方法は一つではないです。
There was a problem hiding this comment.
何を懸念してるのか分かってきました。
tests/create-project.bat や tests/build-project.bat
このPRではVC向けのgoogletestビルド機能を無効化しているわけですが、それだとマズくね？ってことですよね？確かにマズいかも知んないです。
バッチファイルが増えるのが嫌ならtests\build-and-test.bat の中で
分岐してもいいと思います。
増えるの嫌です（笑）・・・なので、この方針で修正コミット積みます。
There was a problem hiding this comment.
まず、appveyor.ymlに条件分岐を書く必要があります。
ローカルで VC に対する cmake によるテストをこれまで通り使用できる
状態にしておくことが目的です。appveyor.yml に直接書くのが難しければ、
tests\build-and-test.bat をラップする バッチファイルを作って
appveyor.yml から呼ぶのでもいいし、
新しいバッチファイル追加して対応してはいかがですか?
それなら、既存のファイルを触らなくていいので簡単だと思います。
必要なら後でリファクタリングすればいいと思います。
|
と、とらびす！（いつの間に！乙です！ 当然ビルドは通らんわけですな。たぶんMinGW向けクロスコンパイル環境を用意してそれ用のtravis.yml的なものを用意しないといけない気がしています。 |
unittest.md
Outdated
| プロジェクトフォルダに packages.config を置いておくと、ソリューションフォルダに packages というフォルダが掘られてパッケージがダウンロードされる仕組みです。たまにsolutionを開いてすぐにダウンロードされないことがありますが、そういう時は「ソリューションエクスプローラー」でソリューションを右クリックし、「NuGetパッケージの復元」をします。エクスプローラーでソリューションフォルダを見て見るとpackagesフォルダが掘られているはずです。 | ||
|
|
||
| packages.config を使う方法は、VC++形式のプロジェクトだとバッチ実行ができません。 | ||
| サクラエディタはCI環境(appveyor)を利用しているので、バッチ実行でビルドできる方法が必要でした。長いので端折りますが、興味のある方は [PR 796](https://github.com/sakura-editor/sakura/pull/796/commits)のコミットメッセージを見てください。 |
There was a problem hiding this comment.
コミットメッセージを見てというのは不親切だし
情報の変更や追加があるときに、めんどくさいので更新されない可能性が極めて高いです。
もし、コミットメッセージに書いてあるのなら、転記すればいいだけだし、もし転記するだけで足りないのなら、それはきちんとまとまっていないと言うことなので、整理してまとめて欲しいです。
berryzplus
force-pushed
the
feature/add_test_project_for_msvc
branch
from
d74cb29 to
257c9e7
Compare
|
ごちゃごちゃしてきたのでコミットを整理してforce pushしました。 |
|
ひさびさに容量枯渇が発生するとか… https://ci.appveyor.com/project/berryzplus/sakura/builds/23045473 |
|
ローカルでビルドすると以下のエラーになりました。 |
|
あと、msbuild でがんばって対応いただいたのにあれなんですが、 msbuild.exe を使ったほうが簡単かと思ったんですが、 |
これはデフォルトで入らないんですね...orz visual studio installer で |
当然、コマンドライン版nugetを同梱するのが一番簡単な方法です。 一応、この件よりも sakura-editor/management-forum#52 (comment) の話のほうが優先度高いと思っとります。 |
|
ちなみに nuget.exe の再配布ライセンスに関しては、どこにも明記されてない気がしています。 必要なインストールオプションを追加する0件はそのうちコミット積むので、取り込み方式はこのままいきたい感じです。（ああ、だからアッチが優先っすｗ |
う～む。 役割は |
|
折衷案を作ってみました。 これでダメならバッチには手を加えない方針でいくしかないかなぁ。 |
|
これと折衷できてますか？> #796 (comment) |
うーむ。詰んでますね。 |
NuGet復元で失敗したとき、そこでビルドが止まるように変更。
先の修正ではブラウザでリンクに飛べないそうなので再修正。 空白をハイフンで置換すれば飛べるらしい。 なお、VS Codeプラグイン[Markdown All in One]のプレビューでは%20でも飛べる。 表示確認はブラウザで行うようにしたほうが良いですね・・・。
build-sln.batの変更は外せないんだった...orz |
berryzplus
force-pushed
the
feature/add_test_project_for_msvc
branch
from
359920b to
188250e
Compare
|
テストのビルドバッチに加えた変更を全面的にrevertしました。 このPRの目的はソリューションにテストプロジェクトを組み込むことです。 組み込むだけでもバッチビルド(build-sln.bat)に影響がでるのでバッチの対応を行ってきましたが、バッチからのテスト環境を再構築する意図は元々ありませんでした。 「テストプロジェクトをソリューションに組み込みたい」を是とするか非とするか、それだけを判断してもらいたいのでノイズになりそうな変更は外します。 |
|
方針の回帰には賛成です。build-sln.bat に手を入れる目的に疑問です。
これまでの build-sln.bat の目的は「エディタをビルドすること」でした。それがこの PR によるソリューションの変更によって「エディタとテストをビルドすること」に変化しました。 20190421.19 sakura-editor.sakura によると、Build solution と Unit test の両方でテストプログラムを作成しています。 やり方としては、msbuild に引数を追加して従来の目的を維持する(=エディタだけをビルドする)ことも考えられます。それでも AppVeyor のビルドは維持されます。Visual Studio でテストプロジェクトをビルドすること(berryzplus さんの目的)もやはり可能なのではありませんか？ build-sln.bat を修正する berryzplus さんの目的は何ですか？ |
テストプロジェクトで必要なNuGetパッケージを復元させるためです。 build-sln.batはもともとエディタ以外のものもビルドしています。 build-sln.batでビルド対象プロジェクトを指定するやり方は可能なので、必要なら別途PR出せばよいと思います。 要望によりbuild-project.batを変更できないので、ビルド構成には関与しない方針です。 |
テストプログラムをビルドしないように build-sln.bat で msbuild に引数を追加しても、「変更しないとビルドが通らない」ままですか？ テストに関して暫定的にでも二重メンテナンス(ソリューションと CMakeLists.txt によるテストプログラム作成)を選択したのだから、バッチビルドをこの PR の方に対応させる必然性はなくなりました。「バッチは cmake」で問題ありません。
しかしテストプログラムは含まれませんでした。それを維持するということです。 |
他人事ですね。 berryzplus さんが「20190421.19 sakura-editor.sakura によると、Build solution と Unit test の両方でテストプログラムを作成しています。」をどう考えるかということです。ソリューションへテストプロジェクトを追加したことによる build-sln.bat の処理コスト増加を他人にどう正当化するのかということです。 |
サクラエディタを構成する技術分野は多岐に渡ります。全部を一人でやるのは大変なので、ある程度分担していきたいです。他人事なんじゃなくて、ぼくはメインでやりませんと言ってるだけです。バッチ分野はサポートに回りますんで好きにしてという話。 build-sln.batでテストをビルドさせない対応は後でコミット積んでおきます。 |
お願いします。求めているのはこれだけです。これはソリューションへテストプロジェクトを追加するこの PR の副作用で、誰が望んでいるのかわからないものでした。 |
| @@ -19,6 +19,27 @@ if "%configuration%" == "Release" ( | |||
| call :showhelp %0 | |||
| exit /b 1 | |||
| ) | |||
|
|
|||
| @rem find a NuGet CLI. | |||
| if not defined CMD_NUGET ( | |||
There was a problem hiding this comment.
この環境変数は他の変数と同様に find-tools.bat で定義するのが
いいと思います。
There was a problem hiding this comment.
まだ帰れんのですが、ソリューションに入ったテストプロジェクトをバッチからビルドする機能はスコープ外になりますので、この部分の記述も消える予定です。
大きなとこではNuGet.exeとドキュメント修正も消える予定です。
build-sln.bat でテストをビルドせずに cmake を使うのであれば |
|
作業時間がとれないので本件はしばらく凍結します。 このPRの変更は、vsソリューションにテストプロジェクトを組み込むことです。 テストのビルドに関連するタスクは他にもたくさんあります。
今回、当初の目的から外れ色々なものを混ぜてしまっていました。 このPRが提案することはとても小さいですが、 GWになれば戻ってこれると思います。 |
|
順番にやるのでいいと思います。明確な方針やゴールを定めないまま、目的を説明できない(※これをキメラと表現しました)バッチファイルとその群れを書き散らすのは悪夢です。 |
|
#999 を作成したので閉じます。 |
closes #793; vs2017ソリューションにテストを組み込みたい
vs2017ソリューションにテストプロジェクトを組み込みます。
変更内容の詳細については、各コミットのコミットメッセージを参照してください。
テストを実行してソースコードを修正した場合に、tests1.exeをリビルドしないと内容が反映されないビミョーな状態になっていたので、リンク処理を記述する場所を変えました。ついででクリーン周りに手が入っていて「ん？」と思う人も多いかと思いますが、この辺はややこしいのでなるべくスルーがいいです。（必要なら別issueでお願いします。）
一番新しく追加されたテストファイルを含めるのを忘れていたので、追加コミットを積みました。
#856 を経て、サクラエディタ本体の中間ファイルを取り込む方法がスマートになりました。
MsBuildのタスクは「タスク名」のアイテムノードで処理対象を指定できるので、
Linkタスクの対象にサクラエディタ本体のobjを指定しています。#856 で入れようとしていたクリーン時タスクの枠組みは別件とします。
GoogleTestのNugetパッケージのバージョンは、バージョン更新の方法を示す目的もかねて最新版としています。googletestの更新は、このPRでは見送りにします。 #796 (comment)