test(demo-wallet): redesign e2e suite — UI mocks + TON Connect mock-dApp#485
Open
lesyuk wants to merge 10 commits into
Open
test(demo-wallet): redesign e2e suite — UI mocks + TON Connect mock-dApp#485lesyuk wants to merge 10 commits into
lesyuk wants to merge 10 commits into
Conversation
…App (TON-1701) UI-mock specs (mock-first via page.route on the walletkit API): unlock, dashboard, assets, nft, history, send, swap, staking, amount-formatting + page objects and a shared walletApi mock. No real wallet/funds — a fresh wallet + mocked Toncenter v3. TON Connect mock-dApp two-tab suite (connect / sendTransaction / signData / signMessage) driven by a self-contained @tonconnect/sdk mock-dApp fixture; asserts the redesigned modal copy + per-type actions. sendTransaction runs without funds via a mocked balance + disableNetworkSend (the wallet signs and responds to the dApp over the bridge but skips the on-chain broadcast). Stable Allure historyId (describe-chain + test title) for zero-manual TestOps linking. DemoWallet expect*Modal assertion helpers; mockdapp + tonconnect Playwright configs.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds mock-first Playwright infrastructure for the demo-wallet, including separate UI and two-tab TON Connect runners, deterministic wallet API mocks, page objects, fixtures, a self-contained mock dApp, and broad UI and TON Connect coverage. CI now runs both suites. ChangesDemo-wallet E2E infrastructure and test suites
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
…quarantine runner specs (TON-1701) The default e2e.config.ts swept every spec under ./e2e — including the two-tab suites (which need extra webServer tabs) and the allure-test-runner specs (whose backend currently returns 500). With --retries=3 that blew the 30-min CI budget (reached 99/122 tests before cancel; 120 "case data 500" errors in the log). - testIgnore ton-connect/** and mock-dapp-tests/** — they run via their own configs (e2e.tonconnect.config.ts / e2e.mockdapp.config.ts), which start the minter :5174 / mock-dApp :5175 tabs themselves. - testIgnore + quarantine the runner specs (connect / signData / sendTransaction / localSendTransaction) until the allure-test-runner backend is restored. - e2e_web.yml: after the ui-tests run, also run the self-contained mock-dApp suite (e2e.mockdapp.config.ts); aggregate exit codes so both report into allure-results. After this the default web e2e run is the mock-first e2e/ui-tests suite (64 tests, 12 files) plus the mock-dApp TON Connect suite.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/e2e_web.yml:
- Around line 90-97: The CI step is missing the dedicated TON Connect test suite
because it only runs the default e2e config and e2e.mockdapp.config.ts, while
e2e.config.ts now excludes ton-connect specs. Update this workflow step to also
invoke the dedicated TON Connect Playwright config so
apps/demo-wallet/e2e/ton-connect/connect.spec.ts runs in CI, and keep the
existing exit-code aggregation behavior in the same shell block.
In `@apps/demo-wallet/e2e.mockdapp.config.ts`:
- Around line 30-45: Reject invalid dot-less mock dApp overrides in the e2e mock
config: in e2e.mockdapp.config.ts, the APP_URL assignment still trusts
process.env.MOCK_DAPP_URL as-is, which allows localhost to bypass the local
server and fail WalletKit’s host guard. Update the APP_URL setup to validate the
override hostname and only accept a dotted host like 127.0.0.1, otherwise fall
back to the default mock-dApp URL; keep this logic close to APP_URL,
WALLET_SOURCE, and mockDappServer so the source of truth remains clear.
In `@apps/demo-wallet/e2e/mock-dapp-tests/connect.spec.ts`:
- Around line 37-45: The rejection test currently asserts the dApp is
disconnected using the initial empty state, so it can pass without verifying the
rejection reached the bridge. Update the connect flow in connect.spec.ts, around
test('Rejecting the connect request leaves the dApp disconnected'), to wait for
an explicit mock-dapp terminal rejection signal before checking dapp-connected,
then assert the disconnected state afterward using the existing wallet and dapp
helpers.
In `@apps/demo-wallet/e2e/mock-dapp/vite.config.ts`:
- Around line 18-27: The mock dApp’s Vite config is resolving `@tonconnect/sdk`
through appkit-minter, which couples demo-wallet to another app’s dependency
graph and the SDK’s internal dist layout. Update the setup around the
sdkCjs/sdkEsm resolution in vite.config.ts so demo-wallet declares
`@tonconnect/sdk` itself and imports/aliases it directly from its own package
graph, removing the createRequire path into appkit-minter.
In `@apps/demo-wallet/e2e/mocks/walletApi.ts`:
- Around line 469-473: The traces mock route in walletApi currently ignores the
request pagination parameters and always returns the full events set, so "Load
more" cannot be tested correctly. Update the page.route(TRACES_RE) handler to
read the request's limit and offset query params and pass a sliced page of
events into tracesBody, while still preserving the account stamping logic used
for toEvent. Keep the fix localized to the traces mock helper so pagination
behavior matches the real /api/v3/traces endpoint.
In `@apps/demo-wallet/e2e/qa/WalletApp.ts`:
- Around line 71-76: The recovery flow in WalletApp’s mount wait only reloads
the page and returns after domcontentloaded, so it can still hand back before
the app is mounted. Update the retry path in the mount-wait logic that uses
hasContent(), page.reload(), and the `#root` > * locator to wait again for the
first root child to be attached after the reload, so the recovery remains
synchronous before continuing.
In `@apps/demo-wallet/e2e/ton-connect/connect.spec.ts`:
- Around line 43-45: The approve-path assertion in connect.spec.ts is only
checking widget.connectButton visibility, which is also covered by the other
case and can hide a missing connected transition. Update the test around the
approval flow in the ton-connect connect spec to assert a connected-only signal
from the widget after approval, such as connected wallet text or connected
state, using the existing widget and connectButton-related selectors/methods to
locate the right UI state.
In `@apps/demo-wallet/e2e/ui-tests/assets.spec.ts`:
- Around line 69-81: Scope the fiat check to the specific asset row being tested
in the asset fiat test, since the current page-wide dollar text match can pass
because of the default GRAM row. In the test that uses mockWalletApi,
createWalletOnDashboard, and AssetsPage, give the mocked jetton a non-zero
balance so it renders a fiat amount, then assert the dollar value within that
jetton’s row instead of using a global page.getByText('$') lookup; use the
existing asset row locator pattern around gramName as a guide for targeting the
correct row.
In `@apps/demo-wallet/e2e/ui-tests/dashboard.spec.ts`:
- Around line 24-30: The fiat-total test is too broad because the current
`getByText('$').first()` and integer-only regex can match unrelated dashboard
text. Update `test('Renders the fiat total once balance and rates load', ...)`
in `dashboard.spec.ts` to scope assertions to the total widget via the specific
container or by checking the full formatted mocked amount rendered by
`BalanceTotal`, so the locator uniquely targets that component instead of any
matching text on the page.
In `@apps/demo-wallet/e2e/ui-tests/staking.spec.ts`:
- Around line 34-38: The Max staking test is too weak because
`expect(...).not.toHaveValue('')` only checks that the input changed, not that
it used the correct reserve-adjusted amount. Update the `Max fills the stake
amount keeping the gas reserve` test in `staking.spec.ts` to assert the exact
expected value after clicking the `Max` button, using the `stake-amount-input`
test id and the balance/reserve behavior described in the test setup so
regressions in the filled amount will fail.
In `@apps/demo-wallet/e2e/ui-tests/swap.spec.ts`:
- Around line 34-39: The Max-fill assertion in swap.spec.ts is too weak because
it only checks that the From input is non-empty. Update the test around the Max
button and fromInput locator to assert the filled value is strictly below the
mocked 12.5 GRAM balance, so it verifies the gas reserve behavior rather than
just any populated value.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 400d52e4-73f5-440a-985d-8237eb98359b
⛔ Files ignored due to path filters (1)
apps/demo-wallet/e2e/mock-dapp/vite.svgis excluded by!**/*.svg
📒 Files selected for processing (35)
.github/workflows/e2e_web.ymlapps/demo-wallet/e2e.config.tsapps/demo-wallet/e2e.mockdapp.config.tsapps/demo-wallet/e2e.tonconnect.config.tsapps/demo-wallet/e2e/demo-wallet/DemoWallet.tsapps/demo-wallet/e2e/mock-dapp-tests/connect.spec.tsapps/demo-wallet/e2e/mock-dapp-tests/signData.spec.tsapps/demo-wallet/e2e/mock-dapp-tests/signMessage.spec.tsapps/demo-wallet/e2e/mock-dapp-tests/transaction.spec.tsapps/demo-wallet/e2e/mock-dapp/index.htmlapps/demo-wallet/e2e/mock-dapp/main.tsapps/demo-wallet/e2e/mock-dapp/tonconnect-manifest.jsonapps/demo-wallet/e2e/mock-dapp/vite.config.tsapps/demo-wallet/e2e/mocks/walletApi.tsapps/demo-wallet/e2e/pages/AssetsPage.tsapps/demo-wallet/e2e/pages/HistoryPage.tsapps/demo-wallet/e2e/pages/NftPage.tsapps/demo-wallet/e2e/pages/UnlockWalletPage.tsapps/demo-wallet/e2e/pages/index.tsapps/demo-wallet/e2e/qa/WalletApp.tsapps/demo-wallet/e2e/ton-connect/MockDapp.tsapps/demo-wallet/e2e/ton-connect/connect.spec.tsapps/demo-wallet/e2e/ton-connect/mockDappFixture.tsapps/demo-wallet/e2e/ton-connect/twoTabFixture.tsapps/demo-wallet/e2e/ui-tests/UITestFixture.tsapps/demo-wallet/e2e/ui-tests/assets.spec.tsapps/demo-wallet/e2e/ui-tests/dashboard.spec.tsapps/demo-wallet/e2e/ui-tests/formatting.spec.tsapps/demo-wallet/e2e/ui-tests/helpers.tsapps/demo-wallet/e2e/ui-tests/history.spec.tsapps/demo-wallet/e2e/ui-tests/nft.spec.tsapps/demo-wallet/e2e/ui-tests/send.spec.tsapps/demo-wallet/e2e/ui-tests/staking.spec.tsapps/demo-wallet/e2e/ui-tests/swap.spec.tsapps/demo-wallet/e2e/ui-tests/unlock.spec.ts
Comment on lines
+90
to
+97
| # 1) Default config: mock-first ui-tests only (see e2e.config.ts testIgnore). | ||
| # 2) Self-contained two-tab TON Connect mock-dApp suite (starts mock-dApp :5175 + | ||
| # demo-wallet :5173 with broadcast/manifest checks disabled). Aggregate exit codes | ||
| # so both runs execute and both report into allure-results. | ||
| set +e | ||
| xvfb-run pnpm e2e; r1=$? | ||
| xvfb-run pnpm exec playwright test --config e2e.mockdapp.config.ts; r2=$? | ||
| exit $(( r1 || r2 )) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Run the dedicated TON Connect config in CI.
apps/demo-wallet/e2e.config.ts now ignores **/ton-connect/**, but this step only runs the default config plus e2e.mockdapp.config.ts. That leaves apps/demo-wallet/e2e/ton-connect/connect.spec.ts unexecuted in CI.
Suggested change
set +e
xvfb-run pnpm e2e; r1=$?
- xvfb-run pnpm exec playwright test --config e2e.mockdapp.config.ts; r2=$?
- exit $(( r1 || r2 ))
+ xvfb-run pnpm exec playwright test --config e2e.tonconnect.config.ts; r2=$?
+ xvfb-run pnpm exec playwright test --config e2e.mockdapp.config.ts; r3=$?
+ exit $(( r1 || r2 || r3 ))📝 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.
Suggested change
| # 1) Default config: mock-first ui-tests only (see e2e.config.ts testIgnore). | |
| # 2) Self-contained two-tab TON Connect mock-dApp suite (starts mock-dApp :5175 + | |
| # demo-wallet :5173 with broadcast/manifest checks disabled). Aggregate exit codes | |
| # so both runs execute and both report into allure-results. | |
| set +e | |
| xvfb-run pnpm e2e; r1=$? | |
| xvfb-run pnpm exec playwright test --config e2e.mockdapp.config.ts; r2=$? | |
| exit $(( r1 || r2 )) | |
| # 1) Default config: mock-first ui-tests only (see e2e.config.ts testIgnore). | |
| # 2) Self-contained two-tab TON Connect mock-dApp suite (starts mock-dApp :5175 + | |
| # demo-wallet :5173 with broadcast/manifest checks disabled). Aggregate exit codes | |
| # so both runs execute and both report into allure-results. | |
| set +e | |
| xvfb-run pnpm e2e; r1=$? | |
| xvfb-run pnpm exec playwright test --config e2e.tonconnect.config.ts; r2=$? | |
| xvfb-run pnpm exec playwright test --config e2e.mockdapp.config.ts; r3=$? | |
| exit $(( r1 || r2 || r3 )) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/e2e_web.yml around lines 90 - 97, The CI step is missing
the dedicated TON Connect test suite because it only runs the default e2e config
and e2e.mockdapp.config.ts, while e2e.config.ts now excludes ton-connect specs.
Update this workflow step to also invoke the dedicated TON Connect Playwright
config so apps/demo-wallet/e2e/ton-connect/connect.spec.ts runs in CI, and keep
the existing exit-code aggregation behavior in the same shell block.
Comment on lines
+43
to
+45
| // After a successful connect the dApp's widget shows its connected (disconnect) control. | ||
| await expect(widget.connectButton).toBeVisible(); | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Assert a connected-only signal in the approval path.
Line 44 and Line 52 verify the same thing, so the “approve” case can still pass when the dApp never transitions out of its pre-connect state. Please assert a connected-only indicator here (for example, connected wallet text/state) instead of generic button visibility.
Also applies to: 51-52
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/demo-wallet/e2e/ton-connect/connect.spec.ts` around lines 43 - 45, The
approve-path assertion in connect.spec.ts is only checking widget.connectButton
visibility, which is also covered by the other case and can hide a missing
connected transition. Update the test around the approval flow in the
ton-connect connect spec to assert a connected-only signal from the widget after
approval, such as connected wallet text or connected state, using the existing
widget and connectButton-related selectors/methods to locate the right UI state.
…te, harden mock-dApp, readable steps (TON-1701)
- Remove the superseded minter two-tab connect suite + its config: the mock-dApp
suite covers connect, and appkit↔wallet is covered by the minter's own gasless e2e.
- mockdapp config: reject a `localhost` MOCK_DAPP_URL override (WalletKit's manifest
host guard rejects dot-less hosts — fail fast instead of a confusing run).
- mock-dApp connect reject: assert the rejection actually round-trips back to the
dApp (#dapp-error via onStatusChange's error handler) instead of an empty
#dapp-connected, which was also the initial state (false pass).
- Declare @tonconnect/sdk as a demo-wallet devDependency (catalog) and drop the
cross-app Vite alias hack in the mock-dApp fixture — resolve it directly.
- Wrap DemoWallet / helpers flows in named allure.step() so TestOps shows readable
steps ("Approve connect request", …) instead of raw Playwright actions.
…stOps) (TON-1701)
…automation (TON-1701) CodeRabbit round-2 (test quality): - walletApi traces mock honors limit/offset (so pagination/"Load more" advances). - recoverIfBlank waits for the app to actually mount (#root children) after the reload. - assets/dashboard fiat assertions scoped to the row / total widget (not page-wide $). - staking/swap Max assert the exact reserve-adjusted amount (reserve kept), not just non-empty. §18 TON Connect — paste-to-connect / queue / guards (mock-first, two-tab): - guards: request modals (connect/sign-data) are non-dismissible (Esc + backdrop ignored); the Connect-to-dApp paste modal IS dismissible. - queue: two concurrent requests are shown one modal at a time (the second only after the first resolves) — asserts the wallet's request queue sequencing. - paste routing: a clipboard tc:// link auto-routes to a connect request, is suppressed while the paste modal is open, and non-TON clipboard text is ignored. Adds a mock-dApp "two requests" control + allure-wrapped DemoWallet/MockDapp helpers.
…inline ui-test steps - Remove internal test-plan section numbers, tracker IDs and TestOps project numbers from e2e comments and test/describe names (this is a public repo); keep behavior descriptions and repo-relative source code-refs. - Wrap the inline import/create-wallet ui-test flows in named allure.step so the Allure/TestOps execution reads as logical steps, on par with the page objects.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/demo-wallet/e2e/ton-connect/MockDapp.ts (1)
79-93: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMinor: selector inconsistency between
settledCount()andwaitForSettledCount().
settledCount()reads viagetByTestId('dapp-settled-count')whilewaitForSettledCount()resolves the same node viagetElementById('dapp-settled-count'). This works only as long as the element'sdata-testidandidstay identical. Aligning both on one accessor avoids silent breakage if the mock-dApp markup diverges.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/demo-wallet/e2e/ton-connect/MockDapp.ts` around lines 79 - 93, `settledCount()` and `waitForSettledCount()` are using two different selectors for the same `dapp-settled-count` element, which can drift if the mock dApp markup changes. Update `MockDapp` so both methods use the same accessor pattern (either `getByTestId` or `getElementById`) when reading/waiting on the settled-count node, keeping the selector choice consistent within `settledCount()` and `waitForSettledCount()`.apps/demo-wallet/e2e/demo-wallet/DemoWallet.ts (1)
199-208: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winParameterize the payload-specific sign-data assertion.
Text Messageis only the text variant label in the upstream modal implementation. Since this helper is named generically, a bytes/cellsignDatarequest would fail here even if the modal is correct. Either pass the expected payload label intoexpectSignDataModal()or limit this helper to invariant copy only.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/demo-wallet/e2e/demo-wallet/DemoWallet.ts` around lines 199 - 208, The sign-data modal helper is asserting a payload-specific label, which makes the generic `expectSignDataModal()` fail for non-text `signData` requests. Update `DemoWallet.expectSignDataModal()` to either accept the expected payload label as an argument and assert against it, or remove the `Text Message` check so it only verifies invariant modal content. Keep the existing `open()`, `step()`, and `sign-data-request` locator flow intact while adjusting the assertion to match the payload variant.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/demo-wallet/e2e/demo-wallet/DemoWallet.ts`:
- Around line 220-230: Add a brief settle/poll period in waitForOneRequestModal
after Promise.race resolves and before counting visible modals, because the
current immediate snapshot can miss a second modal that appears a tick later.
Update the logic in DemoWallet.waitForOneRequestModal so it waits for visibility
to stabilize, then re-checks app.getByTestId(...).isVisible() for each candidate
before asserting exactly one visible modal and returning the id.
---
Nitpick comments:
In `@apps/demo-wallet/e2e/demo-wallet/DemoWallet.ts`:
- Around line 199-208: The sign-data modal helper is asserting a
payload-specific label, which makes the generic `expectSignDataModal()` fail for
non-text `signData` requests. Update `DemoWallet.expectSignDataModal()` to
either accept the expected payload label as an argument and assert against it,
or remove the `Text Message` check so it only verifies invariant modal content.
Keep the existing `open()`, `step()`, and `sign-data-request` locator flow
intact while adjusting the assertion to match the payload variant.
In `@apps/demo-wallet/e2e/ton-connect/MockDapp.ts`:
- Around line 79-93: `settledCount()` and `waitForSettledCount()` are using two
different selectors for the same `dapp-settled-count` element, which can drift
if the mock dApp markup changes. Update `MockDapp` so both methods use the same
accessor pattern (either `getByTestId` or `getElementById`) when reading/waiting
on the settled-count node, keeping the selector choice consistent within
`settledCount()` and `waitForSettledCount()`.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5b4dd618-efa7-4970-a6d1-9941452577a7
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (25)
apps/demo-wallet/e2e.config.tsapps/demo-wallet/e2e.mockdapp.config.tsapps/demo-wallet/e2e/README.mdapps/demo-wallet/e2e/demo-wallet/DemoWallet.tsapps/demo-wallet/e2e/mock-dapp-tests/connect.spec.tsapps/demo-wallet/e2e/mock-dapp-tests/guards.spec.tsapps/demo-wallet/e2e/mock-dapp-tests/paste-routing.spec.tsapps/demo-wallet/e2e/mock-dapp-tests/queue.spec.tsapps/demo-wallet/e2e/mock-dapp/index.htmlapps/demo-wallet/e2e/mock-dapp/main.tsapps/demo-wallet/e2e/mock-dapp/vite.config.tsapps/demo-wallet/e2e/mocks/walletApi.tsapps/demo-wallet/e2e/qa/WalletApp.tsapps/demo-wallet/e2e/ton-connect/MockDapp.tsapps/demo-wallet/e2e/ton-connect/mockDappFixture.tsapps/demo-wallet/e2e/ui-tests/assets.spec.tsapps/demo-wallet/e2e/ui-tests/dashboard.spec.tsapps/demo-wallet/e2e/ui-tests/formatting.spec.tsapps/demo-wallet/e2e/ui-tests/helpers.tsapps/demo-wallet/e2e/ui-tests/importWallet.spec.tsapps/demo-wallet/e2e/ui-tests/newWallet.spec.tsapps/demo-wallet/e2e/ui-tests/nft.spec.tsapps/demo-wallet/e2e/ui-tests/staking.spec.tsapps/demo-wallet/e2e/ui-tests/swap.spec.tsapps/demo-wallet/package.json
✅ Files skipped from review due to trivial changes (2)
- apps/demo-wallet/e2e/mock-dapp-tests/guards.spec.ts
- apps/demo-wallet/e2e/README.md
🚧 Files skipped from review as they are similar to previous changes (14)
- apps/demo-wallet/e2e/ui-tests/formatting.spec.ts
- apps/demo-wallet/e2e/mock-dapp-tests/connect.spec.ts
- apps/demo-wallet/e2e/mock-dapp/index.html
- apps/demo-wallet/e2e/ui-tests/assets.spec.ts
- apps/demo-wallet/e2e/ui-tests/nft.spec.ts
- apps/demo-wallet/e2e.config.ts
- apps/demo-wallet/e2e/ui-tests/swap.spec.ts
- apps/demo-wallet/e2e/ui-tests/helpers.ts
- apps/demo-wallet/e2e.mockdapp.config.ts
- apps/demo-wallet/e2e/ui-tests/staking.spec.ts
- apps/demo-wallet/e2e/qa/WalletApp.ts
- apps/demo-wallet/e2e/ton-connect/mockDappFixture.ts
- apps/demo-wallet/e2e/ui-tests/dashboard.spec.ts
- apps/demo-wallet/e2e/mocks/walletApi.ts
Comment on lines
+220
to
+230
| async waitForOneRequestModal(testIds: string[]): Promise<string> { | ||
| return await step('Wait for a single request modal to be shown', async () => { | ||
| const app = await this.open(); | ||
| // Resolve as soon as any of the candidate modals is visible. | ||
| await Promise.race(testIds.map((id) => app.getByTestId(id).waitFor({ state: 'visible' }))); | ||
| const visible: string[] = []; | ||
| for (const id of testIds) { | ||
| if (await app.getByTestId(id).isVisible()) visible.push(id); | ||
| } | ||
| expect(visible.length, `exactly one request modal visible, saw: [${visible.join(', ')}]`).toBe(1); | ||
| return visible[0]!; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Add a short settle window before asserting “exactly one” modal.
This helper resolves on the first visible modal and snapshots immediately, so a second request modal that mounts a tick later can slip past the visible.length === 1 check. Since queue specs rely on this helper to prove one-at-a-time rendering, add a brief post-visibility settle/poll before counting visible modals.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/demo-wallet/e2e/demo-wallet/DemoWallet.ts` around lines 220 - 230, Add a
brief settle/poll period in waitForOneRequestModal after Promise.race resolves
and before counting visible modals, because the current immediate snapshot can
miss a second modal that appears a tick later. Update the logic in
DemoWallet.waitForOneRequestModal so it waits for visibility to stabilize, then
re-checks app.getByTestId(...).isVisible() for each candidate before asserting
exactly one visible modal and returning the id.
…den selectors - Bind every ui-test and mock-dApp test to its TestOps case with a pinned @allure.id=<N> title token (a DB-level id that takes precedence over the derived path+title hash), replacing the historyId override — the case now survives a test rename, file move, or describe change. README updated. - Set the authored suite tree (parentSuite / suite / subSuite / feature) and the `automated` tag as labels in the fixtures' beforeEach, and run the reporter with detail:false so the TestOps execution shows only the authored steps. - DemoWallet.connectBy waits for the connect entry to be actionable instead of a fixed pause (the real signal, not a race-masking delay). - dashboard / assets fiat assertions key on data-testid (balance-total / balance-total-int / asset-row / asset-fiat) instead of styling-class selectors.
An explicit allure.step that runs inside a beforeEach or a fixture surfaces as a "Before Hooks" node in the TestOps execution (detail:false only prunes empty hook roots). Keep the arrange out of the hooks so each test reads as its authored steps: - DemoWallet.importWallet and the import / new-wallet screen preconditions no longer wrap their actions in a step — they are fixture/hook setup, not reported steps. - dashboard / send / staking / swap now onboard (and open their feature page) in the test body via createWalletOnDashboard, matching the assets / formatting / nft / history specs; their beforeEach only installs the API mocks.
The unlock specs asserted directly with no allure.step, so they rendered as step-less results. Wrap each behaviour (submit / verify) in a named step so the flow reads as step -> result in TestOps, consistent with the other UI specs.
The mock-first UI specs (dashboard, send, staking, swap, assets, history, nft, formatting) asserted with bare expect() in the body, so in TestOps each showed only the 'Onboard...' arrange step and none of its checks. Wrap each test's behaviour (navigate / verify) in named allure steps so the whole suite reads as step -> result, matching the two-tab and onboarding specs. Purely additive: no selector, assertion, value, or @allure.id changed; expect() stays in the spec (page objects hold no assertions).
There was a problem hiding this comment.
🧹 Nitpick comments (2)
apps/demo-wallet/e2e/ui-tests/UITestFixture.ts (1)
80-90: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated
beforeEachAllure suite-tree hook.This hook is identical to the one in
mockDapFixture.tslines 105-115, differing only in the suite name ('UI'vs'TON Connect'). Extracting a shared helper would prevent divergence.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/demo-wallet/e2e/ui-tests/UITestFixture.ts` around lines 80 - 90, Extract the shared Allure suite-tree setup from extended.beforeEach into a reusable helper, parameterized by the suite name. Update both UITestFixture and mockDapFixture to call the helper with their respective names while preserving the existing parentSuite, subSuite, feature, and automated tag behavior.apps/demo-wallet/e2e/ton-connect/mockDappFixture.ts (1)
105-115: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated
beforeEachAllure suite-tree hook.This hook is identical to the one in
UITestFixture.tslines 80-90, differing only in the suite name ('TON Connect'vs'UI'). Extracting a shared helper (e.g.,applyAuthoredSuiteTree(test, suiteName)) would prevent the two implementations from diverging.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/demo-wallet/e2e/ton-connect/mockDappFixture.ts` around lines 105 - 115, Extract the duplicated Allure suite-tree setup from extended.beforeEach and the corresponding UITestFixture hook into a shared helper such as applyAuthoredSuiteTree, parameterized by the suite name. Update both hooks to call the helper with “TON Connect” and “UI” respectively, preserving the existing describe parsing and tagging behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@apps/demo-wallet/e2e/ton-connect/mockDappFixture.ts`:
- Around line 105-115: Extract the duplicated Allure suite-tree setup from
extended.beforeEach and the corresponding UITestFixture hook into a shared
helper such as applyAuthoredSuiteTree, parameterized by the suite name. Update
both hooks to call the helper with “TON Connect” and “UI” respectively,
preserving the existing describe parsing and tagging behavior.
In `@apps/demo-wallet/e2e/ui-tests/UITestFixture.ts`:
- Around line 80-90: Extract the shared Allure suite-tree setup from
extended.beforeEach into a reusable helper, parameterized by the suite name.
Update both UITestFixture and mockDapFixture to call the helper with their
respective names while preserving the existing parentSuite, subSuite, feature,
and automated tag behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9f55c94e-755e-4112-9be2-4c4f23b779a4
📒 Files selected for processing (28)
apps/demo-wallet/e2e.config.tsapps/demo-wallet/e2e.mockdapp.config.tsapps/demo-wallet/e2e/README.mdapps/demo-wallet/e2e/demo-wallet/DemoWallet.tsapps/demo-wallet/e2e/mock-dapp-tests/connect.spec.tsapps/demo-wallet/e2e/mock-dapp-tests/guards.spec.tsapps/demo-wallet/e2e/mock-dapp-tests/paste-routing.spec.tsapps/demo-wallet/e2e/mock-dapp-tests/queue.spec.tsapps/demo-wallet/e2e/mock-dapp-tests/signData.spec.tsapps/demo-wallet/e2e/mock-dapp-tests/signMessage.spec.tsapps/demo-wallet/e2e/mock-dapp-tests/transaction.spec.tsapps/demo-wallet/e2e/pages/AssetsPage.tsapps/demo-wallet/e2e/ton-connect/mockDappFixture.tsapps/demo-wallet/e2e/ui-tests/UITestFixture.tsapps/demo-wallet/e2e/ui-tests/assets.spec.tsapps/demo-wallet/e2e/ui-tests/dashboard.spec.tsapps/demo-wallet/e2e/ui-tests/formatting.spec.tsapps/demo-wallet/e2e/ui-tests/history.spec.tsapps/demo-wallet/e2e/ui-tests/importWallet.spec.tsapps/demo-wallet/e2e/ui-tests/newWallet.spec.tsapps/demo-wallet/e2e/ui-tests/nft.spec.tsapps/demo-wallet/e2e/ui-tests/send.spec.tsapps/demo-wallet/e2e/ui-tests/setupPassword.spec.tsapps/demo-wallet/e2e/ui-tests/staking.spec.tsapps/demo-wallet/e2e/ui-tests/swap.spec.tsapps/demo-wallet/e2e/ui-tests/unlock.spec.tsapps/demo-wallet/src/features/assets/components/asset-row/asset-row.tsxapps/demo-wallet/src/features/dashboard/components/balance-total/balance-total.tsx
✅ Files skipped from review due to trivial changes (4)
- apps/demo-wallet/src/features/assets/components/asset-row/asset-row.tsx
- apps/demo-wallet/src/features/dashboard/components/balance-total/balance-total.tsx
- apps/demo-wallet/e2e/ui-tests/setupPassword.spec.ts
- apps/demo-wallet/e2e/README.md
🚧 Files skipped from review as they are similar to previous changes (21)
- apps/demo-wallet/e2e/mock-dapp-tests/signData.spec.ts
- apps/demo-wallet/e2e/mock-dapp-tests/transaction.spec.ts
- apps/demo-wallet/e2e/ui-tests/nft.spec.ts
- apps/demo-wallet/e2e.config.ts
- apps/demo-wallet/e2e/mock-dapp-tests/signMessage.spec.ts
- apps/demo-wallet/e2e/ui-tests/history.spec.ts
- apps/demo-wallet/e2e/ui-tests/unlock.spec.ts
- apps/demo-wallet/e2e/mock-dapp-tests/connect.spec.ts
- apps/demo-wallet/e2e/pages/AssetsPage.ts
- apps/demo-wallet/e2e/mock-dapp-tests/guards.spec.ts
- apps/demo-wallet/e2e/ui-tests/newWallet.spec.ts
- apps/demo-wallet/e2e/ui-tests/formatting.spec.ts
- apps/demo-wallet/e2e/mock-dapp-tests/queue.spec.ts
- apps/demo-wallet/e2e/ui-tests/staking.spec.ts
- apps/demo-wallet/e2e/ui-tests/swap.spec.ts
- apps/demo-wallet/e2e/ui-tests/dashboard.spec.ts
- apps/demo-wallet/e2e/ui-tests/assets.spec.ts
- apps/demo-wallet/e2e/mock-dapp-tests/paste-routing.spec.ts
- apps/demo-wallet/e2e/ui-tests/send.spec.ts
- apps/demo-wallet/e2e/demo-wallet/DemoWallet.ts
- apps/demo-wallet/e2e.mockdapp.config.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Mock-first Playwright e2e for the redesigned demo-wallet: a fresh wallet +
page.routemocks over the walletkit API, safe to run on every push (no real wallet, no funds). Every test is pinned to its Allure TestOps case and reads as step → result.What changed
UI-mock specs (
apps/demo-wallet/e2e/ui-tests/) — onboarding (create / import / unlock / setup-password), dashboard, assets, NFT, history, send, swap, staking, amount-formatting, plus page objects and a sharedwalletApimock (Toncenter v3 + rates). Wallet data is stubbed viapage.route; web-only flows opt into awebOnlyfixture and skip in extension mode.TON Connect mock-dApp two-tab suite (
apps/demo-wallet/e2e/mock-dapp-tests/+e2e/mock-dapp/) — a self-contained@tonconnect/sdkmock-dApp (a test fixture, not product code) drives connect / sendTransaction / signData / signMessage over the real bridge, plus paste-to-connect routing, the request queue, and modal-dismissibility guards. Each spec asserts the redesigned modal copy + actions and the protocol response the dApp receives.sendTransactionruns without funds: a mocked balance +disableNetworkSend(the wallet signs and responds to the dApp over the bridge but skips the on-chain broadcast).src.TestOps binding — pinned
@allure.id— every test carries an explicit@allure.id=<N>title token (a DB-level id the adapter parses into theALLURE_IDlabel and strips from the display name). It takes precedence over the derived path+title hash, so a case survives a test rename, file move, or describe change — none of which the default hash binding survives. This replaces the earlierhistoryIdoverride, which only groups the History/Retries timeline and does not affect which case a result attaches to.Report readability — the authored suite tree (
parentSuite/suite/subSuite/feature) and theautomatedtag are set as labels in code; the reporter runs withdetail: false; and every behaviour is wrapped in a namedallure.step. The execution now reads as precondition → named steps (no "Before Hooks" plumbing), consistent across the whole suite.Selectors & waits (hardening) — user-facing role/text and
data-testidare preferred over styling-class / XPath selectors, and the connect flow waits on a real signal instead of a fixed delay. Two demo-wallet components gaineddata-testids so their values can be selected stably:src/features/dashboard/components/balance-total/balance-total.tsx—balance-total/balance-total-intsrc/features/assets/components/asset-row/asset-row.tsx—asset-row/asset-fiatWhy this approach
@allure.idover default /historyIdbinding — a redesign renames and moves specs, and the default hash key orphans the case on exactly those moves; ahistoryIdoverride does not change case matching at all. An explicit id is the only binding robust to rename / move / describe changes, so cases keep their history across future refactors.data-testidin two components, not class selectors — the balance total and the per-asset fiat render no accessible role/text handle, so adata-testidis the stable hook (styling classes change with the design). The additions are attributes only — no product logic changes.How it was verified
disableNetworkSend); the connect handshake and every request ride the real bridge.Scope / safety
apps/demo-wallet/e2e/**and twoapps/demo-wallet/src/**components; thesrcedits aredata-testidattributes only — no behaviour or logic change.e2e/mock-dapp/is a test fixture, not shipped code.Follow-ups
Summary by CodeRabbit