Skip to content

Add playwright tests for new PRs - #221

Open
sirpy wants to merge 13 commits into
mainfrom
feat/add-playwright-tests
Open

sirpy wants to merge 13 commits into
mainfrom
feat/add-playwright-tests

Conversation

@sirpy

@sirpy sirpy commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Description

Added playwright tests to the action flow

@sirpy
sirpy requested review from a team, L03TJ3 and blueogin and a lite review from Copilot and removed request for blueogin and Copilot August 31, 2026 14:57
@sirpy

sirpy commented Aug 31, 2026

Copy link
Copy Markdown
Contributor Author

@L03TJ3 playwright tests are failing, please handle this or open a bounty

@sirpy

sirpy commented Sep 1, 2026

Copy link
Copy Markdown
Contributor Author

@copilot create a report why playwright tests are failing and what is required in order to fix them

Copilot AI commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

@copilot create a report why playwright tests are failing and what is required in order to fix them

I checked PR #221’s failing playwright-demo job (99532340394) and reproduced it locally with pnpm test:demo.

Root causes:

  • tests/widgets/ai-credits-widget/states.spec.ts:233 depends on AppKit config. In CI, VITE_REOWN_PROJECT_ID is not set, so the story renders the AppKit not configured fallback instead of AiCreditsWidget-appkit-connect. The test checks the fallback too early and then waits for a root that never appears.
  • Several tests/widgets/goodreserve-widget/states.spec.ts assertions are out of sync with the current rendered values/formatting. Examples from the current page snapshots:
    • expected 108.2500, actual 108.25
    • expected 216.5000, actual 216.50
    • expected 10,230 G$, actual 10,230.46 G$
    • expected Balance: 300.00, actual Balance: 300.123457
  • tests/widgets/citizen-claim-widget/states.spec.ts:88 is live-RPC dependent and flaky. It failed once in the full run, then passed on retry.

What is required to fix:

  1. For the AppKit test: either provide VITE_REOWN_PROJECT_ID to the workflow, or make the test wait for and accept the no-config fallback deterministically.
  2. For GoodReserve: update the assertions to match the widget’s current formatting/fixture outputs, or derive expected display text from the same formatting logic instead of hard-coded stale strings.
  3. For Citizen Claim: decide whether this live-RPC check should stay required in CI. If yes, stabilize it with stronger waiting/mocking; if not, gate or move it to a non-blocking/manual job.

The failing workflow file itself is not the primary issue; the failures are in existing Playwright specs already present in the repo.

@sirpy

sirpy commented Sep 1, 2026

Copy link
Copy Markdown
Contributor Author

@copilot

  1. the vite project id was added to project secrets. verify it solves the appkit issue
  2. fix the out of the sync text in tests

Co-authored-by: sirpy <671095+sirpy@users.noreply.github.com>
Copilot AI lite review requested due to automatic review settings September 1, 2026 12:23

Copilot AI commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

@copilot

  1. the vite project id was added to project secrets. verify it solves the appkit issue
  2. fix the out of the sync text in tests

Addressed in a530c63.

  • The workflow now exports VITE_REOWN_PROJECT_ID into the Playwright step, and the AppKit spec passes when that env var is present.
  • I also updated the stale GoodReserve text assertions to the current rendered values and re-ran the affected Playwright cases successfully.

Copilot AI 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.

Pull request overview

This PR improves Playwright-based CI coverage by adding a dedicated GitHub Actions workflow to run the Storybook/demo Playwright suite on PRs, and adjusts a couple of widget Playwright expectations to match updated numeric formatting and loading timings.

Changes:

  • Added a new GitHub Actions workflow to run pnpm test:demo on pushes/PRs to main.
  • Updated GoodReserveWidget Playwright assertions to expect the new formatted numeric outputs.
  • Increased the visibility assertion timeout for an AiCreditsWidget AppKit connect-wallet test to reduce flakiness.

Reviewed changes

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

