-
Notifications
You must be signed in to change notification settings - Fork 349
refactor: browser:e2eテストをモック用エンジンを使うように変更 #2442
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
The head ref may contain hidden characters: "\u30E2\u30C3\u30AF\u7528\u30A8\u30F3\u30B8\u30F3\u3092\u4F7F\u3063\u305Fe2e\u30C6\u30B9\u30C8\u306B\u5DEE\u3057\u66FF\u3048"
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 5 out of 7 changed files in this pull request and generated no comments.
Files not reviewed (2)
- .env.test: Language not supported
- tests/unit/backend/common/snapshots/configManager.spec.ts.snap: Language not supported
|
🚀 プレビュー用ページを作成しました 🚀 更新時点でのコミットハッシュ: |
|
ブラウザe2eテストを置き換えるにはmockの機能が足りなすぎたので、一旦draftにします。 |
| "enableRubyNotation": false, | ||
| "engineSettings": { | ||
| "074fc39e-678b-4c13-8916-ffca8d505d1d": { | ||
| "00000000-0000-0000-0000-000000000000": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
もともと.env.testを見ていたらしく、ついでに変わってしまった
tests/e2e/browser/辞書ダイアログ.spec.ts
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
未知語だと発音がなくなってしまうので、既知の言葉に
tests/e2e/browser/音声詳細.spec.ts
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
モックは助詞でアクセント句を区切るので、数字は正しくアクセントを区切れない。
ということで助詞だらけの言葉に置き換えました。
|
全ブラウザテストがエンジンmockで通過 🎉 |
|
をマージできたのでレビューを開けました! |
Hiroshiba
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
エンジンとモックそれぞれインスタンスがあって、それを URL に従ってスイッチするようになってます。
正確にはこんな感じ。
EngineFactory = EngineImpl()
MockFactory = MockImpl()
ConnectorFactory = () => {
EngineFactory()
MockFactory()
}|
ちょっと変更が多いので、run.exeの権限を変える部分のリファクタリングのPRと、.env.test-electronの切り出しのPRを別で投げたいと思います! |
73a4f5a to
b80b70e
Compare
| @@ -1,4 +1,4 @@ | |||
| # CI環境でのe2eテスト用の.envファイル。CI時に値が上書きされる。 | |||
| # CI環境でのelectronテスト用の.envファイル。CI時に値が上書きされる。 | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ブラウザe2eはエンジンモックにしたので、electron専用にしました。
tests/e2e/browser/辞書ダイアログ.spec.ts
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mockになって辞書テストが環境を汚さなくなったのでローカルでも実行するように
|
変更箇所で独立させれるものはできる限り別プルリクエストで出し、マージもされたのでレビュー可能になったと思います!! @sevenc-nanashi すみません、もしよかったらレビューいただけると・・・! 🙇 |
sevenc-nanashi
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
特に問題は無さそう。
|
レビューありがとうございます!!! マージします!!! |
内容
モック用エンジンを使ってブラウザe2eテストを書き換えます。
変更点は3つほどです。
mock://mockとするとモック用エンジンが使えるように関連 Issue
その他
が先に必要。
.github/workflows/test.ymlでやってるnemoエンジンダウンロードもなくせるかと思ったけど、まだelectron:e2e側が残ってるので難しそう。electron:e2e側はエンジンプロセス起動もテストに含まれてるので、実際にプロセス起動できないといけなそう・・・。