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

QuestionDialogの文字色を修正、StorybookのVRTを改善 #2314

Merged
Merged
Show file tree
Hide file tree
Changes from 29 commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
2ce3fe8
Fix: ボタンの文字色を修正
sevenc-nanashi Oct 23, 2024
6af788a
Add: ダークテーマもスクリーンショットを撮るように
sevenc-nanashi Oct 23, 2024
b2457f1
Add: PUSH_PATを追加
sevenc-nanashi Oct 23, 2024
b1f0719
Fix: Workflowを修正
sevenc-nanashi Oct 23, 2024
e94ddb2
Fix: スクショの対象を修正
sevenc-nanashi Oct 23, 2024
75b1851
[update snapshots]
sevenc-nanashi Oct 23, 2024
5f3a5b9
Improve: configの出力を追加
sevenc-nanashi Oct 23, 2024
6840218
Fix: \!vrtタグを追加
sevenc-nanashi Oct 23, 2024
5d140c4
Change: !vrt -> no-vrt
sevenc-nanashi Oct 23, 2024
b8b8e53
Fix: skipのコメントアウトを削除
sevenc-nanashi Oct 23, 2024
a56c8b2
Code: コメントを追加
sevenc-nanashi Oct 23, 2024
a95f537
Fix: 出力先を修正
sevenc-nanashi Oct 23, 2024
69b77df
(スナップショットを更新)
github-actions[bot] Oct 23, 2024
b0d45b1
Fix: 条件式を修正
sevenc-nanashi Oct 23, 2024
798562f
Change: 常にfalseに
sevenc-nanashi Oct 23, 2024
b10f716
Delete: allow-emptyを消す
sevenc-nanashi Oct 23, 2024
2a2d732
Delete: 強引に差分を作る
sevenc-nanashi Oct 23, 2024
8018f87
Fix: fetch-depthを指定する
sevenc-nanashi Oct 23, 2024
3bbf027
(スナップショットを更新)
github-actions[bot] Oct 23, 2024
9c876ab
Add: READMEに追記
sevenc-nanashi Oct 23, 2024
20772c7
Merge branch 'main' into fix/question-dialog-color-broken
sevenc-nanashi Oct 23, 2024
a8a8991
Add: 説明を追加
sevenc-nanashi Oct 23, 2024
7d63cdb
(スナップショットを更新)
github-actions[bot] Oct 23, 2024
bae591c
Change: PUSH_PAT -> PUSH_TOKEN
sevenc-nanashi Oct 23, 2024
995fcfa
Add: e2e周りのtest-uiを説明に追加
sevenc-nanashi Oct 23, 2024
403824a
Change: no-vrt -> skipVrt
sevenc-nanashi Oct 23, 2024
c16cd49
Add: 画像を追加
sevenc-nanashi Oct 23, 2024
ec2d297
Revert: 文字列リテラル限定だったのでRevert
sevenc-nanashi Oct 23, 2024
998f146
Delete: 残骸を削除
sevenc-nanashi Oct 23, 2024
72e04e9
Code: 説明をいい感じにする
sevenc-nanashi Oct 24, 2024
ea01fa7
Change: バージョンを固定する
sevenc-nanashi Oct 24, 2024
64d8aa9
Improve: 説明を良い感じにする
sevenc-nanashi Oct 24, 2024
bd67aae
Change: no-vrt -> skip-screenshot
sevenc-nanashi Oct 24, 2024
b2106bd
Fix: 日本語を修正
sevenc-nanashi Oct 24, 2024
4a4b24f
Improve: 改行をいい感じにする
sevenc-nanashi Oct 24, 2024
0743fc8
Change: スクショの撮り方を変える
sevenc-nanashi Oct 24, 2024
4608413
Refactor: 書き方を変える
sevenc-nanashi Oct 24, 2024
3c19750
Fix: フルハッシュに
sevenc-nanashi Oct 24, 2024
d2dc5f2
Fix: 明らかに終わってるバグを修正
sevenc-nanashi Oct 24, 2024
624fab8
(スナップショットを更新)
github-actions[bot] Oct 24, 2024
1f71954
Change: 警告をコミット時に出す
sevenc-nanashi Oct 24, 2024
35a2500
Apply suggestions from code review
Hiroshiba Oct 24, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 28 additions & 2 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ jobs:
id: check-whether-to-update-snapshots
uses: actions/github-script@v7
with:
github-token: ${{ secrets.GITHUB_TOKEN }}
script: |
const commits = ${{ toJson(github.event.commits) }};
if (!commits) {
Expand All @@ -39,6 +38,17 @@ jobs:
core.setOutput("shouldUpdateSnapshots", shouldUpdateSnapshots);
console.log(`shouldUpdateSnapshots: ${shouldUpdateSnapshots}`);

const pushTokenProvided = `${{ secrets.PUSH_TOKEN }}` !== "";
if (shouldUpdateSnapshots && !pushTokenProvided) {
core.warning(
"PUSH_TOKENがSecretsに設定されていません。\n" +
"スナップショットを更新するコミットはpushされますが、" +
"Pull RequestのCIは手動で再実行する必要があります。\n" +
"詳細はREADME.mdを参照してください。"
);
sevenc-nanashi marked this conversation as resolved.
Show resolved Hide resolved
}
console.log(`pushTokenProvided: ${pushTokenProvided}`);

# ビルドのテスト
build-test:
runs-on: windows-latest
Expand Down Expand Up @@ -175,6 +185,11 @@ jobs:
if: needs.config.outputs.shouldUpdateSnapshots == 'true'
steps:
- uses: actions/checkout@v4
with:
# NOTE: デフォルトの設定だとgithub-push-actionが動いてくれないので設定を変える。
Hiroshiba marked this conversation as resolved.
Show resolved Hide resolved
# ref: https://github.com/ad-m/github-push-action/issues/44#issuecomment-581706892
persist-credentials: false
fetch-depth: 0

- name: Download artifacts
uses: actions/download-artifact@v4
Expand All @@ -184,6 +199,7 @@ jobs:
merge-multiple: true

- name: Commit updated snapshots
id: commit-updated-snapshots
run: |
# パッチを適用
for patch in patches/*.diff; do
Expand All @@ -197,11 +213,21 @@ jobs:
git config --global user.email "github-actions[bot]@users.noreply.github.com"
git add tests/
git commit -m "(スナップショットを更新)"
git push

echo "changes_exist=true" >> $GITHUB_OUTPUT
else
echo "No changes to commit"

echo "changes_exist=false" >> $GITHUB_OUTPUT
fi

- name: Push changes
if: steps.commit-updated-snapshots.outputs.changes_exist == 'true'
uses: ad-m/github-push-action@master
sevenc-nanashi marked this conversation as resolved.
Show resolved Hide resolved
with:
github_token: ${{ secrets.PUSH_TOKEN || secrets.GITHUB_TOKEN }}
branch: ${{ github.ref }}

lint:
runs-on: ubuntu-latest
steps:
Expand Down
12 changes: 12 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ Electron の機能が不要な、UI や音声合成などの End to End テス
npm run test:browser-e2e
npm run test-watch:browser-e2e # 監視モード
npm run test-watch:browser-e2e -- --headed # テスト中の UI を表示
npm run test-ui:browser-e2e # Playwright の UI を表示
```

Playwright を使用しているためテストパターンを生成することもできます。
Expand All @@ -168,6 +169,8 @@ Storybook のコンポーネントのスクリーンショットを比較して

```bash
npm run test:storybook-vrt
npm run test-watch:storybook-vrt # 監視モード
npm run test-ui:storybook-vrt # Playwright の UI を表示
```

#### スクリーンショットの更新
Expand All @@ -188,6 +191,15 @@ npm run test:storybook-vrt

4. Github Workflow が完了すると、更新されたスクリーンショットがコミットされます。

> [!NOTE]
> Secrets に `PUSH_TOKEN` という名前で [Fine-granted Tokens](https://github.com/settings/personal-access-tokens/new) を設定すると、そのトークンを使ってコミットを Push します。
> これが設定されていると Pull Request のテストが自動的にもう一度回るので、設定することを強く推奨します。
> トークンには `ユーザー名/voicevox` へのアクセス権を与え、 Repository permissions の Contents を Read and write に設定する必要があります。
> <details>
> <summary>トークンの設定例</summary>
> <img src="./docs/res/Fine-granted_Tokensの作成.png" width="50%">
> </details>
sevenc-nanashi marked this conversation as resolved.
Show resolved Hide resolved

##### ローカルで更新する場合

ローカル PC の OS に対応したもののみが更新されます。
Expand Down
Binary file added docs/res/Fine-granted_Tokensの作成.png
sevenc-nanashi marked this conversation as resolved.
Show resolved Hide resolved
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
3 changes: 3 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,10 @@
"test-watch:electron-e2e": "cross-env PWTEST_WATCH=1 VITE_TARGET=electron playwright test",
"test:browser-e2e": "cross-env VITE_TARGET=browser playwright test",
"test-watch:browser-e2e": "cross-env PWTEST_WATCH=1 VITE_TARGET=browser playwright test",
"test-ui:browser-e2e": "cross-env VITE_TARGET=browser playwright test --ui",
"test:storybook-vrt": "cross-env TARGET=storybook playwright test",
"test-watch:storybook-vrt": "cross-env TARGET=storybook PWTEST_WATCH=1 playwright test",
"test-ui:storybook-vrt": "cross-env TARGET=storybook playwright test --ui",
sevenc-nanashi marked this conversation as resolved.
Show resolved Hide resolved
"lint": "eslint --ext .js,.vue,.ts,.mts *.config.* src tests build .storybook",
"fmt": "eslint --ext .js,.vue,.ts,.mts *.config.* src tests build .storybook --fix",
"markdownlint": "markdownlint --ignore node_modules/ --ignore dist/ --ignore dist_electron/ ./",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ export const Close: Story = {

export const Closed: Story = {
name: "閉じている",
tags: ["no-vrt"],
args: {
openDialog: false,
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ export const Close: Story = {

export const Closed: Story = {
name: "閉じている",
tags: ["no-vrt"],
args: {
modelValue: false,
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ export const ClickBackdropWithCancel: Story = {

export const Closed: Story = {
name: "閉じている",
tags: ["no-vrt"],
args: {
modelValue: false,
},
Expand Down
3 changes: 1 addition & 2 deletions src/components/Dialog/TextDialog/QuestionDialog.vue
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,7 @@
:key="index"
flat
:label="button"
color="toolbar-button"
textColor="toolbar-button-display"
color="display"
class="text-no-wrap text-bold"
@click="onClick(index)"
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,4 +95,5 @@ export const OpenOfficialSite: Story = {

export const Closed: Story = {
name: "閉じている",
tags: ["no-vrt"],
};
48 changes: 37 additions & 11 deletions tests/e2e/storybook/スクリーンショット.spec.mts
Original file line number Diff line number Diff line change
Expand Up @@ -52,18 +52,44 @@ for (const story of currentStories) {
for (const [story, stories] of Object.entries(allStories)) {
test.describe(story, () => {
for (const story of stories) {
test(story.name, async ({ page }) => {
test.skip(
process.platform !== "win32",
"Windows以外のためスキップします",
);
if (story.tags.includes("no-vrt")) {
sevenc-nanashi marked this conversation as resolved.
Show resolved Hide resolved
sevenc-nanashi marked this conversation as resolved.
Show resolved Hide resolved
continue;
}
test.describe(story.name, () => {
for (const [theme, name] of [
["light", "ライト"],
["dark", "ダーク"],
]) {
test(`テーマ:${name}`, async ({ page }) => {
test.skip(
process.platform !== "win32",
"Windows以外のためスキップします",
);

await page.goto(`http://localhost:7357/iframe.html?id=${story.id}`);
const body = page.locator("body.sb-show-main");
await body.waitFor({ state: "visible" });
await expect(page).toHaveScreenshot(`${story.id}.png`, {
fullPage: true,
});
const params = new URLSearchParams();
params.append("id", story.id);
params.append("globals", `theme:${theme}`);
await page.goto(
`http://localhost:7357/iframe.html?${params.toString()}`,
);
const root = page.locator("#storybook-root");
const dialogRoot = page.locator("div[id^=q-portal--dialog--]");
const firstVisible = await Promise.race([
root.waitFor({ state: "visible" }).then(() => "root"),
dialogRoot.waitFor({ state: "attached" }).then(() => "dialog"),
]);

// ダイアログの場合はダイアログのスクリーンショットを、そうでない場合は#storybook-rootのスクリーンショットを撮る。
// q-portal-dialogはそのまま撮るとvisible扱いにならずtoHaveScreenshotが失敗するので、
// 子要素にある実際のダイアログ(.q-dialog)を撮る。
if (firstVisible === "dialog") {
const dialog = dialogRoot.locator(".q-dialog");
await expect(dialog).toHaveScreenshot(`${story.id}-${theme}.png`);
} else {
await expect(root).toHaveScreenshot(`${story.id}-${theme}.png`);
}
Copy link
Member

@Hiroshiba Hiroshiba Oct 24, 2024

Choose a reason for hiding this comment

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

かなりライブラリの実装依存なので、後から修正できるように丁寧に作っておきたいですねぇ。

  • Quasarは後々依存をやめると踏んで、ダイアログ→Quasarダイアログに呼び方を一部変える
  • 最初にダイアログ用かどうかで全コードを分岐するようにする
    • たぶんfirstVisible変数は無くせる(dialogRootの有無で分岐できる)

感じにしてみるのはどうでしょう。

Copy link
Member

@Hiroshiba Hiroshiba Oct 24, 2024

Choose a reason for hiding this comment

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

雰囲気こんな感じのコード?

// Quasarダイアログの場合はダイアログのスクリーンショットを、そうでない場合は#storybook-rootのスクリーンショットを撮る。
const root: Locater;

const dialogRoot = page.locator("div[id^=q-portal--dialog--]");
if (dialogRootがある) {
  // q-portal-dialogはそのまま撮るとvisible扱いにならずtoHaveScreenshotが失敗するので、
  // 子要素にある実際のダイアログ(.q-dialog)を撮る。
  await dialogRoot.waitFor({ state: "attached" })
  root = dialogRoot.locator(".q-dialog")
}
else {
  await root.waitFor({ state: "visible" })
  root = page.locator("#storybook-root");
}

await expect(root).toHaveScreenshot(`${story.id}-${theme}.png`);

Copy link
Member Author

Choose a reason for hiding this comment

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

ちょっと変えました。

});
}
});
}
});
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file not shown.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file not shown.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file not shown.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file not shown.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file not shown.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file not shown.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file not shown.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file not shown.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Copy link
Member

@Hiroshiba Hiroshiba Oct 24, 2024

Choose a reason for hiding this comment

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

あれ、そういえばダークモードだとボタンがprimary colorなんですね

Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Loading