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

Visual Regression Testの導入 #183

Merged
merged 8 commits into from
Aug 19, 2024
Merged

Visual Regression Testの導入 #183

merged 8 commits into from
Aug 19, 2024

Conversation

mii288
Copy link
Member

@mii288 mii288 commented Aug 5, 2024

close #157

Pull Requestが作成されたら、https://oysters.dev と、現在のブランチのスクリーンショットを比較して
テスト結果をコメント通知、レポートを閲覧できるようにします。

テスト成功

#183 (comment)
image

テスト失敗

#184 (comment)
image

できてないこと(修正が面倒で対応してないこと)

MacとUbuntuで画像圧縮に使われるアルゴリズムが若干異なるのか、Macで npm run test:vrt を実行すると画像部分で差分が発生します。
ローカルでDockerコンテナを作ってテストすればいいと思うのですが、コスパが悪そうなので見送りました。

処理手順

  1. Playwrightで https://oysters.dev と、現在のブランチのトップページのスクリーンショット画像を保存
  2. reg-cliで画像差分チェック
  3. reg-cliのテストレポートをNetlifyにデプロイし、そのURLをPRにコメント

その他

  • Playwrightを導入するためにNodeJSのバージョンをLTS(20.16.0)に更新
    engine-strictの設定を有効にしました(違うNodeJSのバージョンでnpmコマンドを実行したときにエラーになります)
  • actions/setup-node@v4 で node-version-file オプションを指定(matrixでの指定を削除)

@mii288 mii288 marked this pull request as draft August 5, 2024 13:12
@mii288 mii288 force-pushed the vrt branch 9 times, most recently from 398ecbe to 1eb0d8d Compare August 5, 2024 16:09
Copy link

github-actions bot commented Aug 5, 2024

✨✨ That's perfect, there is no visual difference! ✨✨
You can check this report out here.

@mii288 mii288 force-pushed the vrt branch 6 times, most recently from 9a72837 to 7de1b85 Compare August 6, 2024 07:57
@oystersjp oystersjp deleted a comment from reg-suit bot Aug 6, 2024
@mii288 mii288 marked this pull request as ready for review August 6, 2024 09:15
@mii288 mii288 changed the title [WIP] Visual Regression Testの導入 Visual Regression Testの導入 Aug 6, 2024
- name: Deploy to Netlify
id: netlify
# FYI: https://github.com/netlify/actions/pull/65
uses: South-Paw/action-netlify-cli@v2
Copy link
Member Author

Choose a reason for hiding this comment

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

このリポジトリで使われている nwtgck/actions-netlify だと、ドット始まりのフォルダがうまくアップロードできなかったので、 別のGithub Actionsを使いました。

最初はNetlifyの公式アクションを使おうと思ったんですが、ちょっとメンテされてないのかWarningが出ていた( netlify/actions#65 )ので、それが修正された South-Paw/action-netlify-cli を使ってます。

Copy link
Contributor

Choose a reason for hiding this comment

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

似たような機能のactionを複数利用していると、分かりにくなるのでこの機会に、CIでのtest deployを行なっているactionもSouth-Paw/action-netlify-cli に修正していただけると助かります🙏

- uses: nwtgck/actions-netlify@v1.0

Copy link
Member Author

Choose a reason for hiding this comment

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

  • コメントで使っているアクションがPRに対してコメントいれるアクションだったこと
  • pushしたコミットごとにデプロイのコメントがつくとSlackの通知的に邪魔だったりPRから探すのに不便

だったので、pull request作成時のみDeploy Previewを作成するように振る舞いを変更しました。(もとの振る舞いのほうがよさそうなら戻します)
↓の用な感じでPRを作成・更新するとDeploy Previewについてのコメントがされて確認できます。
#183 (comment)

.github/actions/setup/action.yml Show resolved Hide resolved
.github/workflows/vrt.yml Show resolved Hide resolved
uses: thollander/actions-comment-pull-request@v2
with:
message: |
**✨✨ That's perfect, there is no visual difference! ✨✨**
Copy link
Member Author

Choose a reason for hiding this comment

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

reg-suit の通知プラグインのテキストを流用

tests/vrt.spec.ts Show resolved Hide resolved
@mii288 mii288 requested a review from YuMuuu August 6, 2024 13:50
Copy link
Contributor

@YuMuuu YuMuuu left a comment

Choose a reason for hiding this comment

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

リグレッションテストの対象はtopページのみでしょうか👀? 以前このリポジトリをgoのフレームワークからnext.jsに移行した際は画像のリサイズ/トリミングやアンカーリンクの遷移がうまく機能せず困った記憶があるので

https://oysters.dev/#about

などのアンカーリンクでの移動先もリグレッションテストの対象にした方が良いのかなと思いました 🙋

追記: 温度感わからないですが、アンカーリンク先の比較がコスパ悪そうなら上記の比較は行わないという判断でも良いと思います

tests/vrt.spec.ts Show resolved Hide resolved
.github/workflows/vrt.yml Show resolved Hide resolved
.github/workflows/vrt.yml Show resolved Hide resolved
- name: Deploy to Netlify
id: netlify
# FYI: https://github.com/netlify/actions/pull/65
uses: South-Paw/action-netlify-cli@v2
Copy link
Contributor

Choose a reason for hiding this comment

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

似たような機能のactionを複数利用していると、分かりにくなるのでこの機会に、CIでのtest deployを行なっているactionもSouth-Paw/action-netlify-cli に修正していただけると助かります🙏

- uses: nwtgck/actions-netlify@v1.0

@mii288
Copy link
Member Author

mii288 commented Aug 18, 2024

リグレッションテストの対象はtopページのみでしょうか👀? 以前このリポジトリをgoのフレームワークからnext.jsに移行した際は画像のリサイズ/トリミングやアンカーリンクの遷移がうまく機能せず困った記憶があるので

https://oysters.dev/#about

などのアンカーリンクでの移動先もリグレッションテストの対象にした方が良いのかなと思いました 🙋

追記: 温度感わからないですが、アンカーリンク先の比較がコスパ悪そうなら上記の比較は行わないという判断でも良いと思います

@YuMuuu
やってもいいと思ってるんですが、新しいデザインにメニューがないのでパスでいいかなと思って抜きましたー!

Copy link
Contributor

@YuMuuu YuMuuu left a comment

Choose a reason for hiding this comment

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

やってもいいと思ってるんですが、新しいデザインにメニューがないのでパスでいいかなと思って抜きましたー!

確かに!このままで良さそうですね

Copy link

✅ Deploy preview is ready!

Name Link
🔨 Latest commit cdb8051
😎 Deploy Preview https://66c1bafcf1bcf750bc0d5fd2--zealous-yalow-0137bf.netlify.app

Copy link
Contributor

@YuMuuu YuMuuu left a comment

Choose a reason for hiding this comment

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

🍎

@mii288 mii288 merged commit c1db20c into master Aug 19, 2024
9 checks passed
@mii288 mii288 deleted the vrt branch August 19, 2024 02:18
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