feat: enhance GHA to publish reports to GH pages#2552
feat: enhance GHA to publish reports to GH pages#2552callMeTheQA wants to merge 7 commits intodevelopfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughAdds a Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant WF as GitHub Actions Workflow
participant Runner as Actions Runner
participant Artifacts as Job Artifacts
participant Pages as GitHub Pages
participant PR as Post-comment Job
WF->>Runner: run test jobs (produce HTML artifact)
Runner->>Artifacts: upload HTML report artifact
WF->>Runner: run publish-report job (needs: merge-reports)
Runner->>Artifacts: download HTML report artifact
Runner->>Pages: configure & deploy report (uses pages/id-token perms)
Pages-->>WF: return deployed page URL (page_url)
WF->>PR: run post-comment job (needs: publish-report)
PR->>WF: post PR comment including conditional link to page_url
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
✅ All snapshot tests passed |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/playwright.yml (1)
194-195: Consider improving the fallback message clarity.The fallback message "HTML Test Report will be available after deployment" may be misleading since at this point in the workflow, the
publish-reportjob has already completed (successfully or not). If deployment failed, users would see this message indefinitely.Consider a more accurate fallback:
🔎 Suggested improvement
- ${{ needs.publish-report.outputs.page_url != '' && needs.publish-report.outputs.page_url != null && format('📊 [View HTML Test Report]({0})', needs.publish-report.outputs.page_url) || '📊 HTML Test Report will be available after deployment' }} + ${{ needs.publish-report.outputs.page_url != '' && needs.publish-report.outputs.page_url != null && format('📊 [View HTML Test Report]({0})', needs.publish-report.outputs.page_url) || '📊 HTML Test Report deployment was skipped or failed' }}
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/playwright.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Test (5 / 6)
- GitHub Check: Test (1 / 6)
- GitHub Check: Test (2 / 6)
- GitHub Check: Test (6 / 6)
- GitHub Check: Test (4 / 6)
- GitHub Check: Test (3 / 6)
🔇 Additional comments (2)
.github/workflows/playwright.yml (2)
9-10: LGTM!The added permissions are correctly required for GitHub Pages deployment.
pages: writeenables the deployment, andid-token: writeis needed for OIDC authentication with the Pages environment.
184-184: LGTM!Adding
publish-reportto theneedsarray ensures the comment includes the deployed report URL. Combined with!cancelled(), the comment will still be posted even if the Pages deployment fails.
callMeTheQA
had a problem deploying
to
github-pages
GitHub Actions
Failure
Playwright test results
Details
Failed testschromium › portfolioPage.spec.ts › Portfolio page › Verify elements and filters on portfolio page › verify portfolio page elements and filters on Tokens and DeFi Protocols tabs (Qase ID: 53) Flaky testschromium › landingPage.spec.ts › Landing page and navigation › Should navigate to the homepage and change tabs (Qase ID: 2) Skipped testschromium › swapActions.spec.ts › On chain swaps [Viewport: Mobile] › ETH chain swap pair (Qase ID: 23) 📊 Download HTML Test Report - Available in the workflow artifacts |
| echo "Report will be published to: $DEST_DIR" | ||
| - name: Deploy report to GitHub Pages | ||
| if: always() | ||
| uses: peaceiris/actions-gh-pages@v4 |
There was a problem hiding this comment.
For actions that are not github's, it's recommended to pin those to the full length sha
ref: https://docs.github.com/en/actions/reference/security/secure-use#:~:text=Pin%20actions%20to%20a%20full%2Dlength%20commit%20SHA
There was a problem hiding this comment.
Also would prefer to use actions/configure-pages@v5 and actions/upload-pages-artifact@v3 as peaceiris/actions-gh-pages seems unmaintained since 1y+
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/playwright.yml (1)
226-238: Fallback message is misleading.The message "HTML Test Report will be available after deployment" is shown when
page_urlis empty/null, but at this point thepublish-reportjob has already completed (it's in theneedsarray). If the URL is missing, it means deployment failed, not that it's pending.🔧 Suggested fix
- ${{ needs.publish-report.outputs.page_url != '' && needs.publish-report.outputs.page_url != null && format('📊 [View HTML Test Report]({0})', needs.publish-report.outputs.page_url) || '📊 HTML Test Report will be available after deployment' }} + ${{ needs.publish-report.outputs.page_url != '' && needs.publish-report.outputs.page_url != null && format('📊 [View HTML Test Report]({0})', needs.publish-report.outputs.page_url) || '📊 HTML Test Report deployment failed or unavailable' }}
🤖 Fix all issues with AI agents
In @.github/workflows/playwright.yml:
- Around line 210-224: The step named "Add report link to summary" currently
uses if: always(), which can add a broken Pages link when the "Deploy to GitHub
Pages" step fails; change the condition to only run after a successful
deployment (e.g., use if: success() or guard with the specific step/job outcome)
so the PAGES_URL echoing logic only executes when the deploy step (named "Deploy
to GitHub Pages") succeeded and the REPORT_PATH/GITHUB_PAGES URL is valid; keep
the REPORT_PATH and PAGES_URL construction but gate the step on deploy success.
🧹 Nitpick comments (3)
.github/workflows/playwright.yml (3)
174-183: Consider DRY: report path logic is repeated three times.The same conditional logic for determining
DEST_DIR/REPORT_PATHappears at lines 177-181, 200-204, and 213-217. Since the value is already computed and stored insteps.report-dest.outputs.destination, reuse it in subsequent steps.♻️ Suggested refactor for lines 200-204
- name: Set page URL id: set-url run: | - if [ "${{ github.event_name }}" = "pull_request" ]; then - REPORT_PATH="${{ github.event.pull_request.number }}" - else - REPORT_PATH="${{ github.run_id }}" - fi + REPORT_PATH="${{ steps.report-dest.outputs.destination }}" # Extract repo name from github.repository (format: owner/repo) REPO_NAME=$(echo "${{ github.repository }}" | cut -d'/' -f2) PAGE_URL="https://${{ github.repository_owner }}.github.io/${REPO_NAME}/${REPORT_PATH}/"Apply the same pattern to lines 213-217.
197-209: URL construction could use deployment output.The
actions/deploy-pagesaction already outputspage_urlatsteps.deployment.outputs.page_url. You could simplify the URL construction:♻️ Suggested simplification
- name: Set page URL id: set-url run: | - if [ "${{ github.event_name }}" = "pull_request" ]; then - REPORT_PATH="${{ github.event.pull_request.number }}" - else - REPORT_PATH="${{ github.run_id }}" - fi - # Extract repo name from github.repository (format: owner/repo) - REPO_NAME=$(echo "${{ github.repository }}" | cut -d'/' -f2) - PAGE_URL="https://${{ github.repository_owner }}.github.io/${REPO_NAME}/${REPORT_PATH}/" + REPORT_PATH="${{ steps.report-dest.outputs.destination }}" + BASE_URL="${{ steps.deployment.outputs.page_url }}" + PAGE_URL="${BASE_URL}${REPORT_PATH}/" echo "page_url=$PAGE_URL" >> "$GITHUB_OUTPUT" echo "Report URL: $PAGE_URL"
184-196: GitHub Pages deployment replaces all site content each time.The
actions/deploy-pages@v4action deplaces the artifact you upload and replaces the entire GitHub Pages site with it. This is expected behavior—previous reports are overwritten with each deployment.If you need to preserve historical reports, consider:
- Deploying to a branch with incremental commits instead of artifact-based deployments
- Using external storage (e.g., artifact links, cloud bucket) for report history
- Fetching existing Pages content and merging with new reports before deployment
Note: If multiple workflow runs execute concurrently, there is a potential race condition where deployments could overwrite each other's reports. Consider adding concurrency controls if this is a concern.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/playwright.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Test (1 / 6)
- GitHub Check: Test (6 / 6)
- GitHub Check: Test (4 / 6)
- GitHub Check: Test (3 / 6)
- GitHub Check: Test (5 / 6)
- GitHub Check: Test (2 / 6)
🔇 Additional comments (2)
.github/workflows/playwright.yml (2)
9-10: Permissions look correct for GitHub Pages deployment.The
pages: writeandid-token: writepermissions are required for theactions/deploy-pagesaction to work with OIDC authentication. These are appropriately scoped for the new functionality.
158-173: Job configuration looks good.The job is appropriately gated with
!cancelled()to publish reports even when tests fail, and the environment configuration for GitHub Pages is correct.
| - name: Add report link to summary | ||
| if: always() | ||
| run: | | ||
| if [ "${{ github.event_name }}" = "pull_request" ]; then | ||
| REPORT_PATH="${{ github.event.pull_request.number }}" | ||
| else | ||
| REPORT_PATH="${{ github.run_id }}" | ||
| fi | ||
| # Extract repo name from github.repository (format: owner/repo) | ||
| REPO_NAME=$(echo "${{ github.repository }}" | cut -d'/' -f2) | ||
| PAGES_URL="https://${{ github.repository_owner }}.github.io/${REPO_NAME}/${REPORT_PATH}/" | ||
| echo "## 📊 Playwright Test Report" >> $GITHUB_STEP_SUMMARY | ||
| echo "" >> $GITHUB_STEP_SUMMARY | ||
| echo "🔗 **[View Report](${PAGES_URL})**" >> $GITHUB_STEP_SUMMARY | ||
| echo "" >> $GITHUB_STEP_SUMMARY |
There was a problem hiding this comment.
always() may add broken links if deployment fails.
Using if: always() means this step runs even when Deploy to GitHub Pages fails, potentially adding a broken link to the summary. Consider conditioning on deployment success:
🔧 Suggested fix
- name: Add report link to summary
- if: always()
+ if: ${{ steps.deployment.outcome == 'success' }}
run: |📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Add report link to summary | |
| if: always() | |
| run: | | |
| if [ "${{ github.event_name }}" = "pull_request" ]; then | |
| REPORT_PATH="${{ github.event.pull_request.number }}" | |
| else | |
| REPORT_PATH="${{ github.run_id }}" | |
| fi | |
| # Extract repo name from github.repository (format: owner/repo) | |
| REPO_NAME=$(echo "${{ github.repository }}" | cut -d'/' -f2) | |
| PAGES_URL="https://${{ github.repository_owner }}.github.io/${REPO_NAME}/${REPORT_PATH}/" | |
| echo "## 📊 Playwright Test Report" >> $GITHUB_STEP_SUMMARY | |
| echo "" >> $GITHUB_STEP_SUMMARY | |
| echo "🔗 **[View Report](${PAGES_URL})**" >> $GITHUB_STEP_SUMMARY | |
| echo "" >> $GITHUB_STEP_SUMMARY | |
| - name: Add report link to summary | |
| if: ${{ steps.deployment.outcome == 'success' }} | |
| run: | | |
| if [ "${{ github.event_name }}" = "pull_request" ]; then | |
| REPORT_PATH="${{ github.event.pull_request.number }}" | |
| else | |
| REPORT_PATH="${{ github.run_id }}" | |
| fi | |
| # Extract repo name from github.repository (format: owner/repo) | |
| REPO_NAME=$(echo "${{ github.repository }}" | cut -d'/' -f2) | |
| PAGES_URL="https://${{ github.repository_owner }}.github.io/${REPO_NAME}/${REPORT_PATH}/" | |
| echo "## 📊 Playwright Test Report" >> $GITHUB_STEP_SUMMARY | |
| echo "" >> $GITHUB_STEP_SUMMARY | |
| echo "🔗 **[View Report](${PAGES_URL})**" >> $GITHUB_STEP_SUMMARY | |
| echo "" >> $GITHUB_STEP_SUMMARY |
🤖 Prompt for AI Agents
In @.github/workflows/playwright.yml around lines 210 - 224, The step named "Add
report link to summary" currently uses if: always(), which can add a broken
Pages link when the "Deploy to GitHub Pages" step fails; change the condition to
only run after a successful deployment (e.g., use if: success() or guard with
the specific step/job outcome) so the PAGES_URL echoing logic only executes when
the deploy step (named "Deploy to GitHub Pages") succeeded and the
REPORT_PATH/GITHUB_PAGES URL is valid; keep the REPORT_PATH and PAGES_URL
construction but gate the step on deploy success.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.github/workflows/playwright.yml:
- Around line 222-224: The post-comment job is being skipped on PRs because it
lists publish-report in its needs but publish-report is intentionally gated to
non‑PR events and thus skipped; update the post-comment job (post-comment) to
remove publish-report from its needs array (keep create-qase-run and
merge-reports) so post-comment can run on PRs, or alternatively change
publish-report's gating if post-comment must depend on it.
🧹 Nitpick comments (1)
.github/workflows/playwright.yml (1)
5-10: Scope Pages permissions to the publish-report job.
pagesandid-tokenare only needed for deployment; keeping them at workflow scope widens privileges unnecessarily.♻️ Suggested refactor
permissions: checks: write pull-requests: write contents: write - pages: write - id-token: write publish-report: + permissions: + pages: write + id-token: write if: ${{ !cancelled() && github.event_name != 'pull_request' }}Also applies to: 158-167
| post-comment: | ||
| needs: [create-qase-run, merge-reports] | ||
| needs: [create-qase-run, merge-reports, publish-report] | ||
| if: ${{ !cancelled() && github.event_name == 'pull_request' }} |
There was a problem hiding this comment.
Post-comment will be skipped on PRs due to skipped dependency.
publish-report is gated to non‑PR events, but post-comment needs it. On PRs, publish-report is skipped and GitHub Actions will skip post-comment, so no PR comment is posted.
🔧 Suggested fix (remove unnecessary dependency)
post-comment:
- needs: [create-qase-run, merge-reports, publish-report]
+ needs: [create-qase-run, merge-reports]
if: ${{ !cancelled() && github.event_name == 'pull_request' }}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| post-comment: | |
| needs: [create-qase-run, merge-reports] | |
| needs: [create-qase-run, merge-reports, publish-report] | |
| if: ${{ !cancelled() && github.event_name == 'pull_request' }} | |
| post-comment: | |
| needs: [create-qase-run, merge-reports] | |
| if: ${{ !cancelled() && github.event_name == 'pull_request' }} |
🤖 Prompt for AI Agents
In @.github/workflows/playwright.yml around lines 222 - 224, The post-comment
job is being skipped on PRs because it lists publish-report in its needs but
publish-report is intentionally gated to non‑PR events and thus skipped; update
the post-comment job (post-comment) to remove publish-report from its needs
array (keep create-qase-run and merge-reports) so post-comment can run on PRs,
or alternatively change publish-report's gating if post-comment must depend on
it.
|
@callMeTheQA what is the state of this PR? |
3 participants
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.