Skip to content

Conversation

@jdkfx
Copy link
Contributor

@jdkfx jdkfx commented Dec 18, 2024

内容

  • 下記Issueから「アクセント欄の読みの部分をクリックすることで読みを変更できる」を追加

関連 Issue

ref #1462

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

その他

Copy link
Contributor Author

@jdkfx jdkfx Dec 18, 2024

Choose a reason for hiding this comment

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

テストを追加したことにより、この処理がないと既存の通っていたテストが通らなくなる。
理由は不明。

  1) [browser] › browser/音声パラメータ.spec.ts:12:5 › 音声パラメータ引き継ぎの設定 ─────────────────────────────────────

    Error: expect(received).toBe(expected) // Object.is equality

    Expected: "1.00"
    Received: "0.50"

       7 | async function validateValue(locator: Locator, expectedValue: string) {
       8 |   const value = await locator.evaluate((e: HTMLInputElement) => e.value);
    >  9 |   expect(value).toBe(expectedValue);
         |                 ^
      10 | }
      11 |
      12 | test("音声パラメータ引き継ぎの設定", async ({ page }) => {

        at validateValue (/Users/jdkfx/Desktop/workspace/voicevox/tests/e2e/browser/音声パラメータ.spec.ts:9:17)
        at /Users/jdkfx/Desktop/workspace/voicevox/tests/e2e/browser/音声パラメータ.spec.ts:37:3

    attachment #1: video (video/webm) ──────────────────────────────────────────────────────────────
    test-results/音声パラメータ-音声パラメータ引き継ぎの設定-browser/video.webm
    ────────────────────────────────────────────────────────────────────────────────────────────────

Copy link
Contributor Author

Choose a reason for hiding this comment

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

flakyになりやすいから、waitForTimeoutではない方法を使った方がいいかも。
https://zenn.dev/cureapp/articles/73cd66a37cc9e8

Copy link
Member

@Hiroshiba Hiroshiba Dec 20, 2024

Choose a reason for hiding this comment

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

flakyになりやすいから、waitForTimeoutではない方法を使った方がいいかも。

同感です!
ちょっとe2eにまだ慣れてないときのコードが多くwaitForTimeoutが点在していますが本当は使わないのがベストなはず。

ここの原因は・・・・ちょっと不正確かもですが、そもそもここはエンジンへのリクエストを送信して返ってくるまで待機してるかもですね・・・。
(テストを追加したことによりテストが並列実行されてエンジンが一瞬詰まる、みたいなのとかが起こる)

ちょっと良い解決策が思いつかないので、これ系はまだしばらくwaitForTimeoutのまま行かないとかもです 😇

@voicevox-preview-pages
Copy link

voicevox-preview-pages bot commented Dec 18, 2024

🚀 プレビュー用ページを作成しました 🚀

更新時点でのコミットハッシュ:0cf6605

@jdkfx jdkfx marked this pull request as ready for review December 18, 2024 15:30
@jdkfx jdkfx requested a review from a team as a code owner December 18, 2024 15:30
@jdkfx jdkfx requested review from Hiroshiba and removed request for a team December 18, 2024 15:30
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.

テスト追加ありがとうございます!!!
かなり良い感じだと思います!!!

@Hiroshiba
Copy link
Member

あっ .mora-tableを見ていた別のテストが落ちてますね!!!
.mora-tableを.accent-phraseに変えれば大丈夫かも…
確認不足でした、すみません🙇🙇

@jdkfx
Copy link
Contributor Author

jdkfx commented Dec 22, 2024

@Hiroshiba 今ほかのPRの対応中でした!こちらも後で直すつもりです!

@jdkfx jdkfx requested a review from Hiroshiba December 22, 2024 10:05
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!!

すみません🙇 修正ありがとうございました!!!

@Hiroshiba Hiroshiba merged commit 762265a into VOICEVOX:main Dec 22, 2024
10 checks passed
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.

2 participants