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

feat(ci): save screenshots for E2E tests #5045

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

AntonioVentilii
Copy link
Collaborator

Motivation

Changes

Tests

@AntonioVentilii AntonioVentilii added the run-e2e-snapshots It needs the CI for E2E Snapshots label Mar 5, 2025
@AntonioVentilii AntonioVentilii requested a review from Copilot March 6, 2025 08:54
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

PR Overview

This PR updates the CI workflows to save E2E test screenshots and automatically update snapshots when changes are detected.

  • Conditional execution of jobs based on pull request labels is added to enable snapshot updates.
  • New steps have been introduced to check for snapshot changes, upload artifacts, and finalize snapshot updates by committing and pushing changes.

Reviewed Changes

File Description
.github/workflows/e2e-tests.yml Adds conditional execution for checkout and snapshot handling, and finalizes snapshot updates.
.github/workflows/update-snapshots.yml Refines job conditions to run only on non-pull-request events.

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Comment on lines 189 to 191
echo "ubuntu_status=${{ needs.update_snapshots.outputs.ubuntu_status }}" >> $GITHUB_ENV
echo "macos_status=${{ needs.update_snapshots.outputs.macos_status }}" >> $GITHUB_ENV

Copy link
Preview

Copilot AI Mar 6, 2025

Choose a reason for hiding this comment

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

The reference 'needs.update_snapshots' is likely incorrect because the snapshot outputs are provided by the 'e2e' job. Update the reference to 'needs.e2e.outputs.macos_status'.

Suggested change
echo "ubuntu_status=${{ needs.update_snapshots.outputs.ubuntu_status }}" >> $GITHUB_ENV
echo "macos_status=${{ needs.update_snapshots.outputs.macos_status }}" >> $GITHUB_ENV
echo "ubuntu_status=${{ needs.e2e.outputs.ubuntu_status }}" >> $GITHUB_ENV
echo "macos_status=${{ needs.e2e.outputs.macos_status }}" >> $GITHUB_ENV

Copilot is powered by AI, so mistakes are possible. Review output carefully before use.

Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
@AntonioVentilii AntonioVentilii requested a review from Copilot March 6, 2025 09:05
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

PR Overview

This PR introduces changes to save screenshots for E2E tests by updating the GitHub workflows. Key changes include:

  • Updating conditional checks to optionally run snapshot-related steps using the "run-e2e-snapshots" label
  • Adding a new job (finalize_snapshots_update) to manage snapshot finalization, commit, and PR creation
  • Refining checkout steps based on whether the event is a pull request or not

Reviewed Changes

File Description
.github/workflows/e2e-tests.yml Updated conditions and added steps to handle snapshot creation/updating
.github/workflows/update-snapshots.yml Simplified the condition to run only on non-pull_request events

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (2)

.github/workflows/e2e-tests.yml:38

  • Ensure that 'github.event.pull_request.labels' is defined in events other than pull_request to avoid unexpected runtime errors in non-pull_request triggers.
if: ${{ needs.check-e2e-changes.outputs.e2e-or-config-changed == 'true' || contains(github.event.pull_request.labels.*.name, 'run-e2e-snapshots') }}

.github/workflows/e2e-tests.yml:42

  • [nitpick] Consider consolidating the two Checkout steps (for pull_request and non-pull_request events) if they use identical parameters, to reduce redundancy and simplify maintenance.
if: ${{ github.event_name != 'pull_request' }}

@@ -16,7 +16,7 @@ concurrency:
jobs:
oisy-backend-wasm:
runs-on: ubuntu-24.04
if: ${{ github.event_name != 'pull_request' || contains(github.event.pull_request.labels.*.name, 'run-e2e-snapshots') }}
if: ${{ github.event_name != 'pull_request'}}
Copy link
Preview

Copilot AI Mar 6, 2025

Choose a reason for hiding this comment

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

Verify that removing the label-based condition from this workflow is intentional, since the e2e-tests workflow still checks for the 'run-e2e-snapshots' label.

Copilot is powered by AI, so mistakes are possible. Review output carefully before use.

Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
run-e2e-snapshots It needs the CI for E2E Snapshots
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant