Skip to content

ci(e2e): align PR e2e workflow with ionic-theme-ios27 - #48

Merged
rdlabo merged 1 commit into
mainfrom
ci/e2e-pr-workflow
Sep 16, 2026
Merged

rdlabo merged 1 commit into
mainfrom
ci/e2e-pr-workflow

Conversation

@rdlabo

@rdlabo rdlabo commented Sep 16, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Trigger PR E2E only for pull requests targeting main.
  • Add deploy-report concurrency group to serialize gh-pages pushes.
  • Use !cancelled() for deploy-report and comment-results so cancellation does not try to publish/download missing artifacts.
  • Add a verification step that curls the published report URL before commenting.
  • Update the report comment to use a marker and edit/remove duplicates.
  • Run test:e2e:visual for Ionic 8 and add the test:e2e:visual demo script.
  • Raise timeout-minutes from 10 to 20 to avoid timing out on large visual diffs with 2 retries.

This is a workflow-only change; it does not touch theme code.

Test plan

  • New PRs to main use this workflow and reach deploy-report.
  • https://<owner>.github.io/ionic-theme-md3/pr-<n>/ is published after the E2E job.

Generated with Devin


Devin Review

- Trigger only for PRs to `main`.
- Add `deploy-report` concurrency group to serialize gh-pages pushes.
- Use `!cancelled()` for `comment-results` and `deploy-report` so
  cancellation does not try to publish a missing report.
- Add `Verify published report URL` step and a marker-based PR comment.
- Run `test:e2e:visual` for Ionic 8 to match ios27; add the script.
- Raise `timeout-minutes` from 10 to 20 because the current visual diff
  plus 2 retries exceeds 10 minutes.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@rdlabo
rdlabo merged commit c38ad26 into main Sep 16, 2026
8 of 9 checks passed

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Devin Review found 5 potential issues.

Devin Review

Comment on lines +124 to +125
group: playwright-report-${{ github.event.pull_request.number }}
cancel-in-progress: false

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 PR別のグループでは共有ブランチへのpushが競合する

異なるPRのデプロイやクリーンアップは、同じ gh-pages ブランチへ同時にpushできます。競合した実行は失敗し、対象レポートが更新または削除されません。

Learn more

すべてのレポートは一つの gh-pages ブランチへ書き込まれます。このグループ名にはPR番号が含まれるため、PR #10とPR #11のデプロイは互いを待ちません。cleanup-report は同じブランチへpushしますが、どのconcurrencyグループにも属しません。各実行が別々にチェックアウトしたブランチを公開するため、同時pushでは少なくとも一方が競合して失敗し得ます。

Example: PR #10とPR #11のテストが同時に完了すると、グループ名はそれぞれ playwright-report-10 と playwright-report-11 です。両方が同じ gh-pages の先端から別コミットを作り、後からpushしたジョブが競合します。

Recommended fix: deploy-report と cleanup-report の両方に、PR番号を含まない共通concurrencyグループを設定してください。gh-pages を更新する他のワークフローがある場合も同じグループへ参加させてください。

Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +124 to +125
group: playwright-report-${{ github.event.pull_request.number }}
cancel-in-progress: false

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 古い実行による最新レポートの上書き

連続pushで新しいテストが先に完了すると、古い実行が後から公開できます。レポートとコメントが現在のPR先端ではなく古い結果へ戻ります。

Learn more

このconcurrency制御はデプロイ開始後にだけ適用され、同じPRの古いテスト実行をキャンセルしません。cancel-in-progress: false なので、古い実行もグループを取得できれば必ず公開を試みます。テスト完了順はコミット順と一致しないため、古い成果物が最新成果物の後に公開されます。コメント本文もその古いworkflow runへのリンクで更新されます。

Example: コミットAのテスト中にコミットBをpushし、Bのテストが先に終わるとBのレポートが公開されます。その後Aが終わるとAのレポートと実行リンクで上書きされますが、PRの先端はBのままです。

Recommended fix: PR単位で古いworkflow run全体をキャンセルするか、公開直前に実行対象SHAが現在のPR head SHAと一致することを確認してください。共有ブランチのpush直列化とは別に、古い成果物を公開しない仕組みが必要です。

Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +170 to +173
- name: Verify published report URL
env:
REPORT_URL: https://${{ github.repository_owner }}.github.io/${{ github.event.repository.name }}/pr-${{ github.event.pull_request.number }}/
run: curl --fail --silent --show-error --location --retry 6 --retry-delay 10 --retry-all-errors --max-time 20 --output /dev/null "$REPORT_URL"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 失敗した公開確認の後にもレポートリンクが投稿される

URL確認が失敗しても、後続コメントは always() により実行されます。未公開または到達不能なレポートへのリンクがPRへ投稿されます。

Learn more

このステップはcurlの終了コードで公開URLを検証します。しかし、直後のコメントステップには if: always() があるため、このステップが失敗してもコメント処理が実行されます。その結果、追加した検証はジョブを失敗させるだけで、リンク投稿を止めません。

Example: Pagesの反映が全リトライ後も404ならcurlは失敗します。それでもコメントは作成または更新され、利用者には404のURLが案内されます。

Recommended fix: コメントステップを検証成功時だけ実行してください。例えば検証ステップへIDを付けて成功を条件にするか、コメントステップの if: always() を通常の成功条件へ変更します。デプロイ失敗時にも既存コメントを整理したい場合は、成功用と失敗用の処理を分けてください。

Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +54 to +59
run: |
if [ "${{ matrix.ionic-major }}" = "8" ]; then
PLAYWRIGHT_JSON_OUTPUT_NAME=e2e/screenshot.spec.ts-ionic8.json npm run test:e2e:visual -- --reporter=json,html
else
PLAYWRIGHT_JSON_OUTPUT_NAME=e2e/screenshot.spec.ts-ionic9.json npm run test:e2e -- --reporter=json,html
fi

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔍 Ionic 8のRTL回帰テストが対象外になる

Ionic 8は視覚テストだけを実行するため、RTL回帰テスト はIonic 9限定になります。このカバレッジ縮小が意図どおりか確認してください。

Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +15 to +16
# ios27 uses 10, but this PR's large visual diff + 2 retries exceeds that.
timeout-minutes: 20

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔍 タイムアウト理由が一時的な文脈に依存する

永続設定のコメントが「this PR」の差分を根拠にしており、マージ後は文脈が失われます。20分の恒久的な根拠へ直すか確認してください。

Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@github-actions

Copy link
Copy Markdown
Contributor

Playwright test results

passed  82 passed

Details

stats  82 tests across 2 suites
duration  1 minute, 40 seconds
commit  f09181e
info  This detailed result covers Ionic 9 only. Ionic 8 runs against the same screenshots in a separate matrix job; check the workflow run for both results. To update the screenshots, comment with /update-screenshots.

github-actions Bot added a commit that referenced this pull request Sep 16, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📊 Ionic 9 Playwright Test Report

View the detailed Ionic 9 report: https://rdlabo-dev.github.io/ionic-theme-md3/pr-48/

Ionic 8 runs against the same screenshots in a separate matrix job. View both results in the workflow run.

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.

1 participant