File Description
tests/widgets/goodreserve-widget/states.spec.ts Updates expected formatted amounts/balances in widget state smoke tests.
tests/widgets/ai-credits-widget/states.spec.ts Adds a longer timeout for the AppKit connect-wallet root visibility check.
.github/workflows/playwright-demo-tests.yml Introduces a CI workflow to run Playwright demo tests on PRs/pushes.
Suppressed comments (3)

tests/widgets/goodreserve-widget/states.spec.ts:116

  • This assertion uses substring text matching, so it won’t catch regressions where the formatted amount is rendered with extra decimals (e.g. 108.2500). Prefer an exact match to validate the formatting produced by formatTokenAmount.
  await expect(page.getByText('108.25')).toBeVisible()

tests/widgets/goodreserve-widget/states.spec.ts:136

  • getByText('216.50') is a substring match by default; it would also match 216.5000, reducing the test’s ability to enforce the intended 2-decimal formatting. Use an exact match here.
  await expect(page.getByText('216.50')).toBeVisible()

tests/widgets/goodreserve-widget/states.spec.ts:176

  • This assertion is currently a substring match. If the UI accidentally renders additional decimals (e.g. 10,230.4600 G$), the test would still pass. Use exact matching to make the formatting check strict.
  await expect(page.getByText('10,230.46 G$')).toBeVisible()

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

Comment thread tests/widgets/goodreserve-widget/states.spec.ts Outdated
Comment thread .github/workflows/playwright-demo-tests.yml
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: sirpy <671095+sirpy@users.noreply.github.com>
@sirpy

sirpy commented Sep 27, 2026

Copy link
Copy Markdown
Contributor Author

@L03TJ3

Copilot AI and others added 2 commits September 28, 2026 08:56
Co-authored-by: sirpy <671095+sirpy@users.noreply.github.com>
Co-authored-by: sirpy <671095+sirpy@users.noreply.github.com>
Copilot AI and others added 2 commits September 28, 2026 09:05
Co-authored-by: sirpy <671095+sirpy@users.noreply.github.com>
Co-authored-by: sirpy <671095+sirpy@users.noreply.github.com>
@L03TJ3

L03TJ3 commented Oct 5, 2026

Copy link
Copy Markdown
Contributor

@copilot

  1. Fix the secret check
    Remove secrets.VITE_REOWN_PROJECT_ID from the job-level if. Pass it through env to the configuration-check and test steps. Check for a missing value inside run and fail with a clear error.

  2. Allow approved fork PRs to run tests
    Replace the blanket fork exclusion with the existing deployment authorization pattern from deploy-storybook.yml: pull_request_target → authorize-tests → test job. Use the external/internal environment selection and require successful authorization before checking out or executing PR code. Test the event’s exact PR head SHA. Limit token permissions and exposed secrets to what the tests need, and disable checkout credential persistence.

Co-authored-by: L03TJ3 <6606028+L03TJ3@users.noreply.github.com>

Copilot AI commented Oct 5, 2026

Copy link
Copy Markdown
Contributor

@copilot

  1. Fix the secret check
    Remove secrets.VITE_REOWN_PROJECT_ID from the job-level if. Pass it through env to the configuration-check and test steps. Check for a missing value inside...

Addressed in f998614.

  • Removed the job-level secret check and now validate VITE_REOWN_PROJECT_ID inside explicit config/test steps with a clear error.
  • Switched the PR path to pull_request_target with an authorize-tests gate using the same external/internal environment approval pattern.
  • The test job now checks out the exact PR head SHA and disables checkout credential persistence.

@L03TJ3

L03TJ3 commented Oct 5, 2026

Copy link
Copy Markdown
Contributor

@copilot
Please update the Playwright workflow in GoodWidget PR #221:

  1. Support PRs targeting any branch, including main and feature branches.
    Remove the main-only restriction from PR triggers; keep pushes limited to main.

  2. Use pull_request to run tests for branches inside GoodDollar/GoodWidget.
    This should let Add playwright tests for new PRs #221 run before it is merged. Skip fork PRs on this trigger.

  3. Use pull_request_target for PRs from forks.
    Reuse the existing external environment approval gate.
    Only after approval, check out the PR’s exact head commit and run the tests. Skip same-repository PRs on this trigger, so each PR runs the tests only once.

  4. On the approved fork checkout step, set allow-unsafe-pr-checkout: true.
    This permits checkout after our approval; it must not bypass the approval gate. Keep contents: read and persist-credentials: false.

  5. Keep the secret fix: pass VITE_REOWN_PROJECT_ID through env to the relevant steps. Check for a missing value inside run, not in the job-level if. Don’t expose deployment credentials to tests.

