Skip to content

Fix e2e workflow (sharded)#6408

Open
witoszekdev wants to merge 3 commits intomainfrom
fix-e2e-workflow
Open

Fix e2e workflow (sharded)#6408
witoszekdev wants to merge 3 commits intomainfrom
fix-e2e-workflow

Conversation

@witoszekdev
Copy link
Member

@witoszekdev witoszekdev commented Mar 10, 2026

Fixed issue where masking caused sharded test workflow to break

This is not a security issue, it worked previously since it's already encrypted

e2e test now pass fine: https://github.com/saleor/saleor-dashboard/actions/runs/22908573370/job/66475269789?pr=6408

Fixed run pw-e2e failing due to missing deployment (triggered by separate tag previously)

@witoszekdev witoszekdev requested a review from a team as a code owner March 10, 2026 14:46
@changeset-bot
Copy link

changeset-bot bot commented Mar 10, 2026

🦋 Changeset detected

Latest commit: 2b353fc

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@witoszekdev witoszekdev added the run pw-e2e Run e2e (basic suite from PR automation) label Mar 10, 2026
Copy link
Contributor

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.

Pull request overview

Updates the prepare-accounts composite GitHub Action used by the Playwright e2e workflows to avoid a sharded-workflow failure caused by masking the generated accounts payload.

Changes:

  • Removes ::add-mask::${ACCOUNTS} from the prepare-accounts action step.
  • Continues to pass the encrypted ACCOUNTS value via $GITHUB_OUTPUT for downstream jobs/steps.

@@ -46,5 +46,4 @@ runs:
E2E_PERMISSIONS_USERS_PASSWORD: ${{ inputs.E2E_PERMISSIONS_USERS_PASSWORD }}
run: |
ACCOUNTS=$(node playwright/auth.js login)
Copy link

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

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

Consider adding a short comment explaining why ACCOUNTS is intentionally not masked (GitHub Actions may refuse to set step/job outputs that contain masked values and will skip the output). This helps prevent accidental reintroduction of ::add-mask:: which can break sharded workflows again.

Suggested change
ACCOUNTS=$(node playwright/auth.js login)
ACCOUNTS=$(node playwright/auth.js login)
# ACCOUNTS is intentionally not masked here; GitHub Actions may skip step/job outputs
# that contain masked values, which would break consumers (including sharded workflows).

Copilot uses AI. Check for mistakes.
@codecov
Copy link

codecov bot commented Mar 10, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 43.13%. Comparing base (e640f8c) to head (2b353fc).

Additional details and impacted files
@@           Coverage Diff            @@
##             main    #6408    +/-   ##
========================================
  Coverage   43.13%   43.13%            
========================================
  Files        2524     2524            
  Lines       44009    44009            
  Branches    10011    10397   +386     
========================================
  Hits        18983    18983            
  Misses      24985    24985            
  Partials       41       41            

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@witoszekdev witoszekdev added the test deployment Deploy Pull Request to *.saleor.rocks environment label Mar 10, 2026
Copilot AI review requested due to automatic review settings March 10, 2026 15:07
Copy link
Contributor

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.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

"saleor-dashboard": patch
---

`run pw-e2e` label on PRs will now also trigger a deployment. Previously e2e test failed due to a missing deployment without clear error.
Copy link

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

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

Minor grammar: “Previously e2e test failed …” reads like a singular test; consider “Previously, e2e tests failed …” (or “the e2e suite failed …”) to match the meaning and improve clarity in the changelog.

Suggested change
`run pw-e2e` label on PRs will now also trigger a deployment. Previously e2e test failed due to a missing deployment without clear error.
`run pw-e2e` label on PRs will now also trigger a deployment. Previously, e2e tests failed due to a missing deployment without a clear error.

Copilot uses AI. Check for mistakes.
jobs:
initialize-cloud:
if: ${{ github.event_name == 'workflow_dispatch' || contains(github.event.pull_request.labels.*.name, 'test deployment') }}
if: ${{ github.event_name == 'workflow_dispatch' || contains(github.event.pull_request.labels.*.name, 'test deployment') || contains(github.event.pull_request.labels.*.name, 'run pw-e2e') }}
Copy link

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

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

initialize-cloud now runs when a PR is labeled run pw-e2e, but unlike deploy-dashboard it doesn’t guard against fork PRs. On forks, required secrets (e.g. OP_SERVICE_ACCOUNT_TOKEN) won’t be available, so this job can fail unexpectedly if the label is applied. Consider adding the same github.event.pull_request.head.repo.full_name == 'saleor/saleor-dashboard' check (for pull_request events) to this job’s if: condition, while keeping workflow_dispatch working.

Suggested change
if: ${{ github.event_name == 'workflow_dispatch' || contains(github.event.pull_request.labels.*.name, 'test deployment') || contains(github.event.pull_request.labels.*.name, 'run pw-e2e') }}
if: ${{ github.event_name == 'workflow_dispatch' || (github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name == 'saleor/saleor-dashboard' && (contains(github.event.pull_request.labels.*.name, 'test deployment') || contains(github.event.pull_request.labels.*.name, 'run pw-e2e'))) }}

Copilot uses AI. Check for mistakes.
Comment on lines +1 to +5
---
"saleor-dashboard": patch
---

Fixed main e2e test suite failing due to using masking on shared job output
Copy link

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

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

This PR adds two separate changeset entries for the same underlying CI/workflow fix. That will produce two patch bumps / changelog entries; consider combining into a single changeset to avoid double version increments for one PR.

Copilot uses AI. Check for mistakes.
"saleor-dashboard": patch
---

Fixed main e2e test suite failing due to using masking on shared job output
Copy link
Member

Choose a reason for hiding this comment

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

I dont this this is actual product changelog. Same way we don't add changelog when we add tests

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

run pw-e2e Run e2e (basic suite from PR automation) test deployment Deploy Pull Request to *.saleor.rocks environment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants