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

E2Eテスト(Playwright)の導入 #863

Merged
merged 20 commits into from
Jul 20, 2022
Merged

E2Eテスト(Playwright)の導入 #863

merged 20 commits into from
Jul 20, 2022

Conversation

so-c
Copy link

@so-c so-c commented Jul 18, 2022

内容

E2EテストツールとしてPlaywrightを導入します(VOICEVOXはVue+Electron構成なのでAutomated Testing | Electronから選択)

npm run test:e2e ではなくnpx playwright testで実行します

主な内容は下記2点で環境整理がメインです。

  • Playwrightによる起動~初期表示の確認テストとその実行コマンド
  • Cypressへの依存を削除

関連 Issue

ref #182
ref so-c#8

スクリーンショット・動画など

なし

その他

手元でelectron:serveでの起動やnpx playwright testが動くところまでは確認したのですが、
electron:build, github actionsの環境までは準備できませんでした。

PR前のローカルビルドで気が付いたのですが、
ビルドに関してはエンジンの参照方法が.envではなくて下記から
VOICEVOX_engine を参照しているように見えます
https://github.com/VOICEVOX/voicevox/blob/main/vue.config.js#L6

so-c#8 はこれで制御できるかもと思いました

Copy link
Member

@Hiroshiba Hiroshiba left a comment

Choose a reason for hiding this comment

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

LGTM!!
(vue-electron-pluginがalpha版になりますが、一旦大丈夫なはずという判断です!)

1点気になったので議論させてください 🙏

npx playwright testでe2eテストを実行しますが、たぶん他のと同じようにnpm run test:e2eとかでplaywrightテストが走るようになっているのが見通しが良いのかなと思いました。
その場合はたぶんpackage.jsonのtest:e2eを変えると良さそうなのですが、いかがでしょう。

@so-c
Copy link
Author

so-c commented Jul 18, 2022

コメントありがとうございます。たしかにそうですね。pacakage.jsonのtest:e2eでe2eテスト実行する形にして、READMEを戻しました(コミットコメントのIssue番号間違えました、すみません……)

実行時オプションやテストコードの書き方(it/test)の見通しまで考えるならtest runnerをunittestにそろえることもできなくはないですが (Third party runners | Playwright)。そこまでやるとE2E側のコンフィギュレーションがまた複雑になるので、E2Eテスト視点だとこれくらいがちょうどよく感じます。

いかがでしょう?

@Hiroshiba
Copy link
Member

テストランナーというのがあるんですね・・・!
とりあえず一旦今の形にしておいて、複雑になっていったときに考えるというのも良いのかなと思いました!

@Hiroshiba Hiroshiba requested a review from takana-v July 19, 2022 18:00
@Hiroshiba
Copy link
Member

Hiroshiba commented Jul 19, 2022

CIが落ちてますね。。
懸念していた、electron buildで落ちてるような気がします。

Error: Application entry file "background.js" in the "D:\a\voicevox\voicevox\dist_electron\win-unpacked\resources\app.asar" does not exist. Seems like a wrong configuration.

background.jsはelectron側のコードですね・・・vue-electron-pluginを変えたからかなぁ・・・。

こちらのエラーをなんとかして解決する必要があるのですが、方針としては3つありそうです。

  1. @so-c さんに解決をお願いする
    • こちら的には最も嬉しいのですが、もしかしたら根が深くて大変かもしれません
    • その場合は↓2つが考えられそうです
  2. 3.0.0-alpha.4化したい旨のissueを建てて誰かが解決してくれるのを待つ
    • 僕も手が空いたら見れると思います
  3. playwright用の実験ブランチを作って、そこにプルリクをお願いする
    • 特別仕様で、coreのrustブランチとかがそうなっています
    • 自由に実験できるので割とおすすめですが、最後にブランチをマージする時にコンフリクトが発生すると大変な傾向があります
    • 今回の場合他とだいぶ独立しているので、ここの心配は小さめかなと思いました!

一旦ちょっと調べていただけると嬉しいです!!

@so-c
Copy link
Author

so-c commented Jul 19, 2022

どこまでできるか分かりませんがひとまず調べてみます!
so-c#6

file source doesn't exist from=D:\a\voicevox\voicevox_engine\run.dist

エンジンの位置を見失いがちですね……

@Hiroshiba
Copy link
Member

あ、そちらは問題ないはずです!
これはビルドが通るかのテストで、エンジンはそもそもありません。
実際他のテストビルドでも同じ情報が出力されていますが、エラー落ちしていません。
https://github.com/VOICEVOX/voicevox/runs/7381167699?check_suite_focus=true#step:12:151

@so-c
Copy link
Author

so-c commented Jul 19, 2022

3.0.0.-alphaのリリースノートにあった非互換Remove the main field from your package.jsonでした。fork先のActionsはグリーンになりました

Copy link
Member

@takana-v takana-v left a comment

Choose a reason for hiding this comment

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

LGTM!
手元の環境でも問題なくテストが実行されていることが確認できました。
テストも通っているみたいですのでマージしたいと思います。

@takana-v takana-v merged commit 7e27b08 into VOICEVOX:main Jul 20, 2022
@Hiroshiba
Copy link
Member

so-cさん、修正ありがとうございます!!
takanaさんもレビューと検証ありがとうございます!!

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.

3 participants