Verify that #221 runs the full suite and reports the result for the tested commit.

Co-authored-by: L03TJ3 <6606028+L03TJ3@users.noreply.github.com>

Copilot AI commented Oct 5, 2026

Copy link
Copy Markdown
Contributor

@copilot
Please update the Playwright workflow in GoodWidget PR #221:

  1. Support PRs targeting any branch, including main and feature branches.
    ...

Addressed in 06905ee.

  • Same-repository PRs now run on pull_request for any target branch, so Add playwright tests for new PRs #221 can run before merge.
  • Fork PRs run on pull_request_target only after the external approval gate passes.
  • Both PR paths check out the exact head SHA; the approved fork path adds allow-unsafe-pr-checkout: true, and checkout credential persistence stays disabled.
  • VITE_REOWN_PROJECT_ID is still passed only to the config/test steps and validated inside run with a clear error.

L03TJ3 commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

@copilot Please fix the six non-CitizenClaim failures from run https://github.com/GoodDollar/GoodWidget/actions/runs/37321167725 on this PR branch:

  1. Governance registration: update the helper in tests/widgets/governance-widget/runtime.spec.ts to fill the current required social-profile and Discourse URL fields instead of the removed mission/distribution textareas. Keep the contract minimum-stake assertion and registration coverage.

  2. Staking migration: update the ready-summary and migrating-timeline tests in tests/widgets/staking-migration-widget/states.spec.ts to assert the current meaningful UI and migration progress. The old “Amount to migrate” and “Migration journey” headings no longer exist. Preserve the state/action coverage.

  3. Superfluid wallet chip: update both the content and leaderboard “Disconnect without an override” cases in tests/widgets/superfluid-campaign-widget/states.spec.ts. With no disconnect override, expect “To disconnect, use your wallet.” and no active Disconnect action.

  4. Governance footer: GovernanceWidget.tsx computes widgetBounds but does not pass them to MemberFooter. Verify the intended bounds API and pass bounds={widgetBounds} so the fixed footer matches the widget. Keep the existing 2px geometry check in showcase.spec.ts.

Keep changes minimal. Leave CitizenClaim code and tests completely untouched and report its existing failure separately. Run the affected tests and then the full pnpm test:demo suite; report results and the tested commit. Do not skip tests or weaken assertions to get a green result. No unrelated workflow changes or merging.

Co-authored-by: L03TJ3 <6606028+L03TJ3@users.noreply.github.com>
Co-authored-by: L03TJ3 <6606028+L03TJ3@users.noreply.github.com>

Copilot AI commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

@copilot Please fix the six non-CitizenClaim failures from run https://github.com/GoodDollar/GoodWidget/actions/runs/37321167725 on this PR branch:

  1. Governance registration: update the helper in te...

Addressed in c866fce.

The six requested non-CitizenClaim regressions are fixed, and the focused rerun passed:

  • governance registration runtime helper
  • governance showcase footer geometry
  • staking migration ready/migrating states
  • both superfluid wallet-chip no-override cases

I also ran pnpm test:demo on c866fce. The existing CitizenClaim live-RPC case still flakes separately as requested (tests/widgets/citizen-claim-widget/states.spec.ts:88). In this sandboxed run, four unrelated Superfluid external-navigation tests also opened chrome-error://chromewebdata/ instead of the target external URLs.

This branch was successfully deployed

2 active (outdated) deployments
preview-storybook — 5bf6a912 Deployed Oct 6, 2026 by Copilot via preview #392
internal — 5bf6a912 Deployed Oct 6, 2026 by Copilot via authorize-preview #392
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.

4 participants