Conversation
Scaffolds dashnote's first Playwright suite: config with chromium-desktop+ chromium-mobile projects, shared fixtures with a pre-connected page and mobile-aware navButton helper, and a smoke spec covering boot/connect, tab navigation, theme toggle, login-modal open/close/validation, mobile hamburger drawer, and the dashnote-lite.html static page. No mnemonic required — read-only flows only. Runs against real testnet, same posture as dashmint-lab. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds an `auth.spec.ts` covering login, logout via the IdentityCard menu, switch-identity, remember-me persistence, and forget-this-device via both the Settings panel and the LoginModal — all gated on PLATFORM_MNEMONIC and run serially against real testnet under both desktop and mobile projects. Adds an `openIdentityMenu` fixture helper that handles the mobile drawer dance, mirroring `navButton`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… fixtures Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR introduces comprehensive Playwright E2E testing infrastructure for the Dashnote example app. It adds a GitHub Actions workflow for automated test execution, Playwright configuration, shared test fixtures and helpers, and test suites covering app boot, identity/authentication, notes CRUD operations, and Settings UI interactions. ChangesDashnote E2E Testing Suite and CI/CD Workflow
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
.github/workflows/dashnote-e2e.yml (2)
100-102: Cross-branch workflow dispatches will queue serially.The fixed concurrency group
dashnote-e2e-read-write(withoutgithub.ref) combined withcancel-in-progress: falsemeans all read-write test runs share a single queue, regardless of which branch or PR triggered them. This is intentional to prevent concurrent writes to the testnet, but it means a manual dispatch from branch A followed by one from branch B will run serially (potential 60+ minute wait).Consider documenting this behavior in the workflow description or in
example-apps/dashnote/CLAUDE.mdto set expectations for contributors usingworkflow_dispatch.🤖 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/dashnote-e2e.yml around lines 100 - 102, The workflow uses a fixed concurrency group "dashnote-e2e-read-write" with cancel-in-progress: false, causing cross-branch workflow_dispatch runs to queue serially; update the docs to explain this behavior (add a short note in the workflow description and in example-apps/dashnote/CLAUDE.md) stating that manual dispatches from different branches will run one-by-one and may incur long waits, and optionally mention the mitigation: either include github.ref in the concurrency group name to isolate queues per branch or keep the single group to enforce serialized testnet writes.
58-58: ⚡ Quick winConsider pinning actions/cache to a commit SHA for consistency.
While using
@v4is acceptable, pinning to a specific commit SHA (likeactions/checkoutandactions/setup-node) improves supply-chain security and ensures reproducible builds.📌 Example pinned version
Check the actions/cache releases for the latest v4 commit SHA:
- uses: actions/cache@v4 + uses: actions/cache@1bd1e32a3bdc45362d1e726936510720a7c30a57 # v4.2.0Apply the same pattern at line 120.
🤖 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/dashnote-e2e.yml at line 58, Replace the floating tag "uses: actions/cache@v4" with a pinned commit SHA reference (e.g., "uses: actions/cache@<commit-sha>") by looking up the latest v4 release commit SHA in the actions/cache GitHub releases and substituting it; ensure the same pinning pattern is applied to other GitHub Action uses in this workflow (notably the entries that currently use tags like actions/checkout and actions/setup-node) so all actions are committed to specific SHAs for reproducible, secure runs.example-apps/dashnote/test/e2e/fixtures.ts (2)
302-309: ⚡ Quick winConsider logging or distinguishing error types in the cleanup try-catch.
The catch block silently continues on any error, which could mask selector changes, UI regressions, or actual bugs. If all candidates fail to load within the 5-second timeout,
targetTitleremains null and cleanup exits early at line 318, leaving notes behind despite the safety counter.Since cleanup is best-effort, consider adding a console.warn or debug log when a candidate fails to help with debugging test flakiness.
🔍 Proposed improvement
try { canonical = await page .getByLabel("Title") .inputValue({ timeout: 5_000 }); } catch { + console.warn(`[cleanupE2eNotes] Failed to read title for candidate ${i}, skipping`); continue; }🤖 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 `@example-apps/dashnote/test/e2e/fixtures.ts` around lines 302 - 309, The catch block around page.getByLabel("Title").inputValue in the cleanup loop currently swallows all errors; change it to log a warning/debug message including the error and some context (e.g., the candidate index or the selector "Title") before continuing so transient timeouts vs real selector/UI regressions can be distinguished—update the try-catch in the cleanup loop that assigns canonical to call console.warn or a test logger with the error and candidate info when an exception occurs.
284-286: ⚡ Quick winConsider logging when the safety counter is exhausted.
The safety counter caps cleanup at 20 notes to prevent runaway loops. If tests consistently leave more than 20 e2e notes, they'll accumulate over time without any signal. Adding a warning when
safetyreaches 0 would improve observability.📊 Proposed logging
let safety = 20; while (safety > 0) { safety -= 1; // ... existing logic ... } + if (safety === 0) { + console.warn('[cleanupE2eNotes] Safety limit reached; some e2e notes may remain'); + }🤖 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 `@example-apps/dashnote/test/e2e/fixtures.ts` around lines 284 - 286, The loop using the safety counter (variable safety in the while (safety > 0) { ... safety -= 1 }) should log a warning when the counter reaches 0 so test authors know cleanup capped out; update the loop to detect when safety would become 0 (after decrement or before breaking) and emit a clear warning message (e.g., via console.warn or the existing test logger) that includes context such as the remaining notes count or which cleanup routine ran, referencing the safety variable and the surrounding cleanup logic in fixtures.ts.
🤖 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 `@example-apps/dashnote/test/e2e/auth.spec.ts`:
- Around line 33-46: The test comment should explain why "Connected" appears
twice after logout: update the comment in the "login with a mnemonic, then
logout via the IdentityCard menu" test to state that IdentityCard renders
"Connected" in two places — the eyebrow label and the status text next to the
connection indicator — so the expect for two occurrences is correct; locate the
comment near the test (references: test name and IdentityCard component) and
replace the existing single-occurrence explanation with a brief note mentioning
both the eyebrow label and the inline status text.
In `@example-apps/dashnote/test/e2e/settings.spec.ts`:
- Around line 40-57: The file-level test.beforeEach (which calls loginViaModal)
is applying to all tests including the readonly suite; move the mnemonic-gated
setup into its own describe block so readonly tests run without auth. Create a
new test.describe(...) for the auth-required specs, and relocate
test.skip(!HAS_MNEMONIC, ...), test.describe.configure({ mode: "serial" }) and
the test.beforeEach(...) (which clears localStorage, reloads, waits for
".conn-dot.connected", and calls loginViaModal) into that describe block,
leaving the readonly tests at top-level unchanged.
---
Nitpick comments:
In @.github/workflows/dashnote-e2e.yml:
- Around line 100-102: The workflow uses a fixed concurrency group
"dashnote-e2e-read-write" with cancel-in-progress: false, causing cross-branch
workflow_dispatch runs to queue serially; update the docs to explain this
behavior (add a short note in the workflow description and in
example-apps/dashnote/CLAUDE.md) stating that manual dispatches from different
branches will run one-by-one and may incur long waits, and optionally mention
the mitigation: either include github.ref in the concurrency group name to
isolate queues per branch or keep the single group to enforce serialized testnet
writes.
- Line 58: Replace the floating tag "uses: actions/cache@v4" with a pinned
commit SHA reference (e.g., "uses: actions/cache@<commit-sha>") by looking up
the latest v4 release commit SHA in the actions/cache GitHub releases and
substituting it; ensure the same pinning pattern is applied to other GitHub
Action uses in this workflow (notably the entries that currently use tags like
actions/checkout and actions/setup-node) so all actions are committed to
specific SHAs for reproducible, secure runs.
In `@example-apps/dashnote/test/e2e/fixtures.ts`:
- Around line 302-309: The catch block around
page.getByLabel("Title").inputValue in the cleanup loop currently swallows all
errors; change it to log a warning/debug message including the error and some
context (e.g., the candidate index or the selector "Title") before continuing so
transient timeouts vs real selector/UI regressions can be distinguished—update
the try-catch in the cleanup loop that assigns canonical to call console.warn or
a test logger with the error and candidate info when an exception occurs.
- Around line 284-286: The loop using the safety counter (variable safety in the
while (safety > 0) { ... safety -= 1 }) should log a warning when the counter
reaches 0 so test authors know cleanup capped out; update the loop to detect
when safety would become 0 (after decrement or before breaking) and emit a clear
warning message (e.g., via console.warn or the existing test logger) that
includes context such as the remaining notes count or which cleanup routine ran,
referencing the safety variable and the surrounding cleanup logic in
fixtures.ts.
🪄 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: b93f2f77-4b2e-4298-aaed-59391a1fb3fd
📒 Files selected for processing (9)
.github/workflows/dashnote-e2e.ymlexample-apps/dashnote/CLAUDE.mdexample-apps/dashnote/package.jsonexample-apps/dashnote/playwright.config.tsexample-apps/dashnote/test/e2e/auth.spec.tsexample-apps/dashnote/test/e2e/fixtures.tsexample-apps/dashnote/test/e2e/notes.spec.tsexample-apps/dashnote/test/e2e/settings.spec.tsexample-apps/dashnote/test/e2e/smoke.spec.ts
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… SHAs Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
example-apps/dashnote/test/e2e/covering every user-facing operation against real Dash Platform testnet. No SDK mocks.smoke,auth,notes,settings) plus a sharedfixtures.tsmirroring the dashmint-lab setup.chromium-desktop(Desktop Chrome) andchromium-mobile(Pixel 7) — same specs cover responsive layout regressions.dashnote-e2e.ymlGitHub Actions workflow with a read-only / read-write split mirroring the existingtest-tutorials.ymlpattern: smoke + settings run on every PR/push touchingexample-apps/dashnote/**; auth + notes areworkflow_dispatch-only to avoid burning testnet credits on routine runs.Coverage
smoke.spec.ts(no auth): SDK boot, tab navigation, theme toggle, login-modal open/close + input validation + WIF preview, mobile hamburger drawer,dashnote-lite.htmlloads.auth.spec.ts(auth-gated, serial): mnemonic login + logout via IdentityCard menu, remember-me persistence across reload, forget-this-device via SettingsPanel AND via LoginModal, switch-identity, Settings tab reachable from both nav button and menu, identity ID displayed after login.notes.spec.ts(auth-gated, serial): create/edit/delete round-trip, search filter (uses persistent[e2e-fixture]notes seeded once), discard-changes confirmation dismissal, mobile list↔editor transitions, two-context concurrent saves (verifies revision bumping across sessions).settings.spec.ts(mixed): readonly sign-in prompt, copy identity ID / contract ID to clipboard, clear local cache (with localStorage assertion), "Use this ID" enable/disable state, DPNS name renders once resolved.Notable design choices
test.skip(testInfo.project.name !== "chromium-mobile", …)rather than in a separate file.[e2e]title prefix and are cleaned up inafterEach. Two persistent[e2e-fixture]notes back the search test so we don't write + delete throwaway notes every run..conn-dot.connectedinstead of text — the readonly IdentityCard renders the "Connected" label twice, breaking strict-mode locators.cleanupE2eNotescallsreturnToListat the top of each loop iteration because the list pane isdisplay: nonewhenever a note is selected, which would otherwise make candidate scans silently return 0 and leak orphans.CI workflow
e2e-read-only(smoke + settings): runs on every push/PR tomainthat touchesexample-apps/dashnote/**or the workflow itself. ~20 min budget.e2e-read-write(auth + notes):workflow_dispatchonly, 60 min budget, concurrency group prevents parallel runs that would conflict on the shared testnet identity.@playwright/testversion, uploadplaywright-report/andtest-results/artifacts on failure (14-day retention).Summary by CodeRabbit