docs: add weekly smoke test cases and prerequisites#772
Conversation
Closes dashpay#771 - Add docs/test-cases.md with 32 test cases covering all implemented user stories from docs/user-stories.md - Add docs/test-prerequisites.md with SPV testnet setup, .env config, bank wallet concept, and cleanup procedures - Test cases are SPV-only (no RPC mode), testnet-only - Structured with Use Case ID, Test Case ID, Pre-Conditions, Steps, Test Data, Expected Result, and Post-Condition - Recommended execution order provided for dependency chain - Covers: WAL, SND, ALK, IDN, DPN, DPY, TOK, DOC, DEV, NET areas
|
@coderabbitai review |
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughTwo new documentation files have been added to define smoke test scenarios and prerequisites. The test-cases.md file specifies comprehensive test coverage across multiple modules (Wallet Management, Send and Receive, Asset Locks, Identity Operations, DPNS, DashPay, Tokens, Contracts, Developer Tools, and Network Settings) in a standardized format. The test-prerequisites.md file outlines shared setup requirements for SPV testnet testing, including environment configuration, .env setup, and test execution guidelines. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes 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 |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
docs/test-cases.md (1)
427-427: Optional: Consider varying sentence structure for readability.The static analysis tool flagged three consecutive test steps beginning with "Verify" (steps 2-4). While this is acceptable in test documentation and follows standard testing terminology, you could optionally vary the phrasing for slightly improved readability.
✨ Optional rewording suggestion
-| **Test Steps** | 1. Navigate to Platform info screen 2. Verify epoch info is displayed 3. Verify total credits shown 4. Verify validator list visible | +| **Test Steps** | 1. Navigate to Platform info screen 2. Verify epoch info is displayed 3. Check that total credits are shown 4. Confirm validator list is visible |🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/test-cases.md` at line 427, The three consecutive test steps in the "Test Steps" table row (steps 2–4) all start with "Verify" which is repetitive; update that table cell so the steps vary phrasing while keeping meaning — e.g. keep step 1 as "Navigate to Platform info screen", change step 2 to "Confirm epoch info is displayed", step 3 to "Check total credits are shown", and step 4 to "Ensure validator list is visible" (or similar synonyms) to improve readability without altering test intent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/test-prerequisites.md`:
- Around line 66-67: Update the listed config directory paths so the Linux entry
matches the macOS casing and add Windows paths: change the Linux entry string
`~/.config/dash-evo-tool/` to use capital D (`~/.config/Dash-Evo-Tool/`) to
match `Dash-Evo-Tool`, and add one or two Windows entries (e.g.,
`%APPDATA%\Dash-Evo-Tool\` and `C:\Users\<User>\AppData\Roaming\Dash-Evo-Tool\`)
alongside the existing macOS line; ensure you edit the same block that contains
the `**macOS:**` and `**Linux:**` lines so formatting/markdown stays consistent.
- Around line 20-25: Update the docs to clarify that although SPV mode does not
use Core RPC at runtime, the NetworkConfig struct requires core_host,
core_rpc_port, core_rpc_user, core_rpc_password, and insight_api_url to be
present during deserialization; state that core_zmq_endpoint is optional
(Option<String>) and that users should supply placeholder values for the
required Core RPC fields in their .env (or .env.example) when running in SPV
mode and keep the TESTNET_dapi_addresses example as shown.
- Around line 27-35: Update the docs to reflect the correct UI path for setting
Core backend mode: replace "Settings → Core Backend Mode" with instructions to
open the Network Chooser screen, go to "Connection Settings" and select the
"Connection Type" dropdown (visible only when Developer Mode is enabled) and
choose "SPV"; also mention that the selection is stored on the backend in the
Settings struct and persisted to the database as core_backend_mode so readers
know where the value is saved.
---
Nitpick comments:
In `@docs/test-cases.md`:
- Line 427: The three consecutive test steps in the "Test Steps" table row
(steps 2–4) all start with "Verify" which is repetitive; update that table cell
so the steps vary phrasing while keeping meaning — e.g. keep step 1 as "Navigate
to Platform info screen", change step 2 to "Confirm epoch info is displayed",
step 3 to "Check total credits are shown", and step 4 to "Ensure validator list
is visible" (or similar synonyms) to improve readability without altering test
intent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e9a3225d-8762-4ca0-9095-6ea1e449bb83
📒 Files selected for processing (2)
docs/test-cases.mddocs/test-prerequisites.md
| ```env | ||
| # Testnet SPV mode — no Core RPC needed | ||
| TESTNET_dapi_addresses=https://34.214.48.68:1443,https://52.12.176.90:1443,https://52.34.144.50:1443,https://44.240.98.102:1443,https://54.201.32.131:1443,https://52.10.229.11:1443,https://52.13.132.146:1443,https://52.40.219.41:1443,https://54.149.33.167:1443,https://35.164.23.245:1443,https://52.33.28.47:1443,https://52.43.13.92:1443,https://52.89.154.48:1443,https://52.24.124.162:1443,https://35.85.21.179:1443,https://54.187.14.232:1443,https://54.68.235.201:1443,https://52.13.250.182:1443 | ||
| ``` | ||
|
|
||
| No `core_host`, `core_rpc_port`, `core_rpc_user`, `core_rpc_password`, or `core_zmq_endpoint` settings are needed for SPV mode. |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify NetworkConfig field requirements and .env.example structure
echo "=== NetworkConfig struct fields ==="
ast-grep --pattern 'pub struct NetworkConfig {
$$$
}'
echo -e "\n=== TESTNET config in .env.example ==="
rg -A 20 'TESTNET_dapi_addresses' .env.exampleRepository: dashpay/dash-evo-tool
Length of output: 2764
🏁 Script executed:
#!/bin/bash
echo "=== Checking for SPV mode handling ==="
rg -i "spv" src/ --type rust -B 2 -A 2
echo -e "\n=== Checking if core_host is actually used in code ==="
rg "core_host" src/ --type rust -B 1 -A 1
echo -e "\n=== Checking for conditional Core RPC logic ==="
rg "core_rpc" src/ --type rust -B 1 -A 1 | head -50Repository: dashpay/dash-evo-tool
Length of output: 50378
Update documentation to clarify that Core RPC config fields are structurally required.
The documentation claims Core RPC fields are "not needed for SPV mode," but the NetworkConfig struct requires core_host, core_rpc_port, core_rpc_user, core_rpc_password, and insight_api_url as non-optional fields. This means .env deserialization will fail if these fields are omitted, regardless of whether SPV mode is active.
Note: core_zmq_endpoint is optional (Option<String>), so only the five fields listed above are structurally required.
The .env.example file correctly includes these fields for TESTNET, confirming they must be present. While SPV mode does provide an alternative backend that doesn't actively use Core RPC at runtime, the configuration structure still mandates their presence.
Fix: Clarify the documentation to state: "While Core RPC settings are required in the configuration struct, they are not actively used when operating in SPV mode. Use placeholder values if needed."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/test-prerequisites.md` around lines 20 - 25, Update the docs to clarify
that although SPV mode does not use Core RPC at runtime, the NetworkConfig
struct requires core_host, core_rpc_port, core_rpc_user, core_rpc_password, and
insight_api_url to be present during deserialization; state that
core_zmq_endpoint is optional (Option<String>) and that users should supply
placeholder values for the required Core RPC fields in their .env (or
.env.example) when running in SPV mode and keep the TESTNET_dapi_addresses
example as shown.
| ### Backend mode | ||
|
|
||
| After launching DET, set the Core backend mode to **SPV**: | ||
|
|
||
| 1. Navigate to Settings → Core Backend Mode | ||
| 2. Select **SPV** | ||
| 3. Wait for SPV peers to connect and sync (status indicator turns green) | ||
|
|
||
| > **Note:** SPV mode is not the default as of v1.0.0-dev. It must be explicitly selected in settings. |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for backend mode or SPV mode selection logic
echo "=== Searching for backend mode config ==="
rg -n 'backend.*mode|spv.*mode|core.*backend' --type rust -i
echo -e "\n=== Searching for Settings struct ==="
ast-grep --pattern 'struct Settings {
$$$
}'
echo -e "\n=== SpvProvider references ==="
rg -n 'SpvProvider' --type rust -C 2Repository: dashpay/dash-evo-tool
Length of output: 29532
🏁 Script executed:
# Check NetworkConfig struct definition
rg -n 'struct NetworkConfig' --type rust -A 15
# Search for Settings screen UI definition
rg -n 'Settings.*screen|settings.*screen' --type rust -i | head -20
# Verify if backend mode selection happens in network chooser vs settings
rg -n 'backend_mode' src/ui/network_chooser_screen.rs | head -5Repository: dashpay/dash-evo-tool
Length of output: 2859
🏁 Script executed:
# Look for screen navigation and backend mode UI implementation
rg -n 'Core Backend Mode|backend.*mode' src/ui/network_chooser_screen.rs -B2 -A2 | head -50
# Search for any Settings screen that might contain backend mode
rg -n 'core_backend_mode' src/ui/ --type rust -l
# Check the network chooser screen structure
head -100 src/ui/network_chooser_screen.rsRepository: dashpay/dash-evo-tool
Length of output: 6109
🏁 Script executed:
# Search for Settings screen or panel that might offer backend mode selection
rg -n 'settings.*screen|settings.*panel|Settings' src/ui/ --type rust -i | grep -i 'screen\|panel' | head -20
# Check if there's any reference to backend mode in UI outside network_chooser_screen
rg -n 'core_backend_mode' src/ui/ --type rust | grep -v network_chooser_screen
# Look at the actual navigation/menu structure
rg -n 'struct.*Screen|enum.*Screen' src/ui/ --type rust | head -30Repository: dashpay/dash-evo-tool
Length of output: 4558
🏁 Script executed:
# Look for the actual UI rendering of backend mode selection
sed -n '192,300p' src/ui/network_chooser_screen.rs
# Check what the "Connection Settings Card" shows
rg -n 'Connection Settings|backend.*mode.*button|SPV.*toggle' src/ui/network_chooser_screen.rs -A 5 -B 2Repository: dashpay/dash-evo-tool
Length of output: 6171
The documented navigation path is inaccurate.
Backend mode selection is not in "Settings → Core Backend Mode" as documented. Users will find it in the Network Chooser screen under "Connection Settings" as a dropdown selector labeled "Connection Type" (shown only when Developer Mode is enabled). The field is correctly stored in the Settings struct and persisted to the database as core_backend_mode. Update the documentation to reflect the actual UI location.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/test-prerequisites.md` around lines 27 - 35, Update the docs to reflect
the correct UI path for setting Core backend mode: replace "Settings → Core
Backend Mode" with instructions to open the Network Chooser screen, go to
"Connection Settings" and select the "Connection Type" dropdown (visible only
when Developer Mode is enabled) and choose "SPV"; also mention that the
selection is stored on the backend in the Settings struct and persisted to the
database as core_backend_mode so readers know where the value is saved.
| - **macOS:** `~/Library/Application Support/Dash-Evo-Tool/` | ||
| - **Linux:** `~/.config/dash-evo-tool/` |
There was a problem hiding this comment.
Same path issues: incorrect Linux casing and missing Windows.
The same path casing issue and missing Windows documentation apply here as well. The Linux path should use capital D (Dash-Evo-Tool), and Windows paths should be included.
📝 Proposed fix
- **macOS:** `~/Library/Application Support/Dash-Evo-Tool/`
- - **Linux:** `~/.config/dash-evo-tool/`
+ - **Linux:** `~/.config/Dash-Evo-Tool/`
+ - **Windows:** `C:\Users\{username}\AppData\Roaming\Dash-Evo-Tool\`📝 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.
| - **macOS:** `~/Library/Application Support/Dash-Evo-Tool/` | |
| - **Linux:** `~/.config/dash-evo-tool/` | |
| - **macOS:** `~/Library/Application Support/Dash-Evo-Tool/` | |
| - **Linux:** `~/.config/Dash-Evo-Tool/` | |
| - **Windows:** `C:\Users\{username}\AppData\Roaming\Dash-Evo-Tool\` |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/test-prerequisites.md` around lines 66 - 67, Update the listed config
directory paths so the Linux entry matches the macOS casing and add Windows
paths: change the Linux entry string `~/.config/dash-evo-tool/` to use capital D
(`~/.config/Dash-Evo-Tool/`) to match `Dash-Evo-Tool`, and add one or two
Windows entries (e.g., `%APPDATA%\Dash-Evo-Tool\` and
`C:\Users\<User>\AppData\Roaming\Dash-Evo-Tool\`) alongside the existing macOS
line; ensure you edit the same block that contains the `**macOS:**` and
`**Linux:**` lines so formatting/markdown stays consistent.
- Add exact button labels, field names, screen navigation - Implement per-test mnemonic strategy with placeholder variables - Add password-protected bank wallet requirement - Add setup/cleanup fund transfer procedures - Add secrets file specification for test mnemonics Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Clarify Core RPC config fields are structurally required by NetworkConfig even in SPV mode; add placeholder values to .env example - Fix UI navigation path: SPV backend is selected via Network Chooser screen Connection Type dropdown, not Settings - Rename 'Expert mode' to 'Developer mode' to match actual UI - Fix Linux config path casing to match macOS (Dash-Evo-Tool) - Remove spurious /config/ segment from Windows .env path - Vary verb choices in TC-DEV-005-01 test steps for readability
…ables Per lklimek's feedback: - Change secrets file path from ~/det-qa-secrets.env to ~/.secrets/det-qa-mnemonics.env - Add all 10 test mnemonic variables with word counts (24/21/18/15/12) - Add TC_TOK, TC_DPY, TC_DOC, TC_DEV, TC_NET, TC_ALK mnemonics
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
Docs-only PR with 32 smoke test cases. Well-structured but has naming inconsistencies between documents and a missing required env variable in the sample config.
Reviewed commit: 0b10731
🟡 2 suggestion(s)
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `docs/test-cases.md`:
- [SUGGESTION] lines 40-42: UI label mismatch: 'Expert mode' should be 'Developer mode'
test-cases.md still uses 'Expert mode' and 'Core backend mode' labels that were corrected to 'Developer mode' and 'Connection Type' in test-prerequisites.md but not propagated back.
In `docs/test-prerequisites.md`:
- [SUGGESTION] lines 78-92: Sample .env missing TESTNET_show_in_ui required field
NetworkConfig has a required `show_in_ui: bool` field. The sample config omits it, which would cause config loading to fail via envy deserialization.
| ### 2. Import the Bank Wallet | ||
|
|
||
| 1. Click **"Wallets"** in the left sidebar. |
There was a problem hiding this comment.
🟡 Suggestion: UI label mismatch: 'Expert mode' should be 'Developer mode'
test-cases.md still uses 'Expert mode' and 'Core backend mode' labels that were corrected to 'Developer mode' and 'Connection Type' in test-prerequisites.md but not propagated back.
source: ['claude-general']
🤖 Fix this with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `docs/test-cases.md`:
- [SUGGESTION] lines 40-42: UI label mismatch: 'Expert mode' should be 'Developer mode'
test-cases.md still uses 'Expert mode' and 'Core backend mode' labels that were corrected to 'Developer mode' and 'Connection Type' in test-prerequisites.md but not propagated back.
| ```env | ||
| # Testnet SPV mode — DAPI addresses plus placeholder Core RPC fields | ||
| TESTNET_dapi_addresses=https://34.214.48.68:1443,https://52.12.176.90:1443,https://52.34.144.50:1443,https://44.240.98.102:1443,https://54.201.32.131:1443,https://52.10.229.11:1443,https://52.13.132.146:1443,https://52.40.219.41:1443,https://54.149.33.167:1443,https://35.164.23.245:1443,https://52.33.28.47:1443,https://52.43.13.92:1443,https://52.89.154.48:1443,https://52.24.124.162:1443,https://35.85.21.179:1443,https://54.187.14.232:1443,https://54.68.235.201:1443,https://52.13.250.182:1443 | ||
|
|
||
| # Core RPC fields are structurally required by NetworkConfig even in SPV mode, | ||
| # but are not actively used at runtime. Supply placeholder values. | ||
| TESTNET_core_host=127.0.0.1 | ||
| TESTNET_core_rpc_port=19998 | ||
| TESTNET_core_rpc_user=user | ||
| TESTNET_core_rpc_password=password | ||
| TESTNET_insight_api_url=https://insight.testnet.networks.dash.org:3002/insight-api | ||
| # core_zmq_endpoint is optional and can be omitted | ||
| ``` | ||
|
|
||
| > **Note:** While Core RPC settings (`core_host`, `core_rpc_port`, `core_rpc_user`, `core_rpc_password`, `insight_api_url`) are not actively used in SPV mode, the `NetworkConfig` struct requires them to be present during deserialization. Use placeholder values as shown above. The `core_zmq_endpoint` field is optional (`Option<String>`) and may be omitted. |
There was a problem hiding this comment.
🟡 Suggestion: Sample .env missing TESTNET_show_in_ui required field
NetworkConfig has a required show_in_ui: bool field. The sample config omits it, which would cause config loading to fail via envy deserialization.
source: ['codex-general']
🤖 Fix this with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `docs/test-prerequisites.md`:
- [SUGGESTION] lines 78-92: Sample .env missing TESTNET_show_in_ui required field
NetworkConfig has a required `show_in_ui: bool` field. The sample config omits it, which would cause config loading to fail via envy deserialization.
|
✅ Review complete (commit 6dada97) |
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
Documentation-only PR adding weekly smoke test cases and prerequisites. Multiple blocking issues will prevent QA from executing the suite as written: Session Setup and TC-NET-005-01 reference UI labels that do not exist ("Expert mode", "Core backend mode", a non-existent "Save" for backend selection), the Linux config path is wrong (case-sensitive), the example .env omits the required TESTNET_show_in_ui field, and several send flows skip the in-form Send click that opens the fee dialog. The two new docs also contradict each other on the SPV setup. Three suggestions improve test reliability; one nitpick noted.
Reviewed commit: 0b10731
🔴 6 blocking | 🟡 3 suggestion(s) | 💬 1 nitpick(s)
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `docs/test-cases.md`:
- [BLOCKING] lines 32-39: Session Setup uses UI labels that don't exist ("Expert mode", "Core backend mode", a Save step)
Steps 2–4 tell testers to check an "Expert mode" checkbox and click "Save", then pick "SPV Client" under "Core backend mode" and click "Save" again. None of those labels exist in the current build:
- The checkbox is **"Developer mode"** (`src/ui/network_chooser_screen.rs:936`), not "Expert mode".
- The selector is labeled **"Connection Type:"** (`src/ui/network_chooser_screen.rs:219`), not "Core backend mode". Options are "SPV Client" / "Dash Core RPC".
- The selector lives on the **Network Chooser** screen under a "Connection Settings" section, not in Settings.
- There is no "Save" step for the backend mode — `set_core_backend_mode` is called immediately on `selectable_value` change (`network_chooser_screen.rs:244`, `:257`). The "Save" button on the screen (line 397) only persists RPC settings.
This also directly contradicts `docs/test-prerequisites.md:96-113`, which already documents the correct "Developer mode" / "Connection Type" wording. Because this is the gate for the entire run, testers will be stuck before any test case begins.
- [BLOCKING] lines 131-142: TC-NET-005-01 references "Expert mode" and "Core backend mode" — neither label exists
The case title, description, expected result, and steps 2/3/7 all reference an "Expert mode" checkbox; the actual label rendered by `StyledCheckbox::new(&mut self.developer_mode, "Developer mode")` (`src/ui/network_chooser_screen.rs:936`) is "Developer mode". Step 4 expects a "Core backend mode" selector to appear, but the actual label is "Connection Type:" (line 219). Rename throughout to match the UI and align with the wording already used in `test-prerequisites.md:96-113`.
- [BLOCKING] lines 72-83: Send flows skip the in-form "Send" click that opens the fee confirmation dialog
In Session Setup steps 4–8, TC-SND-005-01 steps 4–7, TC-SND-001-01 steps 5–8, and Session Cleanup, the instructions go straight from filling "To:"/"Amount:" to "A 'Fee Confirmation Required' dialog appears". The dialog is only triggered by clicking the in-form `Send` button in `render_send_button()` (`src/ui/wallets/single_key_send_screen.rs:812-853`), which calls `validate_and_send`. Without that explicit click, the tester remains on the compose form and never reaches the dialog. Add an explicit "Click Send (in the form)" step before each "Fee Confirmation Required" line.
- [SUGGESTION] lines 504-507: TC-TOK-003-01 expects a token to appear in "My Tokens" but the screen is built from owned balances
The expected result requires the added token to appear in **My Tokens** with a name and balance. The implementation does save the token locally and run `QueryMyTokenBalances` (`src/ui/tokens/tokens_screen/mod.rs:2511-2520`), but `my_tokens()` (`src/ui/tokens/tokens_screen/mod.rs:1472-1502`) is built from `identity_token_balances()` — only tokens the local identities actually hold. If the test identity holds zero of the added token, the screen falls back to the "No Tracked Tokens" empty state (`src/ui/tokens/tokens_screen/my_tokens.rs:226-229`) and the test fails despite add-by-ID working. Either pre-fund a token fixture against the test identity, or change the expected result to verify successful tracking via the Search Tokens / contract list rather than balance visibility.
- [SUGGESTION] lines 325-337: TC-ALK-001-01 step 3 is too vague — "navigate via the identity registration flow or wallet tools"
Step 3 reads "Navigate to asset lock creation (via the identity registration flow or wallet tools)." Every other step in this PR names the exact sidebar item, button, and dialog labels (e.g., TC-WAL-002-01, TC-DPY-001-01). This step asks the tester to guess between two paths, so two testers will execute the same case differently — defeating the smoke-test spec. Spell out the exact sidebar item, button labels, and dialog title for the asset-lock creation entry point used by the identity registration flow.
- [SUGGESTION] lines 342-353: TC-IDN-001-01 navigation/steps are too generic for a smoke-test spec
Steps 2 and 4 read "Navigate to identity creation (breadcrumb: Identities > Create Identity)" and "Follow the multi-stage confirmation flow." Compared with TC-WAL-002-01 or TC-DPY-001-01, this case provides almost no guidance for the multi-step identity registration flow (asset-lock selection, key generation, registration confirmation). Either expand into named steps/buttons, or list the intermediate UI states (e.g., asset-lock selection screen, key-generation summary, registration confirmation) the tester must verify, so two testers produce comparable results.
In `docs/test-prerequisites.md`:
- [BLOCKING] line 75: Wrong Linux config path — must be lowercase `dash-evo-tool`
The `.env` location is documented as `~/.config/Dash-Evo-Tool/.env`, but `directories::ProjectDirs` lower-cases the application name on Linux, so the runtime path is `~/.config/dash-evo-tool/.env`. The bundled `.env.example:2` and `README.md` both confirm this. A Linux tester writing the `.env` to the documented path will silently have it ignored, and the entire smoke run will start with no testnet configuration.
- [BLOCKING] line 163: Same Linux path bug in Fresh Start cleanup
`Linux: ~/.config/Dash-Evo-Tool/` is the wrong case; the actual data dir is `~/.config/dash-evo-tool/`. A Linux tester following the Fresh Start steps will believe they wiped DET state while the real config dir remains untouched, masking onboarding-test bugs in TC-NET-010-01.
- [BLOCKING] lines 78-90: Sample `.env` omits `TESTNET_show_in_ui`, which `NetworkConfig` requires
`NetworkConfig::show_in_ui` is a non-optional `bool` (`src/config.rs:50`), and the bundled `.env.example:21` includes `TESTNET_show_in_ui=true` for that reason. The new sample omits it, so `envy` will fail to deserialize the testnet config and `Config::load()` will not produce a usable testnet entry. A tester who copies the snippet verbatim cannot start the smoke run. Add `TESTNET_show_in_ui=true` to the example block.
| ### 1. Configure SPV Mode | ||
|
|
||
| 1. Click **"Settings"** in the left sidebar. | ||
| 2. Check the **"Expert mode"** checkbox and click **"Save"**. | ||
| 3. Under the now-visible **Core backend mode**, select **"SPV Client"**. | ||
| 4. Click **"Save"**. | ||
| 5. Wait for the connection status indicator (top bar) to turn green. | ||
|
|
There was a problem hiding this comment.
🔴 Blocking: Session Setup uses UI labels that don't exist ("Expert mode", "Core backend mode", a Save step)
Steps 2–4 tell testers to check an "Expert mode" checkbox and click "Save", then pick "SPV Client" under "Core backend mode" and click "Save" again. None of those labels exist in the current build:
- The checkbox is "Developer mode" (
src/ui/network_chooser_screen.rs:936), not "Expert mode". - The selector is labeled "Connection Type:" (
src/ui/network_chooser_screen.rs:219), not "Core backend mode". Options are "SPV Client" / "Dash Core RPC". - The selector lives on the Network Chooser screen under a "Connection Settings" section, not in Settings.
- There is no "Save" step for the backend mode —
set_core_backend_modeis called immediately onselectable_valuechange (network_chooser_screen.rs:244,:257). The "Save" button on the screen (line 397) only persists RPC settings.
This also directly contradicts docs/test-prerequisites.md:96-113, which already documents the correct "Developer mode" / "Connection Type" wording. Because this is the gate for the entire run, testers will be stuck before any test case begins.
source: ['claude', 'codex']
🤖 Fix this with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `docs/test-cases.md`:
- [BLOCKING] lines 32-39: Session Setup uses UI labels that don't exist ("Expert mode", "Core backend mode", a Save step)
Steps 2–4 tell testers to check an "Expert mode" checkbox and click "Save", then pick "SPV Client" under "Core backend mode" and click "Save" again. None of those labels exist in the current build:
- The checkbox is **"Developer mode"** (`src/ui/network_chooser_screen.rs:936`), not "Expert mode".
- The selector is labeled **"Connection Type:"** (`src/ui/network_chooser_screen.rs:219`), not "Core backend mode". Options are "SPV Client" / "Dash Core RPC".
- The selector lives on the **Network Chooser** screen under a "Connection Settings" section, not in Settings.
- There is no "Save" step for the backend mode — `set_core_backend_mode` is called immediately on `selectable_value` change (`network_chooser_screen.rs:244`, `:257`). The "Save" button on the screen (line 397) only persists RPC settings.
This also directly contradicts `docs/test-prerequisites.md:96-113`, which already documents the correct "Developer mode" / "Connection Type" wording. Because this is the gate for the entire run, testers will be stuck before any test case begins.
| ### TC-NET-005-01: Toggle Expert mode | ||
|
|
||
| | Field | Value | | ||
| |-------|-------| | ||
| | **Use Case ID** | NET-005 | | ||
| | **Test Case ID** | TC-NET-005-01 | | ||
| | **Short Description** | Enable and disable Expert mode | | ||
| | **Pre-Conditions** | DET running | | ||
| | **Test Steps** | 1. Click **"Settings"** in the left sidebar.<br>2. Locate the **"Expert mode"** checkbox.<br>3. If unchecked, check **"Expert mode"** and click **"Save"**.<br>4. Verify the **Core backend mode** selector appears (shows **"SPV Client"** or **"Dash Core RPC"**).<br>5. Navigate to **"Wallets"** — verify additional developer elements appear (e.g., address tables, refresh controls).<br>6. Return to **"Settings"**.<br>7. Uncheck **"Expert mode"** and click **"Save"**.<br>8. Verify the **Core backend mode** selector disappears.<br>9. Navigate to **"Wallets"** — verify developer elements are hidden. | | ||
| | **Test Data** | N/A | | ||
| | **Expected Result** | Checking **"Expert mode"** reveals advanced controls (backend mode selector, address tables). Unchecking hides them. | | ||
| | **Post-Condition** | Re-enable **"Expert mode"** and click **"Save"** (required for SPV backend selection). | |
There was a problem hiding this comment.
🔴 Blocking: TC-NET-005-01 references "Expert mode" and "Core backend mode" — neither label exists
The case title, description, expected result, and steps 2/3/7 all reference an "Expert mode" checkbox; the actual label rendered by StyledCheckbox::new(&mut self.developer_mode, "Developer mode") (src/ui/network_chooser_screen.rs:936) is "Developer mode". Step 4 expects a "Core backend mode" selector to appear, but the actual label is "Connection Type:" (line 219). Rename throughout to match the UI and align with the wording already used in test-prerequisites.md:96-113.
source: ['claude']
🤖 Fix this with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `docs/test-cases.md`:
- [BLOCKING] lines 131-142: TC-NET-005-01 references "Expert mode" and "Core backend mode" — neither label exists
The case title, description, expected result, and steps 2/3/7 all reference an "Expert mode" checkbox; the actual label rendered by `StyledCheckbox::new(&mut self.developer_mode, "Developer mode")` (`src/ui/network_chooser_screen.rs:936`) is "Developer mode". Step 4 expects a "Core backend mode" selector to appear, but the actual label is "Connection Type:" (line 219). Rename throughout to match the UI and align with the wording already used in `test-prerequisites.md:96-113`.
| | Platform | Path | | ||
| |---|---| | ||
| | **macOS** | `~/Library/Application Support/Dash-Evo-Tool/.env` | | ||
| | **Linux** | `~/.config/Dash-Evo-Tool/.env` | |
There was a problem hiding this comment.
🔴 Blocking: Wrong Linux config path — must be lowercase dash-evo-tool
The .env location is documented as ~/.config/Dash-Evo-Tool/.env, but directories::ProjectDirs lower-cases the application name on Linux, so the runtime path is ~/.config/dash-evo-tool/.env. The bundled .env.example:2 and README.md both confirm this. A Linux tester writing the .env to the documented path will silently have it ignored, and the entire smoke run will start with no testnet configuration.
💡 Suggested change
| | **Linux** | `~/.config/Dash-Evo-Tool/.env` | | |
| | **Linux** | `~/.config/dash-evo-tool/.env` | |
source: ['claude']
🤖 Fix this with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `docs/test-prerequisites.md`:
- [BLOCKING] line 75: Wrong Linux config path — must be lowercase `dash-evo-tool`
The `.env` location is documented as `~/.config/Dash-Evo-Tool/.env`, but `directories::ProjectDirs` lower-cases the application name on Linux, so the runtime path is `~/.config/dash-evo-tool/.env`. The bundled `.env.example:2` and `README.md` both confirm this. A Linux tester writing the `.env` to the documented path will silently have it ignored, and the entire smoke run will start with no testnet configuration.
| 1. Quit DET completely. | ||
| 2. Remove or rename the data directory: | ||
| - **macOS:** `~/Library/Application Support/Dash-Evo-Tool/` | ||
| - **Linux:** `~/.config/Dash-Evo-Tool/` |
There was a problem hiding this comment.
🔴 Blocking: Same Linux path bug in Fresh Start cleanup
Linux: ~/.config/Dash-Evo-Tool/ is the wrong case; the actual data dir is ~/.config/dash-evo-tool/. A Linux tester following the Fresh Start steps will believe they wiped DET state while the real config dir remains untouched, masking onboarding-test bugs in TC-NET-010-01.
💡 Suggested change
| - **Linux:** `~/.config/Dash-Evo-Tool/` | |
| - **Linux:** `~/.config/dash-evo-tool/` |
source: ['claude']
🤖 Fix this with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `docs/test-prerequisites.md`:
- [BLOCKING] line 163: Same Linux path bug in Fresh Start cleanup
`Linux: ~/.config/Dash-Evo-Tool/` is the wrong case; the actual data dir is `~/.config/dash-evo-tool/`. A Linux tester following the Fresh Start steps will believe they wiped DET state while the real config dir remains untouched, masking onboarding-test bugs in TC-NET-010-01.
| ```env | ||
| # Testnet SPV mode — DAPI addresses plus placeholder Core RPC fields | ||
| TESTNET_dapi_addresses=https://34.214.48.68:1443,https://52.12.176.90:1443,https://52.34.144.50:1443,https://44.240.98.102:1443,https://54.201.32.131:1443,https://52.10.229.11:1443,https://52.13.132.146:1443,https://52.40.219.41:1443,https://54.149.33.167:1443,https://35.164.23.245:1443,https://52.33.28.47:1443,https://52.43.13.92:1443,https://52.89.154.48:1443,https://52.24.124.162:1443,https://35.85.21.179:1443,https://54.187.14.232:1443,https://54.68.235.201:1443,https://52.13.250.182:1443 | ||
|
|
||
| # Core RPC fields are structurally required by NetworkConfig even in SPV mode, | ||
| # but are not actively used at runtime. Supply placeholder values. | ||
| TESTNET_core_host=127.0.0.1 | ||
| TESTNET_core_rpc_port=19998 | ||
| TESTNET_core_rpc_user=user | ||
| TESTNET_core_rpc_password=password | ||
| TESTNET_insight_api_url=https://insight.testnet.networks.dash.org:3002/insight-api | ||
| # core_zmq_endpoint is optional and can be omitted | ||
| ``` |
There was a problem hiding this comment.
🔴 Blocking: Sample .env omits TESTNET_show_in_ui, which NetworkConfig requires
NetworkConfig::show_in_ui is a non-optional bool (src/config.rs:50), and the bundled .env.example:21 includes TESTNET_show_in_ui=true for that reason. The new sample omits it, so envy will fail to deserialize the testnet config and Config::load() will not produce a usable testnet entry. A tester who copies the snippet verbatim cannot start the smoke run. Add TESTNET_show_in_ui=true to the example block.
source: ['codex']
🤖 Fix this with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `docs/test-prerequisites.md`:
- [BLOCKING] lines 78-90: Sample `.env` omits `TESTNET_show_in_ui`, which `NetworkConfig` requires
`NetworkConfig::show_in_ui` is a non-optional `bool` (`src/config.rs:50`), and the bundled `.env.example:21` includes `TESTNET_show_in_ui=true` for that reason. The new sample omits it, so `envy` will fail to deserialize the testnet config and `Config::load()` will not produce a usable testnet entry. A tester who copies the snippet verbatim cannot start the smoke run. Add `TESTNET_show_in_ui=true` to the example block.
| 1. Select the **Bank** wallet on the **"Wallets"** screen. | ||
| 2. If the wallet shows **"Unlock"** instead of **"Lock"**, click **"Unlock"**, enter `${BANK_PASSWORD}` in the popup, and click **"Unlock"**. | ||
| 3. Click **"Send"**. | ||
| 4. In the **"To:"** field (hint: *"Enter Dash address"*), enter `${TC_SND_ADDRESS_0}`. | ||
| 5. In the **"Amount:"** field (hint: *"0.00"*), enter `1`. | ||
| 6. The **"Fee:"** label displays the estimated fee. | ||
| 7. A **"Fee Confirmation Required"** dialog appears — confirm the transaction. | ||
| 8. Wait for the transaction to appear in the **"Transactions"** section. | ||
| 9. Click **"Send"** again. | ||
| 10. In **"To:"**, enter `${TC_IDN_ADDRESS_0}`. | ||
| 11. In **"Amount:"**, enter `3`. | ||
| 12. Confirm in the **"Fee Confirmation Required"** dialog. |
There was a problem hiding this comment.
🔴 Blocking: Send flows skip the in-form "Send" click that opens the fee confirmation dialog
In Session Setup steps 4–8, TC-SND-005-01 steps 4–7, TC-SND-001-01 steps 5–8, and Session Cleanup, the instructions go straight from filling "To:"/"Amount:" to "A 'Fee Confirmation Required' dialog appears". The dialog is only triggered by clicking the in-form Send button in render_send_button() (src/ui/wallets/single_key_send_screen.rs:812-853), which calls validate_and_send. Without that explicit click, the tester remains on the compose form and never reaches the dialog. Add an explicit "Click Send (in the form)" step before each "Fee Confirmation Required" line.
source: ['codex']
🤖 Fix this with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `docs/test-cases.md`:
- [BLOCKING] lines 72-83: Send flows skip the in-form "Send" click that opens the fee confirmation dialog
In Session Setup steps 4–8, TC-SND-005-01 steps 4–7, TC-SND-001-01 steps 5–8, and Session Cleanup, the instructions go straight from filling "To:"/"Amount:" to "A 'Fee Confirmation Required' dialog appears". The dialog is only triggered by clicking the in-form `Send` button in `render_send_button()` (`src/ui/wallets/single_key_send_screen.rs:812-853`), which calls `validate_and_send`. Without that explicit click, the tester remains on the compose form and never reaches the dialog. Add an explicit "Click Send (in the form)" step before each "Fee Confirmation Required" line.
| | **Pre-Conditions** | DET running on testnet, a known token contract ID available | | ||
| | **Test Steps** | 1. Click **"Tokens"** in the left sidebar.<br>2. Click the **"Add Token"** button.<br>3. Verify the heading reads **"Add Token"**.<br>4. In the input field (*"Enter either a Contract ID or Token ID"*), enter a known testnet token contract ID.<br>5. Confirm the addition.<br>6. Verify the token appears in the **"My Tokens"** tab with its name and balance. | | ||
| | **Test Data** | A valid token contract ID from testnet | | ||
| | **Expected Result** | Token added. Appears in the **"My Tokens"** list with name and balance. | |
There was a problem hiding this comment.
🟡 Suggestion: TC-TOK-003-01 expects a token to appear in "My Tokens" but the screen is built from owned balances
The expected result requires the added token to appear in My Tokens with a name and balance. The implementation does save the token locally and run QueryMyTokenBalances (src/ui/tokens/tokens_screen/mod.rs:2511-2520), but my_tokens() (src/ui/tokens/tokens_screen/mod.rs:1472-1502) is built from identity_token_balances() — only tokens the local identities actually hold. If the test identity holds zero of the added token, the screen falls back to the "No Tracked Tokens" empty state (src/ui/tokens/tokens_screen/my_tokens.rs:226-229) and the test fails despite add-by-ID working. Either pre-fund a token fixture against the test identity, or change the expected result to verify successful tracking via the Search Tokens / contract list rather than balance visibility.
source: ['codex']
🤖 Fix this with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `docs/test-cases.md`:
- [SUGGESTION] lines 504-507: TC-TOK-003-01 expects a token to appear in "My Tokens" but the screen is built from owned balances
The expected result requires the added token to appear in **My Tokens** with a name and balance. The implementation does save the token locally and run `QueryMyTokenBalances` (`src/ui/tokens/tokens_screen/mod.rs:2511-2520`), but `my_tokens()` (`src/ui/tokens/tokens_screen/mod.rs:1472-1502`) is built from `identity_token_balances()` — only tokens the local identities actually hold. If the test identity holds zero of the added token, the screen falls back to the "No Tracked Tokens" empty state (`src/ui/tokens/tokens_screen/my_tokens.rs:226-229`) and the test fails despite add-by-ID working. Either pre-fund a token fixture against the test identity, or change the expected result to verify successful tracking via the Search Tokens / contract list rather than balance visibility.
| ### TC-ALK-001-01: Create an asset lock | ||
|
|
||
| | Field | Value | | ||
| |-------|-------| | ||
| | **Use Case ID** | ALK-001 | | ||
| | **Test Case ID** | TC-ALK-001-01 | | ||
| | **Short Description** | Create an asset lock from the Identity Test wallet | | ||
| | **Pre-Conditions** | **Identity Test** wallet selected with >= 2 tDASH | | ||
| | **Test Steps** | 1. Click **"Wallets"** in the left sidebar.<br>2. Select the **Identity Test** wallet.<br>3. Navigate to asset lock creation (via the identity registration flow or wallet tools).<br>4. Enter amount: `0.5` tDASH.<br>5. Review the fee calculation.<br>6. Confirm and create the asset lock.<br>7. Wait for the asset lock transaction to confirm. | | ||
| | **Test Data** | Amount: `0.5` tDASH | | ||
| | **Expected Result** | Asset lock created. Transaction ID is shown. **"Core balance:"** decreases by ~0.5 tDASH + fee. | | ||
| | **Post-Condition** | Asset lock available for identity registration in TC-IDN-001-01. | | ||
|
|
There was a problem hiding this comment.
🟡 Suggestion: TC-ALK-001-01 step 3 is too vague — "navigate via the identity registration flow or wallet tools"
Step 3 reads "Navigate to asset lock creation (via the identity registration flow or wallet tools)." Every other step in this PR names the exact sidebar item, button, and dialog labels (e.g., TC-WAL-002-01, TC-DPY-001-01). This step asks the tester to guess between two paths, so two testers will execute the same case differently — defeating the smoke-test spec. Spell out the exact sidebar item, button labels, and dialog title for the asset-lock creation entry point used by the identity registration flow.
source: ['claude']
🤖 Fix this with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `docs/test-cases.md`:
- [SUGGESTION] lines 325-337: TC-ALK-001-01 step 3 is too vague — "navigate via the identity registration flow or wallet tools"
Step 3 reads "Navigate to asset lock creation (via the identity registration flow or wallet tools)." Every other step in this PR names the exact sidebar item, button, and dialog labels (e.g., TC-WAL-002-01, TC-DPY-001-01). This step asks the tester to guess between two paths, so two testers will execute the same case differently — defeating the smoke-test spec. Spell out the exact sidebar item, button labels, and dialog title for the asset-lock creation entry point used by the identity registration flow.
| ### TC-IDN-001-01: Register a new identity | ||
|
|
||
| | Field | Value | | ||
| |-------|-------| | ||
| | **Use Case ID** | IDN-001 | | ||
| | **Test Case ID** | TC-IDN-001-01 | | ||
| | **Short Description** | Register a new identity funded by asset lock | | ||
| | **Pre-Conditions** | **Identity Test** wallet selected; asset lock from TC-ALK-001-01 is confirmed | | ||
| | **Test Steps** | 1. Click **"Identities"** in the left sidebar.<br>2. Navigate to identity creation (breadcrumb: **Identities > Create Identity**).<br>3. Select the **Identity Test** wallet as the funding source.<br>4. Follow the multi-stage confirmation flow.<br>5. Wait for the identity to be registered on Platform.<br>6. Verify the new identity appears in the identities list with an identity ID (hex string). | | ||
| | **Test Data** | Funding: asset lock from TC-ALK-001-01 | | ||
| | **Expected Result** | Identity registered. Identity ID displayed. Credits balance > 0. | | ||
| | **Post-Condition** | New identity available for DPNS, DashPay, and token tests. | |
There was a problem hiding this comment.
🟡 Suggestion: TC-IDN-001-01 navigation/steps are too generic for a smoke-test spec
Steps 2 and 4 read "Navigate to identity creation (breadcrumb: Identities > Create Identity)" and "Follow the multi-stage confirmation flow." Compared with TC-WAL-002-01 or TC-DPY-001-01, this case provides almost no guidance for the multi-step identity registration flow (asset-lock selection, key generation, registration confirmation). Either expand into named steps/buttons, or list the intermediate UI states (e.g., asset-lock selection screen, key-generation summary, registration confirmation) the tester must verify, so two testers produce comparable results.
source: ['claude']
🤖 Fix this with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `docs/test-cases.md`:
- [SUGGESTION] lines 342-353: TC-IDN-001-01 navigation/steps are too generic for a smoke-test spec
Steps 2 and 4 read "Navigate to identity creation (breadcrumb: Identities > Create Identity)" and "Follow the multi-stage confirmation flow." Compared with TC-WAL-002-01 or TC-DPY-001-01, this case provides almost no guidance for the multi-step identity registration flow (asset-lock selection, key generation, registration confirmation). Either expand into named steps/buttons, or list the intermediate UI states (e.g., asset-lock selection screen, key-generation summary, registration confirmation) the tester must verify, so two testers produce comparable results.
| ```env | ||
| # Testnet SPV mode — DAPI addresses plus placeholder Core RPC fields | ||
| TESTNET_dapi_addresses=https://34.214.48.68:1443,https://52.12.176.90:1443,https://52.34.144.50:1443,https://44.240.98.102:1443,https://54.201.32.131:1443,https://52.10.229.11:1443,https://52.13.132.146:1443,https://52.40.219.41:1443,https://54.149.33.167:1443,https://35.164.23.245:1443,https://52.33.28.47:1443,https://52.43.13.92:1443,https://52.89.154.48:1443,https://52.24.124.162:1443,https://35.85.21.179:1443,https://54.187.14.232:1443,https://54.68.235.201:1443,https://52.13.250.182:1443 | ||
|
|
||
| # Core RPC fields are structurally required by NetworkConfig even in SPV mode, | ||
| # but are not actively used at runtime. Supply placeholder values. | ||
| TESTNET_core_host=127.0.0.1 | ||
| TESTNET_core_rpc_port=19998 | ||
| TESTNET_core_rpc_user=user | ||
| TESTNET_core_rpc_password=password | ||
| TESTNET_insight_api_url=https://insight.testnet.networks.dash.org:3002/insight-api | ||
| # core_zmq_endpoint is optional and can be omitted | ||
| ``` |
There was a problem hiding this comment.
💬 Nitpick: Hardcoded testnet DAPI IP list will go stale
The example .env lists 18 hardcoded testnet DAPI IPs. Testnet evonode addresses change as nodes churn, so a weekly smoke test that hard-codes them will start failing silently when nodes drop out. Point testers at the bundled .env.example (auto-copied via copy_env_file_if_not_exists in src/app_dir.rs:92) or otherwise reference a maintained list rather than embedding the IPs here.
source: ['claude']
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
All 12 findings from Claude and Codex check out against the source at 53e06a8. The largest cluster is the repeated 'click Save' instruction after toggling Developer mode in three docs locations and after switching theme — neither control has a Save button (developer_mode persists inside StyledCheckbox.clicked() at src/ui/network_chooser_screen.rs:936-959; theme persists inside selectable_value.clicked() at :744-792). Other confirmed blockers: TC-WAL-001-01 skips the mandatory 'I wrote it down' checkbox (src/ui/wallets/add_new_wallet_screen.rs:776-781) and uses a non-existent 'Confirm creation' button instead of 'Save Wallet' (:873); TC-DPN-002 points to a non-existent identity owned-names section (owned names live in DPNSSubscreen::Owned, reachable from the DPNS chooser, not from the Identities screen); TC-DPN-003 routes QA to Dashpay but the contests view is under Tools→DPNS; TC-DEV-005-01 expects Platform Info data to appear automatically but the screen shows 'No results yet' until a Fetch button is clicked (src/ui/tools/platform_info_screen.rs:55-91, 180-182). Save-button issues at three locations consolidated into one finding to fit the 10-comment budget.
Reviewed commit: 53e06a8
🔴 6 blocking | 🟡 3 suggestion(s) | 💬 1 nitpick(s)
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `docs/test-cases.md`:
- [BLOCKING] line 35: Developer-mode toggle has no 'Save' button — checkbox click is the action (3 doc locations)
Multiple test instructions tell the tester to click 'Save' after checking/unchecking the Developer mode checkbox: Session Setup §1 step 2 (this line), TC-NET-005-01 steps 3, 8 and Post-Condition (test-cases.md:141-144), and the Enable Developer Mode prereq step 3 (test-prerequisites.md:99-103). The Developer mode checkbox auto-persists on click: src/ui/network_chooser_screen.rs:936 renders `StyledCheckbox::new(&mut self.developer_mode, "Developer mode")` and lines 942-959 call `enable_developer_mode(...)` and `config.save()` synchronously inside the `.clicked()` branch — there is no follow-up Save button. The only Save button on this screen (line 397) is gated by `network == Regtest && backend_mode == Rpc` and persists only the Local Network Password — it is invisible on Testnet/SPV. A tester following these steps will hunt for a Save button that does not exist and may falsely conclude the toggle did not apply. Remove the 'Save' click from all three locations and verify behavior via the side effects already described (Connection Type selector appears/disappears, developer-only Wallets UI appears/disappears).
- [BLOCKING] lines 128-131: TC-NET-004-01 tells testers to click 'Save' after changing the theme — no such button exists
Steps 3 and 5 and the Post-Condition (line 131) instruct: 'Select "🌙 Dark" and click "Save".' The theme combo in src/ui/network_chooser_screen.rs:744-792 dispatches `BackendTask::SystemTask(SystemTask::UpdateThemePreference(...))` inside each selectable_value's `.clicked()` block — the theme persists immediately on selection with no confirmation button. The only Save button on the screen is the Regtest-RPC Local Network Password Save (line 397), invisible on Testnet/SPV. As written, 'Theme changes apply after clicking Save' is unobservable and the case cannot pass. Drop the Save clicks and verify the UI updates immediately after selection.
- [BLOCKING] lines 171-174: TC-WAL-001-01 omits the mandatory 'I wrote it down' checkbox and uses a non-existent 'Confirm creation' button
The create-wallet wizard in src/ui/wallets/add_new_wallet_screen.rs gates steps 4-6 on the 'I wrote it down' checkbox: line 776 renders it and lines 779-781 `return` early when it is unchecked, so the 'Wallet Name' (heading at line 787), 'Optional Password' (:800), and 'Save the wallet' (:865) sections never appear until the checkbox is ticked. The test case jumps straight from 'Record' (step 6) to 'Name' (step 7) without this gating step. Step 9 then says 'Confirm creation', but the actual final button is labeled 'Save Wallet' (line 873) under the heading '6. Save the wallet.' (line 865). A tester following the case will (1) get stuck after writing down the words because steps 7-8 are not visible until the checkbox is ticked, and (2) look for a button that does not exist. Insert a step '(After recording) Check the "I wrote it down" checkbox' before the Name step, and replace 'Confirm creation' with 'Click "Save Wallet"'.
- [BLOCKING] line 421: TC-DPN-002 sends testers to an owned-usernames section that does not exist on the Identities screen
Step 3 says, from the Identities screen, to 'navigate to the owned usernames section'. Owned DPNS names are rendered only by the dedicated DPNS screen when `DPNSSubscreen::Owned` is active (src/ui/dpns/dpns_contested_names_screen.rs and the chooser at src/ui/components/dpns_subscreen_chooser_panel.rs:15, :91-94 which routes 'Owned' → `RootScreenDPNSOwnedNames`). The Identities screen does not expose an owned-usernames panel. Rewrite step 3 as: 'Click "Tools" → "DPNS" in the left sidebar (or use the DPNS subscreen chooser) and select the "Owned Names" subscreen.' Then verify the username appears in that list for the identity created in TC-IDN-001-01.
- [BLOCKING] line 434: TC-DPN-003 points QA to Dashpay, but the DPNS contests screen lives under Tools → DPNS
Step 1 instructs the tester to click 'Dashpay' in the left sidebar, but that route navigates to the DashPay screens whose subscreens are My Profile, Contacts, Payment History, and Search Profiles (src/ui/components/dashpay_subscreen_chooser_panel.rs:18-25, :57-61). The DPNS contest views (Active / Past / Owned / ScheduledVotes) are a separate Tools subscreen (src/ui/components/dpns_subscreen_chooser_panel.rs:12-17) mapped to `RootScreenDPNSActiveContests` and siblings. A tester following the documented path will never reach the contests view. Rewrite step 1 as: 'Click "Tools" in the left sidebar, then select "DPNS" in the Tools subscreen chooser, then select the "Active" subscreen.'
- [BLOCKING] lines 554-556: TC-DEV-005-01 expects Platform Info to display data automatically, but the screen is empty until a Fetch button is clicked
Steps 3-5 tell testers to confirm epoch info, total credits, and validators are displayed immediately. The actual screen exposes discrete actions ('Fetch Basic Platform Info', 'Fetch Current Epoch Info', 'Fetch Total Credits on Platform', 'Fetch Current Validator Set Info', etc. — src/ui/tools/platform_info_screen.rs:55-91), and `refresh_on_arrival` explicitly does not auto-refresh (:180-182). Until one of those buttons is clicked, the result pane shows 'No results yet' with 'Select an action from the left panel to fetch platform information.' (:159-163). Rewrite the steps to click each Fetch button (Current Epoch Info, Total Credits on Platform, Current Validator Set Info) and verify the corresponding result pane.
- [SUGGESTION] lines 352-354: TC-IDN-001-01 says the new user identity ID is shown as a hex string, but user IDs are displayed in Base58
Step 9 says the new identity ID is a 'hex string'. For user identities the UI uses Base58, not hex: `IdentityType::User` returns `Encoding::Base58` from `default_encoding()` (src/model/qualified_identity/mod.rs:50-55), and the identities table renders user IDs with `Encoding::Base58` in `show_identity_id()` (src/ui/identities/identities_screen.rs:248-260). Only Masternode/Evonode identities are shown as hex (ProTxHash). Since this case registers a wallet-funded user identity, change 'hex string' to 'Base58 string' to avoid a false failure condition.
- [SUGGESTION] line 391: TC-IDN-009-01 references a refresh-mode selector that does not exist on the Identities screen
Step 3 tells testers to use a refresh mode selector with options 'Core + Platform', 'Core Only', 'Platform Only' while on the Identities screen. The Identities screen exposes only a top-panel 'Refresh' button (src/ui/identities/identities_screen.rs:1178-1184) — there is no mode selector. The only refresh-mode dropdown is on the Wallets screen, and its actual options are also different ('All (Auto)', 'Core Only', 'Platform (Full/Terminal)', 'Core + Platform (Full/Terminal)' — src/ui/wallets/wallets_screen/mod.rs:39-77). Rewrite the step around the actual Identities-screen 'Refresh' top-panel action.
- [SUGGESTION] lines 32-39: Session Setup treats 'Settings' and 'Network Chooser' as separate screens, but they are the same screen
Step 1 says 'Click "Settings" in the left sidebar', then step 3 says 'Open the Network Chooser screen' as if it is a different destination. The sidebar 'Settings' entry maps to `RootScreenType::RootScreenNetworkChooser` — clicking Settings IS the Network Chooser. Testers will look for a separate navigation path and may skip step 3 or get lost. Reword as 'Stay on the Settings screen (this is the Network Chooser)…' or collapse into one navigation step. TC-NET-005-01 step 4 has the same issue.
| ### 1. Configure SPV Mode | ||
|
|
||
| 1. Click **"Settings"** in the left sidebar. | ||
| 2. Check the **"Developer mode"** checkbox and click **"Save"**. |
There was a problem hiding this comment.
🔴 Blocking: Developer-mode toggle has no 'Save' button — checkbox click is the action (3 doc locations)
Multiple test instructions tell the tester to click 'Save' after checking/unchecking the Developer mode checkbox: Session Setup §1 step 2 (this line), TC-NET-005-01 steps 3, 8 and Post-Condition (test-cases.md:141-144), and the Enable Developer Mode prereq step 3 (test-prerequisites.md:99-103). The Developer mode checkbox auto-persists on click: src/ui/network_chooser_screen.rs:936 renders StyledCheckbox::new(&mut self.developer_mode, "Developer mode") and lines 942-959 call enable_developer_mode(...) and config.save() synchronously inside the .clicked() branch — there is no follow-up Save button. The only Save button on this screen (line 397) is gated by network == Regtest && backend_mode == Rpc and persists only the Local Network Password — it is invisible on Testnet/SPV. A tester following these steps will hunt for a Save button that does not exist and may falsely conclude the toggle did not apply. Remove the 'Save' click from all three locations and verify behavior via the side effects already described (Connection Type selector appears/disappears, developer-only Wallets UI appears/disappears).
source: ['claude']
🤖 Fix this with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `docs/test-cases.md`:
- [BLOCKING] line 35: Developer-mode toggle has no 'Save' button — checkbox click is the action (3 doc locations)
Multiple test instructions tell the tester to click 'Save' after checking/unchecking the Developer mode checkbox: Session Setup §1 step 2 (this line), TC-NET-005-01 steps 3, 8 and Post-Condition (test-cases.md:141-144), and the Enable Developer Mode prereq step 3 (test-prerequisites.md:99-103). The Developer mode checkbox auto-persists on click: src/ui/network_chooser_screen.rs:936 renders `StyledCheckbox::new(&mut self.developer_mode, "Developer mode")` and lines 942-959 call `enable_developer_mode(...)` and `config.save()` synchronously inside the `.clicked()` branch — there is no follow-up Save button. The only Save button on this screen (line 397) is gated by `network == Regtest && backend_mode == Rpc` and persists only the Local Network Password — it is invisible on Testnet/SPV. A tester following these steps will hunt for a Save button that does not exist and may falsely conclude the toggle did not apply. Remove the 'Save' click from all three locations and verify behavior via the side effects already described (Connection Type selector appears/disappears, developer-only Wallets UI appears/disappears).
| | **Test Steps** | 1. Click **"Settings"** in the left sidebar.<br>2. Locate the **"Theme:"** selector.<br>3. Select **"🌙 Dark"** and click **"Save"**.<br>4. Verify the UI updates to dark colors (dark background, light text).<br>5. Select **"☀ Light"** and click **"Save"**.<br>6. Verify the UI updates to light colors (light background, dark text). | | ||
| | **Test Data** | N/A | | ||
| | **Expected Result** | Theme changes apply after clicking **"Save"**. UI is readable in both modes. | | ||
| | **Post-Condition** | Set theme to preferred value and click **"Save"**. | |
There was a problem hiding this comment.
🔴 Blocking: TC-NET-004-01 tells testers to click 'Save' after changing the theme — no such button exists
Steps 3 and 5 and the Post-Condition (line 131) instruct: 'Select "🌙 Dark" and click "Save".' The theme combo in src/ui/network_chooser_screen.rs:744-792 dispatches BackendTask::SystemTask(SystemTask::UpdateThemePreference(...)) inside each selectable_value's .clicked() block — the theme persists immediately on selection with no confirmation button. The only Save button on the screen is the Regtest-RPC Local Network Password Save (line 397), invisible on Testnet/SPV. As written, 'Theme changes apply after clicking Save' is unobservable and the case cannot pass. Drop the Save clicks and verify the UI updates immediately after selection.
source: ['claude']
🤖 Fix this with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `docs/test-cases.md`:
- [BLOCKING] lines 128-131: TC-NET-004-01 tells testers to click 'Save' after changing the theme — no such button exists
Steps 3 and 5 and the Post-Condition (line 131) instruct: 'Select "🌙 Dark" and click "Save".' The theme combo in src/ui/network_chooser_screen.rs:744-792 dispatches `BackendTask::SystemTask(SystemTask::UpdateThemePreference(...))` inside each selectable_value's `.clicked()` block — the theme persists immediately on selection with no confirmation button. The only Save button on the screen is the Regtest-RPC Local Network Password Save (line 397), invisible on Testnet/SPV. As written, 'Theme changes apply after clicking Save' is unobservable and the case cannot pass. Drop the Save clicks and verify the UI updates immediately after selection.
| | **Test Steps** | 1. Click **"Wallets"** in the left sidebar.<br>2. Click **"Create Wallet"** in the top-right corner.<br>3. Verify the breadcrumb reads **Wallets > Create Wallet**.<br>4. **Step 1 — Entropy:** Move the mouse around the entropy grid until the progress bar fills.<br>5. **Step 2 — Generate:** Under the heading *"Select your desired seed phrase language and word count and press Generate"*, select **English** and **24 words**, then click **"Generate"**.<br>6. **Step 3 — Record:** Under the heading *"Write down the passphrase on a piece of paper"*, write down all 24 displayed words.<br>7. **Step 4 — Name:** Under the heading *"Enter a wallet name"*, type `Created Wallet` in the **"Wallet Name:"** field.<br>8. **Step 5 — Password:** Under the heading *"Add a password"*, leave the **"Optional Password:"** field empty.<br>9. Confirm creation. | | ||
| | **Test Data** | Wallet name: `Created Wallet`, no password | | ||
| | **Expected Result** | Wallet appears on the **"Wallets"** screen. **"Core balance:"** shows `0.0000 DASH`. The generated mnemonic was 24 English words. | | ||
| | **Post-Condition** | `Created Wallet` exists with zero balance. | |
There was a problem hiding this comment.
🔴 Blocking: TC-WAL-001-01 omits the mandatory 'I wrote it down' checkbox and uses a non-existent 'Confirm creation' button
The create-wallet wizard in src/ui/wallets/add_new_wallet_screen.rs gates steps 4-6 on the 'I wrote it down' checkbox: line 776 renders it and lines 779-781 return early when it is unchecked, so the 'Wallet Name' (heading at line 787), 'Optional Password' (:800), and 'Save the wallet' (:865) sections never appear until the checkbox is ticked. The test case jumps straight from 'Record' (step 6) to 'Name' (step 7) without this gating step. Step 9 then says 'Confirm creation', but the actual final button is labeled 'Save Wallet' (line 873) under the heading '6. Save the wallet.' (line 865). A tester following the case will (1) get stuck after writing down the words because steps 7-8 are not visible until the checkbox is ticked, and (2) look for a button that does not exist. Insert a step '(After recording) Check the "I wrote it down" checkbox' before the Name step, and replace 'Confirm creation' with 'Click "Save Wallet"'.
source: ['claude']
🤖 Fix this with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `docs/test-cases.md`:
- [BLOCKING] lines 171-174: TC-WAL-001-01 omits the mandatory 'I wrote it down' checkbox and uses a non-existent 'Confirm creation' button
The create-wallet wizard in src/ui/wallets/add_new_wallet_screen.rs gates steps 4-6 on the 'I wrote it down' checkbox: line 776 renders it and lines 779-781 `return` early when it is unchecked, so the 'Wallet Name' (heading at line 787), 'Optional Password' (:800), and 'Save the wallet' (:865) sections never appear until the checkbox is ticked. The test case jumps straight from 'Record' (step 6) to 'Name' (step 7) without this gating step. Step 9 then says 'Confirm creation', but the actual final button is labeled 'Save Wallet' (line 873) under the heading '6. Save the wallet.' (line 865). A tester following the case will (1) get stuck after writing down the words because steps 7-8 are not visible until the checkbox is ticked, and (2) look for a button that does not exist. Insert a step '(After recording) Check the "I wrote it down" checkbox' before the Name step, and replace 'Confirm creation' with 'Click "Save Wallet"'.
| | **Test Case ID** | TC-DPN-002-01 | | ||
| | **Short Description** | Verify registered username appears in the owned names list | | ||
| | **Pre-Conditions** | Username registered in TC-DPN-001-01 | | ||
| | **Test Steps** | 1. Click **"Identities"** in the left sidebar.<br>2. Select the identity from TC-IDN-001-01.<br>3. Navigate to the owned usernames section.<br>4. Verify the username from TC-DPN-001-01 is listed. | |
There was a problem hiding this comment.
🔴 Blocking: TC-DPN-002 sends testers to an owned-usernames section that does not exist on the Identities screen
Step 3 says, from the Identities screen, to 'navigate to the owned usernames section'. Owned DPNS names are rendered only by the dedicated DPNS screen when DPNSSubscreen::Owned is active (src/ui/dpns/dpns_contested_names_screen.rs and the chooser at src/ui/components/dpns_subscreen_chooser_panel.rs:15, :91-94 which routes 'Owned' → RootScreenDPNSOwnedNames). The Identities screen does not expose an owned-usernames panel. Rewrite step 3 as: 'Click "Tools" → "DPNS" in the left sidebar (or use the DPNS subscreen chooser) and select the "Owned Names" subscreen.' Then verify the username appears in that list for the identity created in TC-IDN-001-01.
source: ['codex']
🤖 Fix this with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `docs/test-cases.md`:
- [BLOCKING] line 421: TC-DPN-002 sends testers to an owned-usernames section that does not exist on the Identities screen
Step 3 says, from the Identities screen, to 'navigate to the owned usernames section'. Owned DPNS names are rendered only by the dedicated DPNS screen when `DPNSSubscreen::Owned` is active (src/ui/dpns/dpns_contested_names_screen.rs and the chooser at src/ui/components/dpns_subscreen_chooser_panel.rs:15, :91-94 which routes 'Owned' → `RootScreenDPNSOwnedNames`). The Identities screen does not expose an owned-usernames panel. Rewrite step 3 as: 'Click "Tools" → "DPNS" in the left sidebar (or use the DPNS subscreen chooser) and select the "Owned Names" subscreen.' Then verify the username appears in that list for the identity created in TC-IDN-001-01.
| | **Test Case ID** | TC-DPN-003-01 | | ||
| | **Short Description** | View the DPNS name contests screen | | ||
| | **Pre-Conditions** | DET running on testnet, SPV synced | | ||
| | **Test Steps** | 1. Click **"Dashpay"** in the left sidebar.<br>2. Locate the active DPNS contests view.<br>3. Verify the screen loads without error.<br>4. If contests exist, verify each shows status and vote information. | |
There was a problem hiding this comment.
🔴 Blocking: TC-DPN-003 points QA to Dashpay, but the DPNS contests screen lives under Tools → DPNS
Step 1 instructs the tester to click 'Dashpay' in the left sidebar, but that route navigates to the DashPay screens whose subscreens are My Profile, Contacts, Payment History, and Search Profiles (src/ui/components/dashpay_subscreen_chooser_panel.rs:18-25, :57-61). The DPNS contest views (Active / Past / Owned / ScheduledVotes) are a separate Tools subscreen (src/ui/components/dpns_subscreen_chooser_panel.rs:12-17) mapped to RootScreenDPNSActiveContests and siblings. A tester following the documented path will never reach the contests view. Rewrite step 1 as: 'Click "Tools" in the left sidebar, then select "DPNS" in the Tools subscreen chooser, then select the "Active" subscreen.'
source: ['codex']
🤖 Fix this with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `docs/test-cases.md`:
- [BLOCKING] line 434: TC-DPN-003 points QA to Dashpay, but the DPNS contests screen lives under Tools → DPNS
Step 1 instructs the tester to click 'Dashpay' in the left sidebar, but that route navigates to the DashPay screens whose subscreens are My Profile, Contacts, Payment History, and Search Profiles (src/ui/components/dashpay_subscreen_chooser_panel.rs:18-25, :57-61). The DPNS contest views (Active / Past / Owned / ScheduledVotes) are a separate Tools subscreen (src/ui/components/dpns_subscreen_chooser_panel.rs:12-17) mapped to `RootScreenDPNSActiveContests` and siblings. A tester following the documented path will never reach the contests view. Rewrite step 1 as: 'Click "Tools" in the left sidebar, then select "DPNS" in the Tools subscreen chooser, then select the "Active" subscreen.'
| | **Test Steps** | 1. Click **"Tools"** in the left sidebar.<br>2. The default view is **"Platform Info"**.<br>3. Confirm epoch info is displayed (epoch number, start time).<br>4. Check that total credits in the system are shown.<br>5. Ensure a validator list is visible. | | ||
| | **Test Data** | N/A | | ||
| | **Expected Result** | **"Platform Info"** screen loads. Epoch, credits, and validators shown with current data. | |
There was a problem hiding this comment.
🔴 Blocking: TC-DEV-005-01 expects Platform Info to display data automatically, but the screen is empty until a Fetch button is clicked
Steps 3-5 tell testers to confirm epoch info, total credits, and validators are displayed immediately. The actual screen exposes discrete actions ('Fetch Basic Platform Info', 'Fetch Current Epoch Info', 'Fetch Total Credits on Platform', 'Fetch Current Validator Set Info', etc. — src/ui/tools/platform_info_screen.rs:55-91), and refresh_on_arrival explicitly does not auto-refresh (:180-182). Until one of those buttons is clicked, the result pane shows 'No results yet' with 'Select an action from the left panel to fetch platform information.' (:159-163). Rewrite the steps to click each Fetch button (Current Epoch Info, Total Credits on Platform, Current Validator Set Info) and verify the corresponding result pane.
source: ['codex']
🤖 Fix this with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `docs/test-cases.md`:
- [BLOCKING] lines 554-556: TC-DEV-005-01 expects Platform Info to display data automatically, but the screen is empty until a Fetch button is clicked
Steps 3-5 tell testers to confirm epoch info, total credits, and validators are displayed immediately. The actual screen exposes discrete actions ('Fetch Basic Platform Info', 'Fetch Current Epoch Info', 'Fetch Total Credits on Platform', 'Fetch Current Validator Set Info', etc. — src/ui/tools/platform_info_screen.rs:55-91), and `refresh_on_arrival` explicitly does not auto-refresh (:180-182). Until one of those buttons is clicked, the result pane shows 'No results yet' with 'Select an action from the left panel to fetch platform information.' (:159-163). Rewrite the steps to click each Fetch button (Current Epoch Info, Total Credits on Platform, Current Validator Set Info) and verify the corresponding result pane.
| | **Test Steps** | 1. Click **"Identities"** in the left sidebar.<br>2. Click **"Create Identity"** and verify the breadcrumb shows **Identities > Create Identity**.<br>3. Select the **Identity Test** wallet as the funding source (unlock it if prompted).<br>4. Review the generated identity keys and optional **"Alias:"** field.<br>5. In **"Choose your funding method"**, select **"Unused Evo Funding Locks (recommended)"**.<br>6. Under **"Select an unused asset lock:"**, choose the asset lock created in TC-ALK-001-01 and click **"Select"**.<br>7. Verify the **"Estimated Fee:"** summary, then click **"Create Identity"**.<br>8. Wait through **"=> Waiting for Platform acknowledgement <="** until the success screen appears.<br>9. Verify the new identity appears in the identities list with an identity ID (hex string). | | ||
| | **Test Data** | Funding: asset lock from TC-ALK-001-01 | | ||
| | **Expected Result** | Identity registered. Identity ID displayed. Credits balance > 0. | |
There was a problem hiding this comment.
🟡 Suggestion: TC-IDN-001-01 says the new user identity ID is shown as a hex string, but user IDs are displayed in Base58
Step 9 says the new identity ID is a 'hex string'. For user identities the UI uses Base58, not hex: IdentityType::User returns Encoding::Base58 from default_encoding() (src/model/qualified_identity/mod.rs:50-55), and the identities table renders user IDs with Encoding::Base58 in show_identity_id() (src/ui/identities/identities_screen.rs:248-260). Only Masternode/Evonode identities are shown as hex (ProTxHash). Since this case registers a wallet-funded user identity, change 'hex string' to 'Base58 string' to avoid a false failure condition.
source: ['codex']
🤖 Fix this with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `docs/test-cases.md`:
- [SUGGESTION] lines 352-354: TC-IDN-001-01 says the new user identity ID is shown as a hex string, but user IDs are displayed in Base58
Step 9 says the new identity ID is a 'hex string'. For user identities the UI uses Base58, not hex: `IdentityType::User` returns `Encoding::Base58` from `default_encoding()` (src/model/qualified_identity/mod.rs:50-55), and the identities table renders user IDs with `Encoding::Base58` in `show_identity_id()` (src/ui/identities/identities_screen.rs:248-260). Only Masternode/Evonode identities are shown as hex (ProTxHash). Since this case registers a wallet-funded user identity, change 'hex string' to 'Base58 string' to avoid a false failure condition.
| | **Test Case ID** | TC-IDN-009-01 | | ||
| | **Short Description** | Manually refresh identity data from the network | | ||
| | **Pre-Conditions** | Identity from TC-IDN-001-01 exists | | ||
| | **Test Steps** | 1. Click **"Identities"** in the left sidebar.<br>2. Select the identity from TC-IDN-001-01.<br>3. Click the refresh button, or use the refresh mode selector: **"Core + Platform"**, **"Core Only"**, or **"Platform Only"**.<br>4. Wait for the refresh to complete.<br>5. Verify the credit balance and key state update without error. | |
There was a problem hiding this comment.
🟡 Suggestion: TC-IDN-009-01 references a refresh-mode selector that does not exist on the Identities screen
Step 3 tells testers to use a refresh mode selector with options 'Core + Platform', 'Core Only', 'Platform Only' while on the Identities screen. The Identities screen exposes only a top-panel 'Refresh' button (src/ui/identities/identities_screen.rs:1178-1184) — there is no mode selector. The only refresh-mode dropdown is on the Wallets screen, and its actual options are also different ('All (Auto)', 'Core Only', 'Platform (Full/Terminal)', 'Core + Platform (Full/Terminal)' — src/ui/wallets/wallets_screen/mod.rs:39-77). Rewrite the step around the actual Identities-screen 'Refresh' top-panel action.
source: ['codex']
🤖 Fix this with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `docs/test-cases.md`:
- [SUGGESTION] line 391: TC-IDN-009-01 references a refresh-mode selector that does not exist on the Identities screen
Step 3 tells testers to use a refresh mode selector with options 'Core + Platform', 'Core Only', 'Platform Only' while on the Identities screen. The Identities screen exposes only a top-panel 'Refresh' button (src/ui/identities/identities_screen.rs:1178-1184) — there is no mode selector. The only refresh-mode dropdown is on the Wallets screen, and its actual options are also different ('All (Auto)', 'Core Only', 'Platform (Full/Terminal)', 'Core + Platform (Full/Terminal)' — src/ui/wallets/wallets_screen/mod.rs:39-77). Rewrite the step around the actual Identities-screen 'Refresh' top-panel action.
| ### 1. Configure SPV Mode | ||
|
|
||
| 1. Click **"Settings"** in the left sidebar. | ||
| 2. Check the **"Developer mode"** checkbox and click **"Save"**. | ||
| 3. Open the **Network Chooser** screen (displayed at startup or via the top-bar network selector). | ||
| 4. In **"Connection Settings"**, open the **"Connection Type"** dropdown and select **"SPV Client"**. | ||
| 5. Wait for the connection status indicator (top bar) to turn green. | ||
|
|
There was a problem hiding this comment.
🟡 Suggestion: Session Setup treats 'Settings' and 'Network Chooser' as separate screens, but they are the same screen
Step 1 says 'Click "Settings" in the left sidebar', then step 3 says 'Open the Network Chooser screen' as if it is a different destination. The sidebar 'Settings' entry maps to RootScreenType::RootScreenNetworkChooser — clicking Settings IS the Network Chooser. Testers will look for a separate navigation path and may skip step 3 or get lost. Reword as 'Stay on the Settings screen (this is the Network Chooser)…' or collapse into one navigation step. TC-NET-005-01 step 4 has the same issue.
source: ['claude']
🤖 Fix this with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `docs/test-cases.md`:
- [SUGGESTION] lines 32-39: Session Setup treats 'Settings' and 'Network Chooser' as separate screens, but they are the same screen
Step 1 says 'Click "Settings" in the left sidebar', then step 3 says 'Open the Network Chooser screen' as if it is a different destination. The sidebar 'Settings' entry maps to `RootScreenType::RootScreenNetworkChooser` — clicking Settings IS the Network Chooser. Testers will look for a separate navigation path and may skip step 3 or get lost. Reword as 'Stay on the Settings screen (this is the Network Chooser)…' or collapse into one navigation step. TC-NET-005-01 step 4 has the same issue.
|
|
||
| ```env | ||
| # Testnet SPV mode — DAPI addresses plus placeholder Core RPC fields | ||
| TESTNET_dapi_addresses=https://34.214.48.68:1443,https://52.12.176.90:1443,https://52.34.144.50:1443,https://44.240.98.102:1443,https://54.201.32.131:1443,https://52.10.229.11:1443,https://52.13.132.146:1443,https://52.40.219.41:1443,https://54.149.33.167:1443,https://35.164.23.245:1443,https://52.33.28.47:1443,https://52.43.13.92:1443,https://52.89.154.48:1443,https://52.24.124.162:1443,https://35.85.21.179:1443,https://54.187.14.232:1443,https://54.68.235.201:1443,https://52.13.250.182:1443 |
There was a problem hiding this comment.
💬 Nitpick: Hardcoded testnet DAPI IP list will drift as evonodes churn
The example .env lists 18 hardcoded testnet DAPI IPs. Testnet evonode addresses change as nodes come and go, so a weekly smoke test that hard-codes them will silently rot. Point testers at the bundled .env.example (auto-copied via copy_env_file_if_not_exists in src/app_dir.rs) or otherwise reference a maintained list rather than embedding the IPs here.
source: ['claude']
53e06a8 to
efd9e3a
Compare
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
Documentation-only PR adding QA smoke tests. Three blocking issues remain: (1) the example .env documents only TESTNET_* keys but DET aborts at startup without a Mainnet config; (2) TC-NET-001-01 expects all four network options to be visible, but Devnet/Local require their own contexts that the documented setup never creates; (3) TC-IDN-001-01 step 5 names UI labels that do not exist in the code, and TC-DPY-001-01 skips the mandatory Refresh + Create Profile path for a brand-new identity. Six smaller doc-precision items (advanced-options-only headings, missing hint text, vague navigation, missing funding-method step, hardcoded DAPI IP list) are flagged as non-blocking.
Reviewed commit: efd9e3a
🔴 4 blocking | 🟡 4 suggestion(s) | 💬 1 nitpick(s)
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `docs/test-prerequisites.md`:
- [BLOCKING] lines 78-91: Example .env omits MAINNET_* keys — DET refuses to start with the documented config
The prerequisites snippet defines only TESTNET_* variables, but `AppState::new` unconditionally builds a Mainnet `AppContext` and calls `std::process::exit(1)` if it can't be created (src/app.rs:189-202). `AppContext::new` returns `None` whenever `config.config_for_network(Network::Dash)` is absent (src/context/mod.rs:118). A tester copying the snippet verbatim will see DET exit immediately on launch with 'Failed to create the AppContext. Expected Dash config for mainnet.' Add MAINNET_* entries (host/port/user/password/dapi_addresses/insight_api_url at minimum) — placeholders are fine since SPV runs on Testnet, but the keys must be present for deserialization.
In `docs/test-cases.md`:
- [BLOCKING] lines 115-117: TC-NET-001-01 expects Devnet and Local options that the documented setup never creates
Step 4 / Expected Result tells the tester to verify Mainnet, Testnet, Devnet, and Local are all listed in the network chooser. The chooser only renders Testnet/Devnet/Local entries when the matching `AppContext` exists (src/ui/network_chooser_screen.rs:331-363), and each optional context is only created when a matching config block exists (src/context/mod.rs:118; src/app.rs:204-224). The documented .env only contains TESTNET_* keys, so even after adding the required Mainnet block (see related blocker), Devnet and Local will still be absent. Either drop those options from the expected result, or add example DEVNET_*/LOCAL_* blocks to the prerequisites.
- [BLOCKING] line 352: TC-IDN-001-01 step 5 names funding-method labels that don't exist in the UI
Step 5 reads: 'In "Choose your funding method", select "Unused Evo Funding Locks (recommended)".' Neither label exists in the code: the combobox placeholder is 'Select funding method' (`FundingMethod::NoSelection` Display, src/ui/identities/add_new_identity_screen/mod.rs:62) and the option is 'Unused Asset Lock (recommended)' (`UseUnusedAssetLock` Display, :65). There is no heading 'Choose your funding method' anywhere in the source. Replace with the actual labels — e.g., 'Open the "Select funding method" combobox and choose "Unused Asset Lock (recommended)".'
- [BLOCKING] lines 443-453: TC-DPY-001-01 skips the mandatory Refresh + Create Profile path and uses a vague navigation step
Two problems on the same case:
1. Step 2 ('Navigate to the profile editing screen for the test identity') is vague — every other case names exact controls. The DashPay subscreen chooser exposes 'My Profile', 'Contacts', 'Payment History', 'Search Profiles' (src/ui/components/dashpay_subscreen_chooser_panel.rs:57-62). Tell the tester to click 'My Profile'. The same vagueness applies to TC-DPY-002-01 step 2 — direct the tester to 'Search Profiles' before the search field step.
2. For a freshly registered identity, the My Profile screen does not show editable fields. On first open it shows 'No Profile Loaded' and instructs the user to click the top-panel 'Refresh' button (src/ui/dashpay/profile_screen.rs:675-700). Only after Refresh completes with 'no profile' does the 'No DashPay Profile' card appear with a 'Create Profile' button (:1248-1280), and only after clicking that do the Display Name / Bio/Status / Avatar URL / Save Profile controls become available (:719-1009). Insert the explicit 'Refresh' click and 'Create Profile' click between opening My Profile and typing the display name.
- [SUGGESTION] line 44: Import-flow steps reference 'Select what you want to import' heading that is hidden by default
Session Setup §2-§4 (lines 44, 55, 64) and TC-WAL-002-01 step 4 (line 184) tell testers to choose 'Seed Phrase (HD Wallet)' under 'Select what you want to import'. That heading is only rendered when `show_advanced_options` is true (src/ui/wallets/import_mnemonic_screen.rs:544-547), and `ImportMnemonicScreen::new` defaults `show_advanced_options: false` (:72). With advanced options off, the import-type selector is not visible and the screen jumps straight to '1. Select the seed phrase length and enter all words.' (:577). Either drop the import-type step (mnemonic is the default) or tell the tester to check 'Show Advanced Options' first.
- [SUGGESTION] line 365: TC-IDN-004-01 omits the funding-method selection step on the Top Up Identity screen
The top-up screen renders the same funding-method combobox as registration (src/ui/identities/top_up_identity_screen/mod.rs:241-283), with options 'Please select funding method', 'Unused Asset Locks', 'Wallet Balance', 'Platform Address'. The tester must pick one before the amount/confirm step is reachable. The case currently jumps directly from 'Navigate to top-up' to 'Enter amount' to 'Confirm the top-up transaction'. Add a step that names the actual combobox and the choice (e.g., 'Open the funding-method dropdown and choose "Wallet Balance" (or "Unused Asset Locks" if one is available).').
- [SUGGESTION] line 378: TC-IDN-008-01 step 3 'Navigate to the keys/details view' is too vague
Other cases name exact buttons or breadcrumbs (TC-WAL-005-01, TC-IDN-001-01, TC-IDN-009-01); this step asks the tester to guess. Two testers will take different paths to satisfy it. Replace with the concrete entry point — e.g., 'On the Identities screen, click the keys/details button on the row for the test identity', and quote the actual button label from `identities_screen.rs`.
- [SUGGESTION] line 507: TC-TOK-003-01 step 4 references a hint text the Add Token screen does not render
Step 4 reads: 'In the input field ("Enter either a Contract ID or Token ID"), enter a known testnet token contract ID.' The actual field is labeled 'Contract or Token ID:' (src/ui/tokens/add_token_by_id_screen.rs:68) and the `text_edit_singleline` call (:69) has no `.hint_text(...)`, so no italic placeholder text appears. A literal tester will hunt for the parenthetical hint and not find it. Replace with the real label, e.g., 'In the "Contract or Token ID:" field, enter a known testnet token contract ID.'
| ```env | ||
| # Testnet SPV mode — DAPI addresses plus placeholder Core RPC fields | ||
| TESTNET_dapi_addresses=https://34.214.48.68:1443,https://52.12.176.90:1443,https://52.34.144.50:1443,https://44.240.98.102:1443,https://54.201.32.131:1443,https://52.10.229.11:1443,https://52.13.132.146:1443,https://52.40.219.41:1443,https://54.149.33.167:1443,https://35.164.23.245:1443,https://52.33.28.47:1443,https://52.43.13.92:1443,https://52.89.154.48:1443,https://52.24.124.162:1443,https://35.85.21.179:1443,https://54.187.14.232:1443,https://54.68.235.201:1443,https://52.13.250.182:1443 | ||
| TESTNET_show_in_ui=true | ||
|
|
||
| # Core RPC fields are structurally required by NetworkConfig even in SPV mode, | ||
| # but are not actively used at runtime. Supply placeholder values. | ||
| TESTNET_core_host=127.0.0.1 | ||
| TESTNET_core_rpc_port=19998 | ||
| TESTNET_core_rpc_user=user | ||
| TESTNET_core_rpc_password=password | ||
| TESTNET_insight_api_url=https://insight.testnet.networks.dash.org:3002/insight-api | ||
| # core_zmq_endpoint is optional and can be omitted | ||
| ``` |
There was a problem hiding this comment.
🔴 Blocking: Example .env omits MAINNET_ keys — DET refuses to start with the documented config*
The prerequisites snippet defines only TESTNET_* variables, but AppState::new unconditionally builds a Mainnet AppContext and calls std::process::exit(1) if it can't be created (src/app.rs:189-202). AppContext::new returns None whenever config.config_for_network(Network::Dash) is absent (src/context/mod.rs:118). A tester copying the snippet verbatim will see DET exit immediately on launch with 'Failed to create the AppContext. Expected Dash config for mainnet.' Add MAINNET_* entries (host/port/user/password/dapi_addresses/insight_api_url at minimum) — placeholders are fine since SPV runs on Testnet, but the keys must be present for deserialization.
source: ['codex']
🤖 Fix this with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `docs/test-prerequisites.md`:
- [BLOCKING] lines 78-91: Example .env omits MAINNET_* keys — DET refuses to start with the documented config
The prerequisites snippet defines only TESTNET_* variables, but `AppState::new` unconditionally builds a Mainnet `AppContext` and calls `std::process::exit(1)` if it can't be created (src/app.rs:189-202). `AppContext::new` returns `None` whenever `config.config_for_network(Network::Dash)` is absent (src/context/mod.rs:118). A tester copying the snippet verbatim will see DET exit immediately on launch with 'Failed to create the AppContext. Expected Dash config for mainnet.' Add MAINNET_* entries (host/port/user/password/dapi_addresses/insight_api_url at minimum) — placeholders are fine since SPV runs on Testnet, but the keys must be present for deserialization.
| | **Test Steps** | 1. Click **"Settings"** in the left sidebar.<br>2. Locate the network chooser at the top of the settings screen.<br>3. Verify **"Testnet"** is the currently selected network.<br>4. Verify the other options are listed: **"Mainnet"**, **"Devnet"**, **"Local"**.<br>5. Do NOT switch networks. | | ||
| | **Test Data** | N/A | | ||
| | **Expected Result** | **"Testnet"** is selected. All four network options (**"Mainnet"**, **"Testnet"**, **"Devnet"**, **"Local"**) are visible. | |
There was a problem hiding this comment.
🔴 Blocking: TC-NET-001-01 expects Devnet and Local options that the documented setup never creates
Step 4 / Expected Result tells the tester to verify Mainnet, Testnet, Devnet, and Local are all listed in the network chooser. The chooser only renders Testnet/Devnet/Local entries when the matching AppContext exists (src/ui/network_chooser_screen.rs:331-363), and each optional context is only created when a matching config block exists (src/context/mod.rs:118; src/app.rs:204-224). The documented .env only contains TESTNET_* keys, so even after adding the required Mainnet block (see related blocker), Devnet and Local will still be absent. Either drop those options from the expected result, or add example DEVNET_/LOCAL_ blocks to the prerequisites.
source: ['codex']
🤖 Fix this with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `docs/test-cases.md`:
- [BLOCKING] lines 115-117: TC-NET-001-01 expects Devnet and Local options that the documented setup never creates
Step 4 / Expected Result tells the tester to verify Mainnet, Testnet, Devnet, and Local are all listed in the network chooser. The chooser only renders Testnet/Devnet/Local entries when the matching `AppContext` exists (src/ui/network_chooser_screen.rs:331-363), and each optional context is only created when a matching config block exists (src/context/mod.rs:118; src/app.rs:204-224). The documented .env only contains TESTNET_* keys, so even after adding the required Mainnet block (see related blocker), Devnet and Local will still be absent. Either drop those options from the expected result, or add example DEVNET_*/LOCAL_* blocks to the prerequisites.
| | **Test Case ID** | TC-IDN-001-01 | | ||
| | **Short Description** | Register a new identity funded by asset lock | | ||
| | **Pre-Conditions** | **Identity Test** wallet selected; asset lock from TC-ALK-001-01 is confirmed | | ||
| | **Test Steps** | 1. Click **"Identities"** in the left sidebar.<br>2. Click **"Create Identity"** and verify the breadcrumb shows **Identities > Create Identity**.<br>3. Select the **Identity Test** wallet as the funding source (unlock it if prompted).<br>4. Review the generated identity keys and optional **"Alias:"** field.<br>5. In **"Choose your funding method"**, select **"Unused Evo Funding Locks (recommended)"**.<br>6. Under **"Select an unused asset lock:"**, choose the asset lock created in TC-ALK-001-01 and click **"Select"**.<br>7. Verify the **"Estimated Fee:"** summary, then click **"Create Identity"**.<br>8. Wait through **"=> Waiting for Platform acknowledgement <="** until the success screen appears.<br>9. Verify the new identity appears in the identities list with an identity ID (Base58 string). | |
There was a problem hiding this comment.
🔴 Blocking: TC-IDN-001-01 step 5 names funding-method labels that don't exist in the UI
Step 5 reads: 'In "Choose your funding method", select "Unused Evo Funding Locks (recommended)".' Neither label exists in the code: the combobox placeholder is 'Select funding method' (FundingMethod::NoSelection Display, src/ui/identities/add_new_identity_screen/mod.rs:62) and the option is 'Unused Asset Lock (recommended)' (UseUnusedAssetLock Display, :65). There is no heading 'Choose your funding method' anywhere in the source. Replace with the actual labels — e.g., 'Open the "Select funding method" combobox and choose "Unused Asset Lock (recommended)".'
source: ['claude']
🤖 Fix this with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `docs/test-cases.md`:
- [BLOCKING] line 352: TC-IDN-001-01 step 5 names funding-method labels that don't exist in the UI
Step 5 reads: 'In "Choose your funding method", select "Unused Evo Funding Locks (recommended)".' Neither label exists in the code: the combobox placeholder is 'Select funding method' (`FundingMethod::NoSelection` Display, src/ui/identities/add_new_identity_screen/mod.rs:62) and the option is 'Unused Asset Lock (recommended)' (`UseUnusedAssetLock` Display, :65). There is no heading 'Choose your funding method' anywhere in the source. Replace with the actual labels — e.g., 'Open the "Select funding method" combobox and choose "Unused Asset Lock (recommended)".'
| ### TC-DPY-001-01: Create a DashPay profile | ||
|
|
||
| | Field | Value | | ||
| |-------|-------| | ||
| | **Use Case ID** | DPY-001 | | ||
| | **Test Case ID** | TC-DPY-001-01 | | ||
| | **Short Description** | Create a DashPay profile for the test identity | | ||
| | **Pre-Conditions** | Identity from TC-IDN-001-01 with DPNS username and sufficient credits | | ||
| | **Test Steps** | 1. Click **"Dashpay"** in the left sidebar.<br>2. Navigate to the profile editing screen for the test identity.<br>3. In the **"Display Name:"** field (hint: *"Enter your display name (required)"*), type `Smoke Test Bot`.<br>4. In the **"Bio/Status:"** field, type `Automated QA testing`.<br>5. Leave the **"Avatar URL:"** field empty.<br>6. Save/publish the profile.<br>7. Wait for Platform confirmation.<br>8. Verify the profile screen shows **Display Name:** `Smoke Test Bot` and **Bio/Status:** `Automated QA testing`. | | ||
| | **Test Data** | Display Name: `Smoke Test Bot`, Bio: `Automated QA testing` | | ||
| | **Expected Result** | Profile created. Display name and bio visible on the profile screen. | |
There was a problem hiding this comment.
🔴 Blocking: TC-DPY-001-01 skips the mandatory Refresh + Create Profile path and uses a vague navigation step
Two problems on the same case:
-
Step 2 ('Navigate to the profile editing screen for the test identity') is vague — every other case names exact controls. The DashPay subscreen chooser exposes 'My Profile', 'Contacts', 'Payment History', 'Search Profiles' (src/ui/components/dashpay_subscreen_chooser_panel.rs:57-62). Tell the tester to click 'My Profile'. The same vagueness applies to TC-DPY-002-01 step 2 — direct the tester to 'Search Profiles' before the search field step.
-
For a freshly registered identity, the My Profile screen does not show editable fields. On first open it shows 'No Profile Loaded' and instructs the user to click the top-panel 'Refresh' button (src/ui/dashpay/profile_screen.rs:675-700). Only after Refresh completes with 'no profile' does the 'No DashPay Profile' card appear with a 'Create Profile' button (:1248-1280), and only after clicking that do the Display Name / Bio/Status / Avatar URL / Save Profile controls become available (:719-1009). Insert the explicit 'Refresh' click and 'Create Profile' click between opening My Profile and typing the display name.
source: ['claude', 'codex']
🤖 Fix this with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `docs/test-cases.md`:
- [BLOCKING] lines 443-453: TC-DPY-001-01 skips the mandatory Refresh + Create Profile path and uses a vague navigation step
Two problems on the same case:
1. Step 2 ('Navigate to the profile editing screen for the test identity') is vague — every other case names exact controls. The DashPay subscreen chooser exposes 'My Profile', 'Contacts', 'Payment History', 'Search Profiles' (src/ui/components/dashpay_subscreen_chooser_panel.rs:57-62). Tell the tester to click 'My Profile'. The same vagueness applies to TC-DPY-002-01 step 2 — direct the tester to 'Search Profiles' before the search field step.
2. For a freshly registered identity, the My Profile screen does not show editable fields. On first open it shows 'No Profile Loaded' and instructs the user to click the top-panel 'Refresh' button (src/ui/dashpay/profile_screen.rs:675-700). Only after Refresh completes with 'no profile' does the 'No DashPay Profile' card appear with a 'Create Profile' button (:1248-1280), and only after clicking that do the Display Name / Bio/Status / Avatar URL / Save Profile controls become available (:719-1009). Insert the explicit 'Refresh' click and 'Create Profile' click between opening My Profile and typing the display name.
|
|
||
| 1. Click **"Wallets"** in the left sidebar. | ||
| 2. Click **"Import Wallet"** (top-right). | ||
| 3. Under **"Select what you want to import"**, choose **"Seed Phrase (HD Wallet)"**. |
There was a problem hiding this comment.
🟡 Suggestion: Import-flow steps reference 'Select what you want to import' heading that is hidden by default
Session Setup §2-§4 (lines 44, 55, 64) and TC-WAL-002-01 step 4 (line 184) tell testers to choose 'Seed Phrase (HD Wallet)' under 'Select what you want to import'. That heading is only rendered when show_advanced_options is true (src/ui/wallets/import_mnemonic_screen.rs:544-547), and ImportMnemonicScreen::new defaults show_advanced_options: false (:72). With advanced options off, the import-type selector is not visible and the screen jumps straight to '1. Select the seed phrase length and enter all words.' (:577). Either drop the import-type step (mnemonic is the default) or tell the tester to check 'Show Advanced Options' first.
source: ['claude']
🤖 Fix this with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `docs/test-cases.md`:
- [SUGGESTION] line 44: Import-flow steps reference 'Select what you want to import' heading that is hidden by default
Session Setup §2-§4 (lines 44, 55, 64) and TC-WAL-002-01 step 4 (line 184) tell testers to choose 'Seed Phrase (HD Wallet)' under 'Select what you want to import'. That heading is only rendered when `show_advanced_options` is true (src/ui/wallets/import_mnemonic_screen.rs:544-547), and `ImportMnemonicScreen::new` defaults `show_advanced_options: false` (:72). With advanced options off, the import-type selector is not visible and the screen jumps straight to '1. Select the seed phrase length and enter all words.' (:577). Either drop the import-type step (mnemonic is the default) or tell the tester to check 'Show Advanced Options' first.
| | **Test Case ID** | TC-IDN-004-01 | | ||
| | **Short Description** | Add credits to an existing identity | | ||
| | **Pre-Conditions** | Identity from TC-IDN-001-01 exists; **Identity Test** wallet has remaining funds | | ||
| | **Test Steps** | 1. Click **"Identities"** in the left sidebar.<br>2. Select the identity created in TC-IDN-001-01.<br>3. Navigate to top-up (breadcrumb: **Top Up Identity**).<br>4. Enter amount: `0.1` tDASH worth of credits.<br>5. Confirm the top-up transaction.<br>6. Wait for Platform confirmation.<br>7. Verify the identity's credit balance increased. | |
There was a problem hiding this comment.
🟡 Suggestion: TC-IDN-004-01 omits the funding-method selection step on the Top Up Identity screen
The top-up screen renders the same funding-method combobox as registration (src/ui/identities/top_up_identity_screen/mod.rs:241-283), with options 'Please select funding method', 'Unused Asset Locks', 'Wallet Balance', 'Platform Address'. The tester must pick one before the amount/confirm step is reachable. The case currently jumps directly from 'Navigate to top-up' to 'Enter amount' to 'Confirm the top-up transaction'. Add a step that names the actual combobox and the choice (e.g., 'Open the funding-method dropdown and choose "Wallet Balance" (or "Unused Asset Locks" if one is available).').
source: ['claude']
🤖 Fix this with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `docs/test-cases.md`:
- [SUGGESTION] line 365: TC-IDN-004-01 omits the funding-method selection step on the Top Up Identity screen
The top-up screen renders the same funding-method combobox as registration (src/ui/identities/top_up_identity_screen/mod.rs:241-283), with options 'Please select funding method', 'Unused Asset Locks', 'Wallet Balance', 'Platform Address'. The tester must pick one before the amount/confirm step is reachable. The case currently jumps directly from 'Navigate to top-up' to 'Enter amount' to 'Confirm the top-up transaction'. Add a step that names the actual combobox and the choice (e.g., 'Open the funding-method dropdown and choose "Wallet Balance" (or "Unused Asset Locks" if one is available).').
| | **Test Case ID** | TC-IDN-008-01 | | ||
| | **Short Description** | Inspect identity key list and details | | ||
| | **Pre-Conditions** | Identity from TC-IDN-001-01 exists | | ||
| | **Test Steps** | 1. Click **"Identities"** in the left sidebar.<br>2. Select the identity from TC-IDN-001-01.<br>3. Navigate to the keys/details view.<br>4. Verify a key list is displayed.<br>5. Verify each key shows: type, purpose (e.g., AUTHENTICATION), and status. | |
There was a problem hiding this comment.
🟡 Suggestion: TC-IDN-008-01 step 3 'Navigate to the keys/details view' is too vague
Other cases name exact buttons or breadcrumbs (TC-WAL-005-01, TC-IDN-001-01, TC-IDN-009-01); this step asks the tester to guess. Two testers will take different paths to satisfy it. Replace with the concrete entry point — e.g., 'On the Identities screen, click the keys/details button on the row for the test identity', and quote the actual button label from identities_screen.rs.
source: ['claude']
🤖 Fix this with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `docs/test-cases.md`:
- [SUGGESTION] line 378: TC-IDN-008-01 step 3 'Navigate to the keys/details view' is too vague
Other cases name exact buttons or breadcrumbs (TC-WAL-005-01, TC-IDN-001-01, TC-IDN-009-01); this step asks the tester to guess. Two testers will take different paths to satisfy it. Replace with the concrete entry point — e.g., 'On the Identities screen, click the keys/details button on the row for the test identity', and quote the actual button label from `identities_screen.rs`.
| | **Test Case ID** | TC-TOK-003-01 | | ||
| | **Short Description** | Add a token using a contract or token ID | | ||
| | **Pre-Conditions** | DET running on testnet, a known token contract ID available | | ||
| | **Test Steps** | 1. Click **"Tokens"** in the left sidebar.<br>2. Click the **"Add Token"** button.<br>3. Verify the heading reads **"Add Token"**.<br>4. In the input field (*"Enter either a Contract ID or Token ID"*), enter a known testnet token contract ID.<br>5. Click **"Search"** and select the desired token if multiple results are shown.<br>6. Click **"Add Token"** and verify the **"Token Added Successfully"** screen appears.<br>7. Click **"Back to Tokens screen"**, open **"Search Tokens"**, and verify the added token/contract can be found by its name or ID. | |
There was a problem hiding this comment.
🟡 Suggestion: TC-TOK-003-01 step 4 references a hint text the Add Token screen does not render
Step 4 reads: 'In the input field ("Enter either a Contract ID or Token ID"), enter a known testnet token contract ID.' The actual field is labeled 'Contract or Token ID:' (src/ui/tokens/add_token_by_id_screen.rs:68) and the text_edit_singleline call (:69) has no .hint_text(...), so no italic placeholder text appears. A literal tester will hunt for the parenthetical hint and not find it. Replace with the real label, e.g., 'In the "Contract or Token ID:" field, enter a known testnet token contract ID.'
source: ['claude']
🤖 Fix this with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `docs/test-cases.md`:
- [SUGGESTION] line 507: TC-TOK-003-01 step 4 references a hint text the Add Token screen does not render
Step 4 reads: 'In the input field ("Enter either a Contract ID or Token ID"), enter a known testnet token contract ID.' The actual field is labeled 'Contract or Token ID:' (src/ui/tokens/add_token_by_id_screen.rs:68) and the `text_edit_singleline` call (:69) has no `.hint_text(...)`, so no italic placeholder text appears. A literal tester will hunt for the parenthetical hint and not find it. Replace with the real label, e.g., 'In the "Contract or Token ID:" field, enter a known testnet token contract ID.'
|
|
||
| ```env | ||
| # Testnet SPV mode — DAPI addresses plus placeholder Core RPC fields | ||
| TESTNET_dapi_addresses=https://34.214.48.68:1443,https://52.12.176.90:1443,https://52.34.144.50:1443,https://44.240.98.102:1443,https://54.201.32.131:1443,https://52.10.229.11:1443,https://52.13.132.146:1443,https://52.40.219.41:1443,https://54.149.33.167:1443,https://35.164.23.245:1443,https://52.33.28.47:1443,https://52.43.13.92:1443,https://52.89.154.48:1443,https://52.24.124.162:1443,https://35.85.21.179:1443,https://54.187.14.232:1443,https://54.68.235.201:1443,https://52.13.250.182:1443 |
There was a problem hiding this comment.
💬 Nitpick: Hardcoded testnet DAPI IP list will drift as evonodes churn
The example .env embeds 18 specific testnet DAPI IPs. Testnet evonode addresses change as nodes come and go, so a weekly smoke test that hard-codes them will silently rot. Point testers at the bundled .env.example (auto-copied via copy_env_file_if_not_exists in src/app_dir.rs) or to a maintained list rather than inlining the IPs.
source: ['claude']
efd9e3a to
23ebe6c
Compare
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
Five blocking issues remain on the latest head 23ebe6c. Two are independent, blocking flaws in Session Setup §1: (a) the steps tell the tester to find the Developer-mode checkbox and the Connection Type selector in Settings, but the Developer-mode checkbox lives inside an 'Advanced Settings' card that is force-closed on screen open, so the tester cannot reach the Connection Type dropdown at all; and (b) DET defaults to Mainnet (Settings::default = Network::Dash) and the setup never tells the tester to switch the Network: combobox to Testnet, so even after working around (a) the SPV connect step would run against placeholder Mainnet DAPI hosts. TC-IDN-001-01 names the wrong funding-method option label, TC-TOK-003-01's verification path uses a keyword-only search tab that does not match by name or ID, and the Windows .env path is missing the \config\ segment that directories 6.0 inserts. One suggestion (the entropy grid has no progress bar) and one nitpick (hardcoded testnet DAPI IPs) round out the findings.
Reviewed commit: 23ebe6c
🔴 5 blocking | 🟡 1 suggestion(s)
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `docs/test-cases.md`:
- [BLOCKING] lines 32-38: Session Setup §1 never switches the app from the default Mainnet to Testnet
The suite is documented as 'SPV mode on testnet,' but Session Setup §1 only opens Settings, enables Developer mode, and picks 'SPV Client.' It never touches the separate 'Network:' selector. On a fresh state DET defaults to Mainnet — `Settings::default()` sets `network` to `Network::Dash` (`src/model/settings.rs:104`), and the Connection Type dropdown writes the backend mode for `self.current_network` while the Network selector at `src/ui/network_chooser_screen.rs:315-340` is a separate combobox. As written, QA will enable SPV on Mainnet, not Testnet, and step 5's 'wait for the connection status indicator to turn green' will not succeed with the prereq's placeholder `MAINNET_dapi_addresses=https://mainnet-dapi.example:443`. Add an explicit step under Connection Settings: 'Locate the "Network:" dropdown and select "Testnet"' before selecting SPV Client.
- [BLOCKING] lines 32-39: Theme, Developer mode, and the Dash-QT controls live inside the collapsed 'Advanced Settings' card — tests don't tell the tester to expand it
Session Setup §1 step 2, TC-NET-004-01 step 2 (line 128), TC-NET-005-01 step 2 (line 141), and the prereq 'Enable Developer Mode' step 2 (`docs/test-prerequisites.md:112`) all instruct the tester to 'Locate the "Developer mode" checkbox' or 'Locate the "Theme:" selector' immediately after opening Settings. Both controls live inside an 'Advanced Settings' card whose body is built via `CollapsingState::load_with_default_open(ctx, id, false)` (`src/ui/network_chooser_screen.rs:664-668`) and is explicitly forced closed each time the screen opens by `state.set_open(false)` (`src/ui/network_chooser_screen.rs:671-673`). The Theme combobox (line 748) and Developer-mode checkbox (line 936) are both inside `state.show_body_unindented(...)` (line 738), invisible until the 'Advanced Settings' header is clicked. Without expanding the section first the tester cannot enable Developer mode, which means the entire Connection Type / SPV gating in Session Setup is unreachable. Insert a step before each control-location step: 'Click the "Advanced Settings" header to expand it.'
- [BLOCKING] line 352: TC-IDN-001-01 step 5 names a funding-method option that does not exist in the dropdown
Step 5 says: 'Open the "Select funding method" combobox and choose "Unused Asset Lock (recommended)".' The combobox placeholder/selected_text matches that string via `FundingMethod`'s Display impl (`src/ui/identities/add_new_identity_screen/mod.rs:62-65`), but the actual clickable option for the register-identity flow is hardcoded with a different label: `selectable_value(&mut *funding_method, FundingMethod::UseUnusedAssetLock, "Unused Evo Funding Locks (recommended)")` at `src/ui/identities/add_new_identity_screen/mod.rs:457`. The tester will open the dropdown and see 'Unused Evo Funding Locks (recommended)' (plus 'Wallet Balance' / 'Address with QR Code' / 'Platform Address'), not the label this step tells them to click. Note this only affects the register-identity screen — top-up uses 'Unused Asset Locks' (plural), which TC-IDN-004-01 already matches. Replace the option name in step 5 with 'Unused Evo Funding Locks (recommended)'.
- [BLOCKING] lines 507-509: TC-TOK-003-01 verifies through 'Search Tokens', but that tab only supports keyword search
Step 7 and the expected result say the added token/contract should be findable from the 'Search Tokens' tab 'by its name or ID.' The implementation does not support that. The Search Tokens UI is keyword-only: it is headed 'Search Tokens by Keyword', labels its field 'Enter Keyword:', and dispatches `TokenTask::QueryDescriptionsByKeyword(...)` on search (`src/ui/tokens/tokens_screen/keyword_search.rs:33-74`). There is no path there for contract IDs, token IDs, or the locally tracked-token list. The add-token flow itself just saves the contract/token and refreshes balances (`src/ui/tokens/add_token_by_id_screen.rs:130-155`). QA can successfully add a token and still fail the verification because the documented post-check path is unsupported by the UI. Change the post-check to a screen that actually reflects locally tracked tokens/contracts (e.g., the contracts list or the local tokens view after holding a balance), not the keyword-search tab.
- [SUGGESTION] line 171: TC-WAL-001-01 step 4 references a 'progress bar' the entropy grid does not render
Step 4 says: 'Step 1 — Entropy: Move the mouse around the entropy grid until the progress bar fills.' `U256EntropyGrid::ui` (`src/ui/components/entropy_grid.rs:30-102`) renders a single heading ('1. Move your cursor over this grid to create extra randomness for your wallet\'s seed phrase.') and an 8×32 grid of clickable bit cells — there is no progress bar, no fill state, and the Generate button on the next heading is independently enabled. Rewrite step 4 around the actual UI, e.g.: 'Hover or click within the entropy grid (the 256-cell array under the heading "1. Move your cursor over this grid to create extra randomness for your wallet\'s seed phrase") to introduce additional entropy.'
In `docs/test-prerequisites.md`:
- [BLOCKING] line 76: Windows `.env` path is missing the `\config\` segment that `directories` 6.0 inserts
The documented Windows path is `C:\Users\<User>\AppData\Roaming\Dash-Evo-Tool\.env`. DET locates this file via `app_user_data_dir_path()` (`src/app_dir.rs:27`), which calls `ProjectDirs::from("", "", "Dash-Evo-Tool").config_dir()`. `directories = "6.0.0"` returns `{FOLDERID_RoamingAppData}\<project_path>\config` on Windows, so the real path is `C:\Users\<User>\AppData\Roaming\Dash-Evo-Tool\config\.env`. The project's own `CLAUDE.md` documents this same `...\Dash-Evo-Tool\config\.env` location. A Windows tester who follows the prerequisites will drop the `.env` one directory above where DET reads it; the config will be silently ignored and the smoke run starts with no testnet at all.
| ### 1. Configure SPV Mode | ||
|
|
||
| 1. Click **"Settings"** in the left sidebar. | ||
| 2. Check the **"Developer mode"** checkbox. | ||
| 3. Stay on **Settings (Network Chooser)** and verify the **"Connection Type"** dropdown appears in **"Connection Settings"**. | ||
| 4. In **"Connection Settings"**, open the **"Connection Type"** dropdown and select **"SPV Client"**. | ||
| 5. Wait for the connection status indicator (top bar) to turn green. |
There was a problem hiding this comment.
🔴 Blocking: Session Setup §1 never switches the app from the default Mainnet to Testnet
The suite is documented as 'SPV mode on testnet,' but Session Setup §1 only opens Settings, enables Developer mode, and picks 'SPV Client.' It never touches the separate 'Network:' selector. On a fresh state DET defaults to Mainnet — Settings::default() sets network to Network::Dash (src/model/settings.rs:104), and the Connection Type dropdown writes the backend mode for self.current_network while the Network selector at src/ui/network_chooser_screen.rs:315-340 is a separate combobox. As written, QA will enable SPV on Mainnet, not Testnet, and step 5's 'wait for the connection status indicator to turn green' will not succeed with the prereq's placeholder MAINNET_dapi_addresses=https://mainnet-dapi.example:443. Add an explicit step under Connection Settings: 'Locate the "Network:" dropdown and select "Testnet"' before selecting SPV Client.
source: ['codex']
🤖 Fix this with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `docs/test-cases.md`:
- [BLOCKING] lines 32-38: Session Setup §1 never switches the app from the default Mainnet to Testnet
The suite is documented as 'SPV mode on testnet,' but Session Setup §1 only opens Settings, enables Developer mode, and picks 'SPV Client.' It never touches the separate 'Network:' selector. On a fresh state DET defaults to Mainnet — `Settings::default()` sets `network` to `Network::Dash` (`src/model/settings.rs:104`), and the Connection Type dropdown writes the backend mode for `self.current_network` while the Network selector at `src/ui/network_chooser_screen.rs:315-340` is a separate combobox. As written, QA will enable SPV on Mainnet, not Testnet, and step 5's 'wait for the connection status indicator to turn green' will not succeed with the prereq's placeholder `MAINNET_dapi_addresses=https://mainnet-dapi.example:443`. Add an explicit step under Connection Settings: 'Locate the "Network:" dropdown and select "Testnet"' before selecting SPV Client.
| ### 1. Configure SPV Mode | ||
|
|
||
| 1. Click **"Settings"** in the left sidebar. | ||
| 2. Check the **"Developer mode"** checkbox. | ||
| 3. Stay on **Settings (Network Chooser)** and verify the **"Connection Type"** dropdown appears in **"Connection Settings"**. | ||
| 4. In **"Connection Settings"**, open the **"Connection Type"** dropdown and select **"SPV Client"**. | ||
| 5. Wait for the connection status indicator (top bar) to turn green. | ||
|
|
There was a problem hiding this comment.
🔴 Blocking: Theme, Developer mode, and the Dash-QT controls live inside the collapsed 'Advanced Settings' card — tests don't tell the tester to expand it
Session Setup §1 step 2, TC-NET-004-01 step 2 (line 128), TC-NET-005-01 step 2 (line 141), and the prereq 'Enable Developer Mode' step 2 (docs/test-prerequisites.md:112) all instruct the tester to 'Locate the "Developer mode" checkbox' or 'Locate the "Theme:" selector' immediately after opening Settings. Both controls live inside an 'Advanced Settings' card whose body is built via CollapsingState::load_with_default_open(ctx, id, false) (src/ui/network_chooser_screen.rs:664-668) and is explicitly forced closed each time the screen opens by state.set_open(false) (src/ui/network_chooser_screen.rs:671-673). The Theme combobox (line 748) and Developer-mode checkbox (line 936) are both inside state.show_body_unindented(...) (line 738), invisible until the 'Advanced Settings' header is clicked. Without expanding the section first the tester cannot enable Developer mode, which means the entire Connection Type / SPV gating in Session Setup is unreachable. Insert a step before each control-location step: 'Click the "Advanced Settings" header to expand it.'
source: ['claude']
🤖 Fix this with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `docs/test-cases.md`:
- [BLOCKING] lines 32-39: Theme, Developer mode, and the Dash-QT controls live inside the collapsed 'Advanced Settings' card — tests don't tell the tester to expand it
Session Setup §1 step 2, TC-NET-004-01 step 2 (line 128), TC-NET-005-01 step 2 (line 141), and the prereq 'Enable Developer Mode' step 2 (`docs/test-prerequisites.md:112`) all instruct the tester to 'Locate the "Developer mode" checkbox' or 'Locate the "Theme:" selector' immediately after opening Settings. Both controls live inside an 'Advanced Settings' card whose body is built via `CollapsingState::load_with_default_open(ctx, id, false)` (`src/ui/network_chooser_screen.rs:664-668`) and is explicitly forced closed each time the screen opens by `state.set_open(false)` (`src/ui/network_chooser_screen.rs:671-673`). The Theme combobox (line 748) and Developer-mode checkbox (line 936) are both inside `state.show_body_unindented(...)` (line 738), invisible until the 'Advanced Settings' header is clicked. Without expanding the section first the tester cannot enable Developer mode, which means the entire Connection Type / SPV gating in Session Setup is unreachable. Insert a step before each control-location step: 'Click the "Advanced Settings" header to expand it.'
| | **Test Case ID** | TC-IDN-001-01 | | ||
| | **Short Description** | Register a new identity funded by asset lock | | ||
| | **Pre-Conditions** | **Identity Test** wallet selected; asset lock from TC-ALK-001-01 is confirmed | | ||
| | **Test Steps** | 1. Click **"Identities"** in the left sidebar.<br>2. Click **"Create Identity"** and verify the breadcrumb shows **Identities > Create Identity**.<br>3. Select the **Identity Test** wallet as the funding source (unlock it if prompted).<br>4. Review the generated identity keys and optional **"Alias:"** field.<br>5. Open the **"Select funding method"** combobox and choose **"Unused Asset Lock (recommended)"**.<br>6. Under **"Select an unused asset lock:"**, choose the asset lock created in TC-ALK-001-01 and click **"Select"**.<br>7. Verify the **"Estimated Fee:"** summary, then click **"Create Identity"**.<br>8. Wait through **"=> Waiting for Platform acknowledgement <="** until the success screen appears.<br>9. Verify the new identity appears in the identities list with an identity ID (Base58 string). | |
There was a problem hiding this comment.
🔴 Blocking: TC-IDN-001-01 step 5 names a funding-method option that does not exist in the dropdown
Step 5 says: 'Open the "Select funding method" combobox and choose "Unused Asset Lock (recommended)".' The combobox placeholder/selected_text matches that string via FundingMethod's Display impl (src/ui/identities/add_new_identity_screen/mod.rs:62-65), but the actual clickable option for the register-identity flow is hardcoded with a different label: selectable_value(&mut *funding_method, FundingMethod::UseUnusedAssetLock, "Unused Evo Funding Locks (recommended)") at src/ui/identities/add_new_identity_screen/mod.rs:457. The tester will open the dropdown and see 'Unused Evo Funding Locks (recommended)' (plus 'Wallet Balance' / 'Address with QR Code' / 'Platform Address'), not the label this step tells them to click. Note this only affects the register-identity screen — top-up uses 'Unused Asset Locks' (plural), which TC-IDN-004-01 already matches. Replace the option name in step 5 with 'Unused Evo Funding Locks (recommended)'.
source: ['claude']
🤖 Fix this with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `docs/test-cases.md`:
- [BLOCKING] line 352: TC-IDN-001-01 step 5 names a funding-method option that does not exist in the dropdown
Step 5 says: 'Open the "Select funding method" combobox and choose "Unused Asset Lock (recommended)".' The combobox placeholder/selected_text matches that string via `FundingMethod`'s Display impl (`src/ui/identities/add_new_identity_screen/mod.rs:62-65`), but the actual clickable option for the register-identity flow is hardcoded with a different label: `selectable_value(&mut *funding_method, FundingMethod::UseUnusedAssetLock, "Unused Evo Funding Locks (recommended)")` at `src/ui/identities/add_new_identity_screen/mod.rs:457`. The tester will open the dropdown and see 'Unused Evo Funding Locks (recommended)' (plus 'Wallet Balance' / 'Address with QR Code' / 'Platform Address'), not the label this step tells them to click. Note this only affects the register-identity screen — top-up uses 'Unused Asset Locks' (plural), which TC-IDN-004-01 already matches. Replace the option name in step 5 with 'Unused Evo Funding Locks (recommended)'.
| | **Test Steps** | 1. Click **"Tokens"** in the left sidebar.<br>2. Click the **"Add Token"** button.<br>3. Verify the heading reads **"Add Token"**.<br>4. In the **"Contract or Token ID:"** field, enter a known testnet token contract ID.<br>5. Click **"Search"** and select the desired token if multiple results are shown.<br>6. Click **"Add Token"** and verify the **"Token Added Successfully"** screen appears.<br>7. Click **"Back to Tokens screen"**, open **"Search Tokens"**, and verify the added token/contract can be found by its name or ID. | | ||
| | **Test Data** | A valid token contract ID from testnet | | ||
| | **Expected Result** | Token is saved/tracked locally and searchable by name or ID. **"My Tokens"** only shows the token if one of the local identities holds a non-zero balance. | |
There was a problem hiding this comment.
🔴 Blocking: TC-TOK-003-01 verifies through 'Search Tokens', but that tab only supports keyword search
Step 7 and the expected result say the added token/contract should be findable from the 'Search Tokens' tab 'by its name or ID.' The implementation does not support that. The Search Tokens UI is keyword-only: it is headed 'Search Tokens by Keyword', labels its field 'Enter Keyword:', and dispatches TokenTask::QueryDescriptionsByKeyword(...) on search (src/ui/tokens/tokens_screen/keyword_search.rs:33-74). There is no path there for contract IDs, token IDs, or the locally tracked-token list. The add-token flow itself just saves the contract/token and refreshes balances (src/ui/tokens/add_token_by_id_screen.rs:130-155). QA can successfully add a token and still fail the verification because the documented post-check path is unsupported by the UI. Change the post-check to a screen that actually reflects locally tracked tokens/contracts (e.g., the contracts list or the local tokens view after holding a balance), not the keyword-search tab.
source: ['codex']
🤖 Fix this with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `docs/test-cases.md`:
- [BLOCKING] lines 507-509: TC-TOK-003-01 verifies through 'Search Tokens', but that tab only supports keyword search
Step 7 and the expected result say the added token/contract should be findable from the 'Search Tokens' tab 'by its name or ID.' The implementation does not support that. The Search Tokens UI is keyword-only: it is headed 'Search Tokens by Keyword', labels its field 'Enter Keyword:', and dispatches `TokenTask::QueryDescriptionsByKeyword(...)` on search (`src/ui/tokens/tokens_screen/keyword_search.rs:33-74`). There is no path there for contract IDs, token IDs, or the locally tracked-token list. The add-token flow itself just saves the contract/token and refreshes balances (`src/ui/tokens/add_token_by_id_screen.rs:130-155`). QA can successfully add a token and still fail the verification because the documented post-check path is unsupported by the UI. Change the post-check to a screen that actually reflects locally tracked tokens/contracts (e.g., the contracts list or the local tokens view after holding a balance), not the keyword-search tab.
| |---|---| | ||
| | **macOS** | `~/Library/Application Support/Dash-Evo-Tool/.env` | | ||
| | **Linux** | `~/.config/dash-evo-tool/.env` | | ||
| | **Windows** | `C:\Users\<User>\AppData\Roaming\Dash-Evo-Tool\.env` | |
There was a problem hiding this comment.
🔴 Blocking: Windows .env path is missing the \config\ segment that directories 6.0 inserts
The documented Windows path is C:\Users\<User>\AppData\Roaming\Dash-Evo-Tool\.env. DET locates this file via app_user_data_dir_path() (src/app_dir.rs:27), which calls ProjectDirs::from("", "", "Dash-Evo-Tool").config_dir(). directories = "6.0.0" returns {FOLDERID_RoamingAppData}\<project_path>\config on Windows, so the real path is C:\Users\<User>\AppData\Roaming\Dash-Evo-Tool\config\.env. The project's own CLAUDE.md documents this same ...\Dash-Evo-Tool\config\.env location. A Windows tester who follows the prerequisites will drop the .env one directory above where DET reads it; the config will be silently ignored and the smoke run starts with no testnet at all.
💡 Suggested change
| | **Windows** | `C:\Users\<User>\AppData\Roaming\Dash-Evo-Tool\.env` | | |
| | **Windows** | `C:\Users\<User>\AppData\Roaming\Dash-Evo-Tool\config\.env` | |
source: ['claude']
🤖 Fix this with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `docs/test-prerequisites.md`:
- [BLOCKING] line 76: Windows `.env` path is missing the `\config\` segment that `directories` 6.0 inserts
The documented Windows path is `C:\Users\<User>\AppData\Roaming\Dash-Evo-Tool\.env`. DET locates this file via `app_user_data_dir_path()` (`src/app_dir.rs:27`), which calls `ProjectDirs::from("", "", "Dash-Evo-Tool").config_dir()`. `directories = "6.0.0"` returns `{FOLDERID_RoamingAppData}\<project_path>\config` on Windows, so the real path is `C:\Users\<User>\AppData\Roaming\Dash-Evo-Tool\config\.env`. The project's own `CLAUDE.md` documents this same `...\Dash-Evo-Tool\config\.env` location. A Windows tester who follows the prerequisites will drop the `.env` one directory above where DET reads it; the config will be silently ignored and the smoke run starts with no testnet at all.
| | **Test Case ID** | TC-WAL-001-01 | | ||
| | **Short Description** | Create a new wallet with generated mnemonic | | ||
| | **Pre-Conditions** | DET running, SPV synced | | ||
| | **Test Steps** | 1. Click **"Wallets"** in the left sidebar.<br>2. Click **"Create Wallet"** in the top-right corner.<br>3. Verify the breadcrumb reads **Wallets > Create Wallet**.<br>4. **Step 1 — Entropy:** Move the mouse around the entropy grid until the progress bar fills.<br>5. **Step 2 — Generate:** Under the heading *"Select your desired seed phrase language and word count and press Generate"*, select **English** and **24 words**, then click **"Generate"**.<br>6. **Step 3 — Record:** Under the heading *"Write down the passphrase on a piece of paper"*, write down all 24 displayed words.<br>7. Check **"I wrote it down"**.<br>8. **Step 4 — Name:** Under the heading *"Enter a wallet name"*, type `Created Wallet` in the **"Wallet Name:"** field.<br>9. **Step 5 — Password:** Under the heading *"Add a password"*, leave the **"Optional Password:"** field empty.<br>10. Click **"Save Wallet"**. | |
There was a problem hiding this comment.
🟡 Suggestion: TC-WAL-001-01 step 4 references a 'progress bar' the entropy grid does not render
Step 4 says: 'Step 1 — Entropy: Move the mouse around the entropy grid until the progress bar fills.' U256EntropyGrid::ui (src/ui/components/entropy_grid.rs:30-102) renders a single heading ('1. Move your cursor over this grid to create extra randomness for your wallet's seed phrase.') and an 8×32 grid of clickable bit cells — there is no progress bar, no fill state, and the Generate button on the next heading is independently enabled. Rewrite step 4 around the actual UI, e.g.: 'Hover or click within the entropy grid (the 256-cell array under the heading "1. Move your cursor over this grid to create extra randomness for your wallet's seed phrase") to introduce additional entropy.'
source: ['claude']
🤖 Fix this with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `docs/test-cases.md`:
- [SUGGESTION] line 171: TC-WAL-001-01 step 4 references a 'progress bar' the entropy grid does not render
Step 4 says: 'Step 1 — Entropy: Move the mouse around the entropy grid until the progress bar fills.' `U256EntropyGrid::ui` (`src/ui/components/entropy_grid.rs:30-102`) renders a single heading ('1. Move your cursor over this grid to create extra randomness for your wallet\'s seed phrase.') and an 8×32 grid of clickable bit cells — there is no progress bar, no fill state, and the Generate button on the next heading is independently enabled. Rewrite step 4 around the actual UI, e.g.: 'Hover or click within the entropy grid (the 256-cell array under the heading "1. Move your cursor over this grid to create extra randomness for your wallet\'s seed phrase") to introduce additional entropy.'
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
PR is documentation-only (smoke test docs). Verified Claude's findings against worktree at 678fdc5 — all five hold up. Three blocking issues remain in the Send-flow steps: tests instruct testers to confirm sends in a 'Fee Confirmation Required' dialog that exists only in single_key_send_screen.rs:561 (not in the HD send_screen.rs used for mnemonic-imported wallets); the field labels/hints ('To:'/'Amount:'/'Enter Dash address'/'0.00') match the advanced multi-output path (line 2173) rather than the default render at lines 1366/1397/1418/1515 ('Send to', 'Enter address (X.../y.../evo1.../tevo1...)', 'Amount', 'Enter amount'); and TC-SND-005-01 asserts a live pre-send 'Fee:' label that does not exist on the HD send screen. One suggestion (network-chooser localization) and one nitpick (hard-coded testnet DAPI IP list) verified.
Reviewed commit: 678fdc5
🔴 3 blocking | 🟡 1 suggestion(s)
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `docs/test-cases.md`:
- [BLOCKING] lines 81-87: HD-wallet Send flow has no 'Fee Confirmation Required' dialog — six test steps will hang
Six steps direct testers to confirm sends in a 'Fee Confirmation Required' dialog: Session Setup §5 step 8 (line 81) and step 14 (line 87), TC-SND-005-01 step 8 (line 307), TC-SND-001-01 step 9 (line 320), Session Cleanup §1 step 7 (line 575) and §2 step 6 (line 585). Grep confirms that window exists at exactly one location: `src/ui/wallets/single_key_send_screen.rs:561` (`egui::Window::new("Fee Confirmation Required")`). Bank, Send Test, and Identity Test are mnemonic-imported HD wallets, so clicking 'Send' routes via `src/ui/wallets/wallets_screen/mod.rs:872-876` to `WalletSendScreen` (`send_screen.rs`), which has zero `Window::new` calls and dispatches `SendWalletPayment` directly with no confirmation popup. Tests will stall. Either remove the confirmation step for HD-wallet sends or rewrite to describe the actual flow (button transitions to 'Sending…' and the success banner reports the fee after broadcast).
- [BLOCKING] lines 77-78: Send-screen field labels and hint text don't match the default HD send UI
Session Setup §5 steps 4-5 (lines 77-78), TC-SND-005-01 steps 4-5 (line 307), and TC-SND-001-01 steps 5-6 (line 320) instruct testers to use a 'To:' field with hint *Enter Dash address* and an 'Amount:' field with hint *0.00*. Verified against `src/ui/wallets/send_screen.rs`: the default render produces 'Send to' (line 1366), address hint 'Enter address (X.../y.../evo1.../tevo1...)' (line 1397), 'Amount' (line 1418), and amount hint 'Enter amount' (line 1515). The 'To:'/'Amount:' labels only appear in the advanced multi-output path at line 2173, which testers do not open. A literal tester following the doc may believe they are on the wrong screen. Update the doc to match the default labels/hints.
- [BLOCKING] lines 307-309: TC-SND-005-01 expects a pre-send 'Fee:' estimate label that the HD send screen does not render
Step 6 says 'Verify the **"Fee:"** label shows a non-zero fee estimate,' and the Expected Result requires the fee estimate to be visible before confirming. Grep of `src/ui/wallets/send_screen.rs` finds no occurrences of `"Fee:"`; the only fee-related strings are 'Estimated fee:' embedded inside validation error messages and the post-send success banner (`\n\nFee: Estimated {} • Actual {}`). There is no live pre-send fee surface on the HD send screen, so step 6 cannot be completed and the case as written cannot pass. Combined with the missing confirmation dialog, the entire 'see fee before confirming' premise of TC-SND-005-01 needs reworking — either delete the case or rewrite it to verify fee on the post-send success banner.
- [SUGGESTION] line 117: TC-NET-001-01 step 2 mislocates the network chooser
Step 2 reads 'Locate the network chooser at the top of the settings screen.' Per `src/ui/network_chooser_screen.rs:197-294`, the dropdown is rendered inside the 'Connection Settings' card next to a 'Network:' label — not 'at the top' and not labeled 'network chooser.' Other test cases consistently name the precise label/section (e.g., 'Locate the "Theme:" selector'). Rewrite for consistency, e.g.: 'In the "Connection Settings" section, locate the "Network:" dropdown.'
| 8. A **"Fee Confirmation Required"** dialog appears — confirm the transaction. | ||
| 9. Wait for the transaction to appear in the **"Transactions"** section. | ||
| 10. Click **"Send"** again. | ||
| 11. In **"To:"**, enter `${TC_IDN_ADDRESS_0}`. | ||
| 12. In **"Amount:"**, enter `3`. | ||
| 13. Click **"Send"** in the form. | ||
| 14. Confirm in the **"Fee Confirmation Required"** dialog. |
There was a problem hiding this comment.
🔴 Blocking: HD-wallet Send flow has no 'Fee Confirmation Required' dialog — six test steps will hang
Six steps direct testers to confirm sends in a 'Fee Confirmation Required' dialog: Session Setup §5 step 8 (line 81) and step 14 (line 87), TC-SND-005-01 step 8 (line 307), TC-SND-001-01 step 9 (line 320), Session Cleanup §1 step 7 (line 575) and §2 step 6 (line 585). Grep confirms that window exists at exactly one location: src/ui/wallets/single_key_send_screen.rs:561 (egui::Window::new("Fee Confirmation Required")). Bank, Send Test, and Identity Test are mnemonic-imported HD wallets, so clicking 'Send' routes via src/ui/wallets/wallets_screen/mod.rs:872-876 to WalletSendScreen (send_screen.rs), which has zero Window::new calls and dispatches SendWalletPayment directly with no confirmation popup. Tests will stall. Either remove the confirmation step for HD-wallet sends or rewrite to describe the actual flow (button transitions to 'Sending…' and the success banner reports the fee after broadcast).
source: ['claude']
🤖 Fix this with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `docs/test-cases.md`:
- [BLOCKING] lines 81-87: HD-wallet Send flow has no 'Fee Confirmation Required' dialog — six test steps will hang
Six steps direct testers to confirm sends in a 'Fee Confirmation Required' dialog: Session Setup §5 step 8 (line 81) and step 14 (line 87), TC-SND-005-01 step 8 (line 307), TC-SND-001-01 step 9 (line 320), Session Cleanup §1 step 7 (line 575) and §2 step 6 (line 585). Grep confirms that window exists at exactly one location: `src/ui/wallets/single_key_send_screen.rs:561` (`egui::Window::new("Fee Confirmation Required")`). Bank, Send Test, and Identity Test are mnemonic-imported HD wallets, so clicking 'Send' routes via `src/ui/wallets/wallets_screen/mod.rs:872-876` to `WalletSendScreen` (`send_screen.rs`), which has zero `Window::new` calls and dispatches `SendWalletPayment` directly with no confirmation popup. Tests will stall. Either remove the confirmation step for HD-wallet sends or rewrite to describe the actual flow (button transitions to 'Sending…' and the success banner reports the fee after broadcast).
| 4. In the **"To:"** field (hint: *"Enter Dash address"*), enter `${TC_SND_ADDRESS_0}`. | ||
| 5. In the **"Amount:"** field (hint: *"0.00"*), enter `1`. |
There was a problem hiding this comment.
🔴 Blocking: Send-screen field labels and hint text don't match the default HD send UI
Session Setup §5 steps 4-5 (lines 77-78), TC-SND-005-01 steps 4-5 (line 307), and TC-SND-001-01 steps 5-6 (line 320) instruct testers to use a 'To:' field with hint Enter Dash address and an 'Amount:' field with hint 0.00. Verified against src/ui/wallets/send_screen.rs: the default render produces 'Send to' (line 1366), address hint 'Enter address (X.../y.../evo1.../tevo1...)' (line 1397), 'Amount' (line 1418), and amount hint 'Enter amount' (line 1515). The 'To:'/'Amount:' labels only appear in the advanced multi-output path at line 2173, which testers do not open. A literal tester following the doc may believe they are on the wrong screen. Update the doc to match the default labels/hints.
source: ['claude']
🤖 Fix this with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `docs/test-cases.md`:
- [BLOCKING] lines 77-78: Send-screen field labels and hint text don't match the default HD send UI
Session Setup §5 steps 4-5 (lines 77-78), TC-SND-005-01 steps 4-5 (line 307), and TC-SND-001-01 steps 5-6 (line 320) instruct testers to use a 'To:' field with hint *Enter Dash address* and an 'Amount:' field with hint *0.00*. Verified against `src/ui/wallets/send_screen.rs`: the default render produces 'Send to' (line 1366), address hint 'Enter address (X.../y.../evo1.../tevo1...)' (line 1397), 'Amount' (line 1418), and amount hint 'Enter amount' (line 1515). The 'To:'/'Amount:' labels only appear in the advanced multi-output path at line 2173, which testers do not open. A literal tester following the doc may believe they are on the wrong screen. Update the doc to match the default labels/hints.
| | **Test Steps** | 1. Click **"Wallets"** in the left sidebar.<br>2. Select the **Send Test** wallet.<br>3. Click **"Send"**.<br>4. In the **"To:"** field (hint: *"Enter Dash address"*), enter `${BANK_ADDRESS_0}`.<br>5. In the **"Amount:"** field (hint: *"0.00"*), enter `0.01`.<br>6. Verify the **"Fee:"** label shows a non-zero fee estimate.<br>7. Click **"Send"** in the form.<br>8. A **"Fee Confirmation Required"** dialog appears.<br>9. Verify the dialog shows the amount, fee, and total deduction.<br>10. Cancel or close the dialog — do NOT confirm the send. | | ||
| | **Test Data** | Amount: `0.01` tDASH, Destination: `${BANK_ADDRESS_0}` | | ||
| | **Expected Result** | **"Fee:"** estimate and total deduction are clearly visible before confirming. | |
There was a problem hiding this comment.
🔴 Blocking: TC-SND-005-01 expects a pre-send 'Fee:' estimate label that the HD send screen does not render
Step 6 says 'Verify the "Fee:" label shows a non-zero fee estimate,' and the Expected Result requires the fee estimate to be visible before confirming. Grep of src/ui/wallets/send_screen.rs finds no occurrences of "Fee:"; the only fee-related strings are 'Estimated fee:' embedded inside validation error messages and the post-send success banner (\n\nFee: Estimated {} • Actual {}). There is no live pre-send fee surface on the HD send screen, so step 6 cannot be completed and the case as written cannot pass. Combined with the missing confirmation dialog, the entire 'see fee before confirming' premise of TC-SND-005-01 needs reworking — either delete the case or rewrite it to verify fee on the post-send success banner.
source: ['claude']
🤖 Fix this with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `docs/test-cases.md`:
- [BLOCKING] lines 307-309: TC-SND-005-01 expects a pre-send 'Fee:' estimate label that the HD send screen does not render
Step 6 says 'Verify the **"Fee:"** label shows a non-zero fee estimate,' and the Expected Result requires the fee estimate to be visible before confirming. Grep of `src/ui/wallets/send_screen.rs` finds no occurrences of `"Fee:"`; the only fee-related strings are 'Estimated fee:' embedded inside validation error messages and the post-send success banner (`\n\nFee: Estimated {} • Actual {}`). There is no live pre-send fee surface on the HD send screen, so step 6 cannot be completed and the case as written cannot pass. Combined with the missing confirmation dialog, the entire 'see fee before confirming' premise of TC-SND-005-01 needs reworking — either delete the case or rewrite it to verify fee on the post-send success banner.
| | **Test Case ID** | TC-NET-001-01 | | ||
| | **Short Description** | Verify DET is on testnet and network options are visible | | ||
| | **Pre-Conditions** | DET running, SPV synced | | ||
| | **Test Steps** | 1. Click **"Settings"** in the left sidebar.<br>2. Locate the network chooser at the top of the settings screen.<br>3. Verify **"Testnet"** is the currently selected network.<br>4. Verify the other chooser entry listed by the documented config is **"Mainnet"**.<br>5. Do NOT switch networks. | |
There was a problem hiding this comment.
🟡 Suggestion: TC-NET-001-01 step 2 mislocates the network chooser
Step 2 reads 'Locate the network chooser at the top of the settings screen.' Per src/ui/network_chooser_screen.rs:197-294, the dropdown is rendered inside the 'Connection Settings' card next to a 'Network:' label — not 'at the top' and not labeled 'network chooser.' Other test cases consistently name the precise label/section (e.g., 'Locate the "Theme:" selector'). Rewrite for consistency, e.g.: 'In the "Connection Settings" section, locate the "Network:" dropdown.'
source: ['claude']
🤖 Fix this with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `docs/test-cases.md`:
- [SUGGESTION] line 117: TC-NET-001-01 step 2 mislocates the network chooser
Step 2 reads 'Locate the network chooser at the top of the settings screen.' Per `src/ui/network_chooser_screen.rs:197-294`, the dropdown is rendered inside the 'Connection Settings' card next to a 'Network:' label — not 'at the top' and not labeled 'network chooser.' Other test cases consistently name the precise label/section (e.g., 'Locate the "Theme:" selector'). Rewrite for consistency, e.g.: 'In the "Connection Settings" section, locate the "Network:" dropdown.'
678fdc5 to
aed3de7
Compare
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
Documentation-only PR. Both reviewers correctly identified that earlier blocking issues (Fee Confirmation dialog, field labels, TC-SND-005-01, TC-NET-001-01) have been resolved on aed3de7. Two new/remaining blocking issues are valid against the worktree: (1) Session Setup §5 and three other Send steps in test-cases.md assert the success banner shows a fee, but the HD wallet success path (WalletPayment) only renders 'Sent {amount} to {address}' — the fee line only appears for credit transfers; (2) the Bank Fund Consolidation section in test-prerequisites.md still uses the single-key Send UI ("To:" field, "Fee Confirmation Required" dialog) even though the Bank wallet is imported from a mnemonic and therefore routes through WalletSendScreen. One non-blocking precision issue and one nitpick remain.
Reviewed commit: aed3de7
🔴 2 blocking | 🟡 1 suggestion(s)
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `docs/test-cases.md`:
- [BLOCKING] lines 80-85: Send success banner does not include a fee for HD wallet sends
Session Setup §5 steps 7 and 12 (lines 80, 85), TC-SND-005-01 step 9 and Expected Result (~line 306-308), and Session Cleanup §1 step 7 / §2 step 6 (~lines 573, 583) all assert the success banner 'reports the broadcast transaction, including the fee paid.' That assertion does not match the code. `WalletSendScreen::display_task_result` handles the only success variant produced by HD wallet Send — `BackendTaskSuccessResult::WalletPayment` — at src/ui/wallets/send_screen.rs:2694-2710 by formatting `SendStatus::Complete("Sent {amount} to {address}")` with no fee. The `\n\nFee: Estimated {} • Actual {}` line only appears in the `TransferredCredits` arm at src/ui/wallets/send_screen.rs:2712-2719, which is a platform credit transfer, not an HD-wallet Send. Testers following the doc will see a success heading like 'Sent 1.0000 DASH to y…' with no fee surfaced anywhere on the screen, then either fail the verification or fabricate a result. Rewrite the affected steps so the banner is verified to show the sent amount and destination address; if fee reporting must remain in the smoke test, point at the post-send Transactions row, which exposes the fee, rather than the success banner.
- [SUGGESTION] lines 79-84: 'Verify the button changes to Sending…' is not what testers actually see on submit
Session Setup §5 steps 6 and 11 (lines 79, 84), TC-SND-005-01 step 7, TC-SND-001-01 step 8, and Session Cleanup §1 step 6 / §2 step 5 tell the tester to 'verify the button changes to Sending…' after clicking Send. `WalletSendScreen::show` short-circuits the entire form when `send_status` is not Idle (`if let Some(status_action) = self.render_send_status(...) { return status_action; }` at src/ui/wallets/send_screen.rs:2630-2632), and `SendStatus::WaitingForResult` is set in the same handler as the click (src/ui/wallets/send_screen.rs:493). The next frame renders a vertical_centered status view with `egui::Spinner::new()` and `ui.heading("Sending...")` (src/ui/wallets/send_screen.rs:1124-1141). The `button_text = if is_sending { "Sending..." } else { "Send" }` branch at ~line 1700 is effectively dead because the form is replaced before the next paint. Rewrite as 'Verify the form is replaced by a spinner and the heading "Sending…"' to match what testers actually see; the same fix applies to the Bank consolidation rewrite.
In `docs/test-prerequisites.md`:
- [BLOCKING] lines 155-163: Bank Fund Consolidation still documents the single-key send UI, not the HD wallet send flow QA actually gets
The Bank wallet is imported from a mnemonic (docs/test-prerequisites.md:141-149), so clicking **Send** routes through `WalletSendScreen`, not `SingleKeyWalletSendScreen` (src/ui/wallets/wallets_screen/mod.rs:872-880 — mnemonic-imported wallets always enter the `selected_wallet` branch). `WalletSendScreen` labels its fields **"Send to"** and **"Amount"** (src/ui/wallets/send_screen.rs:1366-1398, 1418-1516) and broadcasts directly without any **"Fee Confirmation Required"** dialog; that dialog exists only in `SingleKeyWalletSendScreen` (src/ui/wallets/single_key_send_screen.rs:561). A tester following the current consolidation steps will look for a non-existent **"To:"** field and a non-existent fee-confirmation dialog, so this prerequisite — which is a precondition for the entire suite — cannot be completed as written. Rewrite to: select Bank → Receive (note `${BANK_ADDRESS_0}`) → Send → paste address into **"Send to"** → click **"Max"** → click **"Send"** → wait for the success banner showing the sent amount and destination → click **"Back to Wallet"** → wait for one confirmation. Do not assert the fee in the success banner (see related finding) and do not assert the in-form button text changes to 'Sending…' (the form is replaced with a spinner — see related finding).
| 7. Wait for the success banner and verify it reports the broadcast transaction, including the fee paid. | ||
| 8. Click **"Send Another"**. | ||
| 9. In the **"Send to"** field, enter `${TC_IDN_ADDRESS_0}`. | ||
| 10. In the **"Amount"** field, enter `3`. | ||
| 11. Click **"Send"** in the form and verify the button changes to **"Sending..."** while the transaction is broadcast. | ||
| 12. Wait for the success banner and verify it reports the broadcast transaction, including the fee paid. |
There was a problem hiding this comment.
🔴 Blocking: Send success banner does not include a fee for HD wallet sends
Session Setup §5 steps 7 and 12 (lines 80, 85), TC-SND-005-01 step 9 and Expected Result (~line 306-308), and Session Cleanup §1 step 7 / §2 step 6 (~lines 573, 583) all assert the success banner 'reports the broadcast transaction, including the fee paid.' That assertion does not match the code. WalletSendScreen::display_task_result handles the only success variant produced by HD wallet Send — BackendTaskSuccessResult::WalletPayment — at src/ui/wallets/send_screen.rs:2694-2710 by formatting SendStatus::Complete("Sent {amount} to {address}") with no fee. The \n\nFee: Estimated {} • Actual {} line only appears in the TransferredCredits arm at src/ui/wallets/send_screen.rs:2712-2719, which is a platform credit transfer, not an HD-wallet Send. Testers following the doc will see a success heading like 'Sent 1.0000 DASH to y…' with no fee surfaced anywhere on the screen, then either fail the verification or fabricate a result. Rewrite the affected steps so the banner is verified to show the sent amount and destination address; if fee reporting must remain in the smoke test, point at the post-send Transactions row, which exposes the fee, rather than the success banner.
source: ['claude']
🤖 Fix this with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `docs/test-cases.md`:
- [BLOCKING] lines 80-85: Send success banner does not include a fee for HD wallet sends
Session Setup §5 steps 7 and 12 (lines 80, 85), TC-SND-005-01 step 9 and Expected Result (~line 306-308), and Session Cleanup §1 step 7 / §2 step 6 (~lines 573, 583) all assert the success banner 'reports the broadcast transaction, including the fee paid.' That assertion does not match the code. `WalletSendScreen::display_task_result` handles the only success variant produced by HD wallet Send — `BackendTaskSuccessResult::WalletPayment` — at src/ui/wallets/send_screen.rs:2694-2710 by formatting `SendStatus::Complete("Sent {amount} to {address}")` with no fee. The `\n\nFee: Estimated {} • Actual {}` line only appears in the `TransferredCredits` arm at src/ui/wallets/send_screen.rs:2712-2719, which is a platform credit transfer, not an HD-wallet Send. Testers following the doc will see a success heading like 'Sent 1.0000 DASH to y…' with no fee surfaced anywhere on the screen, then either fail the verification or fabricate a result. Rewrite the affected steps so the banner is verified to show the sent amount and destination address; if fee reporting must remain in the smoke test, point at the post-send Transactions row, which exposes the fee, rather than the success banner.
| 1. Select the **Bank** wallet on the **"Wallets"** screen. | ||
| 2. Click **"Receive"** and note the address shown (this is `${BANK_ADDRESS_0}`). | ||
| 3. Click **"Send"**. | ||
| 4. In the **"To:"** field, paste `${BANK_ADDRESS_0}`. | ||
| 5. Click **"Max"** to set the maximum sendable amount. | ||
| 6. Click **"Send"** in the form. | ||
| 7. A **"Fee Confirmation Required"** dialog appears — review the fee and total. | ||
| 8. Confirm the transaction. | ||
| 9. Wait for at least one confirmation. |
There was a problem hiding this comment.
🔴 Blocking: Bank Fund Consolidation still documents the single-key send UI, not the HD wallet send flow QA actually gets
The Bank wallet is imported from a mnemonic (docs/test-prerequisites.md:141-149), so clicking Send routes through WalletSendScreen, not SingleKeyWalletSendScreen (src/ui/wallets/wallets_screen/mod.rs:872-880 — mnemonic-imported wallets always enter the selected_wallet branch). WalletSendScreen labels its fields "Send to" and "Amount" (src/ui/wallets/send_screen.rs:1366-1398, 1418-1516) and broadcasts directly without any "Fee Confirmation Required" dialog; that dialog exists only in SingleKeyWalletSendScreen (src/ui/wallets/single_key_send_screen.rs:561). A tester following the current consolidation steps will look for a non-existent "To:" field and a non-existent fee-confirmation dialog, so this prerequisite — which is a precondition for the entire suite — cannot be completed as written. Rewrite to: select Bank → Receive (note ${BANK_ADDRESS_0}) → Send → paste address into "Send to" → click "Max" → click "Send" → wait for the success banner showing the sent amount and destination → click "Back to Wallet" → wait for one confirmation. Do not assert the fee in the success banner (see related finding) and do not assert the in-form button text changes to 'Sending…' (the form is replaced with a spinner — see related finding).
source: ['codex']
🤖 Fix this with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `docs/test-prerequisites.md`:
- [BLOCKING] lines 155-163: Bank Fund Consolidation still documents the single-key send UI, not the HD wallet send flow QA actually gets
The Bank wallet is imported from a mnemonic (docs/test-prerequisites.md:141-149), so clicking **Send** routes through `WalletSendScreen`, not `SingleKeyWalletSendScreen` (src/ui/wallets/wallets_screen/mod.rs:872-880 — mnemonic-imported wallets always enter the `selected_wallet` branch). `WalletSendScreen` labels its fields **"Send to"** and **"Amount"** (src/ui/wallets/send_screen.rs:1366-1398, 1418-1516) and broadcasts directly without any **"Fee Confirmation Required"** dialog; that dialog exists only in `SingleKeyWalletSendScreen` (src/ui/wallets/single_key_send_screen.rs:561). A tester following the current consolidation steps will look for a non-existent **"To:"** field and a non-existent fee-confirmation dialog, so this prerequisite — which is a precondition for the entire suite — cannot be completed as written. Rewrite to: select Bank → Receive (note `${BANK_ADDRESS_0}`) → Send → paste address into **"Send to"** → click **"Max"** → click **"Send"** → wait for the success banner showing the sent amount and destination → click **"Back to Wallet"** → wait for one confirmation. Do not assert the fee in the success banner (see related finding) and do not assert the in-form button text changes to 'Sending…' (the form is replaced with a spinner — see related finding).
| 6. Click **"Send"** in the form and verify the button changes to **"Sending..."** while the transaction is broadcast. | ||
| 7. Wait for the success banner and verify it reports the broadcast transaction, including the fee paid. | ||
| 8. Click **"Send Another"**. | ||
| 9. In the **"Send to"** field, enter `${TC_IDN_ADDRESS_0}`. | ||
| 10. In the **"Amount"** field, enter `3`. | ||
| 11. Click **"Send"** in the form and verify the button changes to **"Sending..."** while the transaction is broadcast. |
There was a problem hiding this comment.
🟡 Suggestion: 'Verify the button changes to Sending…' is not what testers actually see on submit
Session Setup §5 steps 6 and 11 (lines 79, 84), TC-SND-005-01 step 7, TC-SND-001-01 step 8, and Session Cleanup §1 step 6 / §2 step 5 tell the tester to 'verify the button changes to Sending…' after clicking Send. WalletSendScreen::show short-circuits the entire form when send_status is not Idle (if let Some(status_action) = self.render_send_status(...) { return status_action; } at src/ui/wallets/send_screen.rs:2630-2632), and SendStatus::WaitingForResult is set in the same handler as the click (src/ui/wallets/send_screen.rs:493). The next frame renders a vertical_centered status view with egui::Spinner::new() and ui.heading("Sending...") (src/ui/wallets/send_screen.rs:1124-1141). The button_text = if is_sending { "Sending..." } else { "Send" } branch at ~line 1700 is effectively dead because the form is replaced before the next paint. Rewrite as 'Verify the form is replaced by a spinner and the heading "Sending…"' to match what testers actually see; the same fix applies to the Bank consolidation rewrite.
source: ['claude']
🤖 Fix this with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `docs/test-cases.md`:
- [SUGGESTION] lines 79-84: 'Verify the button changes to Sending…' is not what testers actually see on submit
Session Setup §5 steps 6 and 11 (lines 79, 84), TC-SND-005-01 step 7, TC-SND-001-01 step 8, and Session Cleanup §1 step 6 / §2 step 5 tell the tester to 'verify the button changes to Sending…' after clicking Send. `WalletSendScreen::show` short-circuits the entire form when `send_status` is not Idle (`if let Some(status_action) = self.render_send_status(...) { return status_action; }` at src/ui/wallets/send_screen.rs:2630-2632), and `SendStatus::WaitingForResult` is set in the same handler as the click (src/ui/wallets/send_screen.rs:493). The next frame renders a vertical_centered status view with `egui::Spinner::new()` and `ui.heading("Sending...")` (src/ui/wallets/send_screen.rs:1124-1141). The `button_text = if is_sending { "Sending..." } else { "Send" }` branch at ~line 1700 is effectively dead because the form is replaced before the next paint. Rewrite as 'Verify the form is replaced by a spinner and the heading "Sending…"' to match what testers actually see; the same fix applies to the Bank consolidation rewrite.
aed3de7 to
b6ff270
Compare
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
Documentation-only PR. Three blocking issues remain on this SHA: (1) every mnemonic-import commit step in test-cases.md and test-prerequisites.md tells the tester to click 'Import Wallet' to save, but the actual button is 'Save Wallet' ('Import Wallet' is only a breadcrumb on the import screen); (2) TC-SND-005-01 still asks QA to verify a fee on the Transactions row, but that table has only Date/Type/Amount/Status/TxID columns; (3) TC-SND-001-01 instructs the tester to click 'Back to Wallet' immediately after the form is replaced by the spinner, but that button only renders in SendStatus::Complete. One nitpick about hardcoded testnet DAPI IPs carries forward.
Reviewed commit: b6ff270
🔴 3 blocking
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `docs/test-cases.md`:
- [BLOCKING] line 51: Mnemonic import commit step says 'Import Wallet'; the actual button is 'Save Wallet'
Five steps direct the tester to commit a mnemonic import by clicking 'Import Wallet': test-cases.md Session Setup §2 step 8 (line 51), §3 step 6 (line 61), §4 step 6 (line 70), TC-WAL-002-01 step 9 (line 185), and test-prerequisites.md Bank import step 8 (line 148). In src/ui/wallets/import_mnemonic_screen.rs:729-732 the commit button is hard-coded to 'Save Wallet' for ImportType::Mnemonic (the heading just above at line 718 reads '{n}. Save the wallet.'). 'Import Wallet' appears on this screen only as the top-panel breadcrumb at line 507 — not a clickable submit. A literal tester who finishes entering the mnemonic, name, and password will look for an 'Import Wallet' button on the form and not find one; the breadcrumb will look like a label. The earlier 'Click Import Wallet (top-right)' steps that navigate into the import screen are correct and should not be changed. TC-WAL-001-01 step 10 already uses 'Save Wallet' correctly (line 172); the import flow needs the same fix at the five commit-step locations.
- [BLOCKING] lines 304-308: TC-SND-005-01 verifies a fee on the Transactions row, but the UI has no fee column
The rewrite moved the fee assertion from the success banner (correctly, since it isn't there) to the wallet's Transactions section, but that surface does not expose a fee either. render_transactions_section() in src/ui/wallets/wallets_screen/mod.rs:1004-1083 builds a table with exactly five columns — Date, Type, Amount, Status, TxID — and the per-row rendering (lines 1052-1081) emits only those five values; there is no fee column, tooltip, or expanded row. Step 11 ('verify the fee is reported on that row') and the Expected Result cannot be satisfied by the current UI, so TC-SND-005-01 will still fail. Either drop the fee verification or point QA to a fee surface that actually exists (for example, deriving fee from balance delta or from a different developer view if one is intended).
- [BLOCKING] line 319: TC-SND-001-01 step 9 clicks 'Back to Wallet' from the spinner state where that button does not exist
Step 8 correctly says the send form is replaced by the spinner and 'Sending...' heading, but step 9 immediately instructs the tester to click 'Back to Wallet'. In src/ui/wallets/send_screen.rs:1102-1140, 'Back to Wallet' is rendered only inside the SendStatus::Complete arm (lines 1104-1118); the SendStatus::WaitingForResult arm shown after submission renders only the spinner, the 'Sending...' heading, and an elapsed-time label (lines 1124-1140). A literal tester cannot perform step 9 from the state produced by step 8. Insert an explicit wait-for-success-banner step between steps 8 and 9 (mirroring the Session Setup and TC-SND-005-01 sequences, which correctly wait for the banner before clicking 'Back to Wallet').
| 5. Enter each word into the numbered fields (**"1:"**, **"2:"**, ...). | ||
| 6. In **"Name:"**, type `Bank`. | ||
| 7. In **"Optional Password:"**, enter `${BANK_PASSWORD}`. | ||
| 8. Click **"Import Wallet"**. |
There was a problem hiding this comment.
🔴 Blocking: Mnemonic import commit step says 'Import Wallet'; the actual button is 'Save Wallet'
Five steps direct the tester to commit a mnemonic import by clicking 'Import Wallet': test-cases.md Session Setup §2 step 8 (line 51), §3 step 6 (line 61), §4 step 6 (line 70), TC-WAL-002-01 step 9 (line 185), and test-prerequisites.md Bank import step 8 (line 148). In src/ui/wallets/import_mnemonic_screen.rs:729-732 the commit button is hard-coded to 'Save Wallet' for ImportType::Mnemonic (the heading just above at line 718 reads '{n}. Save the wallet.'). 'Import Wallet' appears on this screen only as the top-panel breadcrumb at line 507 — not a clickable submit. A literal tester who finishes entering the mnemonic, name, and password will look for an 'Import Wallet' button on the form and not find one; the breadcrumb will look like a label. The earlier 'Click Import Wallet (top-right)' steps that navigate into the import screen are correct and should not be changed. TC-WAL-001-01 step 10 already uses 'Save Wallet' correctly (line 172); the import flow needs the same fix at the five commit-step locations.
source: ['claude']
🤖 Fix this with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `docs/test-cases.md`:
- [BLOCKING] line 51: Mnemonic import commit step says 'Import Wallet'; the actual button is 'Save Wallet'
Five steps direct the tester to commit a mnemonic import by clicking 'Import Wallet': test-cases.md Session Setup §2 step 8 (line 51), §3 step 6 (line 61), §4 step 6 (line 70), TC-WAL-002-01 step 9 (line 185), and test-prerequisites.md Bank import step 8 (line 148). In src/ui/wallets/import_mnemonic_screen.rs:729-732 the commit button is hard-coded to 'Save Wallet' for ImportType::Mnemonic (the heading just above at line 718 reads '{n}. Save the wallet.'). 'Import Wallet' appears on this screen only as the top-panel breadcrumb at line 507 — not a clickable submit. A literal tester who finishes entering the mnemonic, name, and password will look for an 'Import Wallet' button on the form and not find one; the breadcrumb will look like a label. The earlier 'Click Import Wallet (top-right)' steps that navigate into the import screen are correct and should not be changed. TC-WAL-001-01 step 10 already uses 'Save Wallet' correctly (line 172); the import flow needs the same fix at the five commit-step locations.
| | **Short Description** | Verify the send success banner reports the sent amount and destination, and the fee is visible from the post-send Transactions row | | ||
| | **Pre-Conditions** | **Send Test** wallet selected with >= 1 tDASH | | ||
| | **Test Steps** | 1. Click **"Wallets"** in the left sidebar.<br>2. Select the **Send Test** wallet.<br>3. Click **"Send"**.<br>4. In the **"Send to"** field (hint: *"Enter address (X.../y.../evo1.../tevo1...)"*), enter `${BANK_ADDRESS_0}`.<br>5. In the **"Amount"** field (hint: *"Enter amount"*), enter `0.01`.<br>6. Click **"Send"** in the form.<br>7. Verify the form is replaced by a spinner and a **"Sending..."** heading while the transaction is broadcast.<br>8. Wait for the success banner.<br>9. Verify the success banner shows **"Sent 0.01 DASH to ${BANK_ADDRESS_0}"** (sent amount and destination address).<br>10. Click **"Back to Wallet"**.<br>11. Locate the just-sent transaction in the **"Transactions"** section and verify the fee is reported on that row.<br>12. Select the **Bank** wallet and verify the **"Transactions"** section shows the incoming 0.01 tDASH transaction. | | ||
| | **Test Data** | Amount: `0.01` tDASH, Destination: `${BANK_ADDRESS_0}` | | ||
| | **Expected Result** | The success banner shows the sent amount and destination address. The fee paid is visible on the post-send row in the sender's **"Transactions"** section. The receiving wallet shows the incoming 0.01 tDASH transaction. | |
There was a problem hiding this comment.
🔴 Blocking: TC-SND-005-01 verifies a fee on the Transactions row, but the UI has no fee column
The rewrite moved the fee assertion from the success banner (correctly, since it isn't there) to the wallet's Transactions section, but that surface does not expose a fee either. render_transactions_section() in src/ui/wallets/wallets_screen/mod.rs:1004-1083 builds a table with exactly five columns — Date, Type, Amount, Status, TxID — and the per-row rendering (lines 1052-1081) emits only those five values; there is no fee column, tooltip, or expanded row. Step 11 ('verify the fee is reported on that row') and the Expected Result cannot be satisfied by the current UI, so TC-SND-005-01 will still fail. Either drop the fee verification or point QA to a fee surface that actually exists (for example, deriving fee from balance delta or from a different developer view if one is intended).
source: ['codex']
🤖 Fix this with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `docs/test-cases.md`:
- [BLOCKING] lines 304-308: TC-SND-005-01 verifies a fee on the Transactions row, but the UI has no fee column
The rewrite moved the fee assertion from the success banner (correctly, since it isn't there) to the wallet's Transactions section, but that surface does not expose a fee either. render_transactions_section() in src/ui/wallets/wallets_screen/mod.rs:1004-1083 builds a table with exactly five columns — Date, Type, Amount, Status, TxID — and the per-row rendering (lines 1052-1081) emits only those five values; there is no fee column, tooltip, or expanded row. Step 11 ('verify the fee is reported on that row') and the Expected Result cannot be satisfied by the current UI, so TC-SND-005-01 will still fail. Either drop the fee verification or point QA to a fee surface that actually exists (for example, deriving fee from balance delta or from a different developer view if one is intended).
| | **Test Case ID** | TC-SND-001-01 | | ||
| | **Short Description** | Send tDASH from the Send Test wallet to the bank wallet | | ||
| | **Pre-Conditions** | **Send Test** wallet selected with >= 0.5 tDASH | | ||
| | **Test Steps** | 1. Click **"Wallets"** in the left sidebar.<br>2. Select the **Send Test** wallet.<br>3. Note the current **"Core balance:"** value.<br>4. Click **"Send"**.<br>5. In the **"Send to"** field (hint: *"Enter address (X.../y.../evo1.../tevo1...)"*), enter `${BANK_ADDRESS_0}`.<br>6. In the **"Amount"** field (hint: *"Enter amount"*), enter `0.1`.<br>7. Click **"Send"** in the form.<br>8. Verify the form is replaced by a spinner and a **"Sending..."** heading while the transaction is broadcast.<br>9. Click **"Back to Wallet"**.<br>10. Wait for the transaction to appear in the **"Transactions"** section.<br>11. Verify **"Core balance:"** decreased by approximately 0.1 tDASH plus the fee.<br>12. Select the **Bank** wallet.<br>13. Verify the **"Transactions"** section shows the incoming 0.1 tDASH transaction. | |
There was a problem hiding this comment.
🔴 Blocking: TC-SND-001-01 step 9 clicks 'Back to Wallet' from the spinner state where that button does not exist
Step 8 correctly says the send form is replaced by the spinner and 'Sending...' heading, but step 9 immediately instructs the tester to click 'Back to Wallet'. In src/ui/wallets/send_screen.rs:1102-1140, 'Back to Wallet' is rendered only inside the SendStatus::Complete arm (lines 1104-1118); the SendStatus::WaitingForResult arm shown after submission renders only the spinner, the 'Sending...' heading, and an elapsed-time label (lines 1124-1140). A literal tester cannot perform step 9 from the state produced by step 8. Insert an explicit wait-for-success-banner step between steps 8 and 9 (mirroring the Session Setup and TC-SND-005-01 sequences, which correctly wait for the banner before clicking 'Back to Wallet').
source: ['codex']
🤖 Fix this with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `docs/test-cases.md`:
- [BLOCKING] line 319: TC-SND-001-01 step 9 clicks 'Back to Wallet' from the spinner state where that button does not exist
Step 8 correctly says the send form is replaced by the spinner and 'Sending...' heading, but step 9 immediately instructs the tester to click 'Back to Wallet'. In src/ui/wallets/send_screen.rs:1102-1140, 'Back to Wallet' is rendered only inside the SendStatus::Complete arm (lines 1104-1118); the SendStatus::WaitingForResult arm shown after submission renders only the spinner, the 'Sending...' heading, and an elapsed-time label (lines 1124-1140). A literal tester cannot perform step 9 from the state produced by step 8. Insert an explicit wait-for-success-banner step between steps 8 and 9 (mirroring the Session Setup and TC-SND-005-01 sequences, which correctly wait for the banner before clicking 'Back to Wallet').
b6ff270 to
54b53b0
Compare
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
Documentation-only PR for the weekly smoke-test suite. All three prior blocking findings (mnemonic 'Save Wallet' button, send-history fee column, basic-send success banner wait) are resolved on 54b53b0. Remaining issues are two consistency suggestions where navigation steps omit exact button names ('📛 Register DPNS Name', '💰 Top up'), a label mismatch where TC-DPY-001-01/002-01 assert a 'Bio/Status:' label that only exists in edit mode (display mode shows 'Bio:'), and a maintainability nitpick about the inlined testnet DAPI/insight URL block which has already drifted from the bundled .env.example.
Reviewed commit: 54b53b0
🟡 3 suggestion(s) | 💬 1 nitpick(s)
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `docs/test-cases.md`:
- [SUGGESTION] lines 452-466: TC-DPY-001-01 step 10 and TC-DPY-002-01 step 6 assert a 'Bio/Status:' label that only exists in edit mode
Both steps tell QA to verify the profile shows 'Bio/Status: Automated QA testing'. In src/ui/dashpay/profile_screen.rs, 'Bio/Status:' is only the edit-form label (line 786); the display/view side renders the same field under 'Bio:' (line 1230). After Save Profile (step 8), the screen switches back to display mode and a literal tester will see 'Bio:', not 'Bio/Status:', and may mark the case as failed. Update the verification text in both cases to use 'Bio:' (or describe both states explicitly).
- [SUGGESTION] line 409: TC-DPN-001-01 step 3 doesn't name the button used to reach the DPNS registration screen
Step 3 reads 'Navigate to DPNS registration — the heading reads "Register DPNS Name"', which is a verification rather than an action. The actual navigation is clicking the per-identity '📛 Register DPNS Name' button (src/ui/identities/identities_screen.rs:684). Every other navigation step in this suite names the literal button label (e.g., TC-IDN-008-01 step 3 says 'Click the "Keys" button'). Rewrite as: 'Click the "📛 Register DPNS Name" button for that identity and verify the heading reads "Register DPNS Name".'
- [SUGGESTION] line 366: TC-IDN-004-01 step 3 'Open top-up' should name the actual '💰 Top up' button
Step 3 says 'Open top-up and verify the breadcrumb shows "Top Up Identity".' The per-identity button on the Identities screen is labelled '💰 Top up' (src/ui/identities/identities_screen.rs:648). For consistency with the rest of the suite, rewrite step 3 as: 'Click the "💰 Top up" button for that identity and verify the breadcrumb shows "Top Up Identity".'
| | **Test Steps** | 1. Click **"Dashpay"** in the left sidebar.<br>2. Click **"My Profile"** in the DashPay subscreen chooser.<br>3. Click **"Refresh"** in the top panel and wait for the profile view to load.<br>4. If the **"No DashPay Profile"** card appears, click **"Create Profile"**.<br>5. In the **"Display Name:"** field (hint: *"Enter your display name (required)"*), type `Smoke Test Bot`.<br>6. In the **"Bio/Status:"** field, type `Automated QA testing`.<br>7. Leave the **"Avatar URL:"** field empty.<br>8. Click **"Save Profile"**.<br>9. Wait for Platform confirmation.<br>10. Verify the profile screen shows **Display Name:** `Smoke Test Bot` and **Bio/Status:** `Automated QA testing`. | | ||
| | **Test Data** | Display Name: `Smoke Test Bot`, Bio: `Automated QA testing` | | ||
| | **Expected Result** | Profile created. Display name and bio visible on the profile screen. | | ||
| | **Post-Condition** | Profile exists for search test. | | ||
|
|
||
| ### TC-DPY-002-01: Search DashPay profiles | ||
|
|
||
| | Field | Value | | ||
| |-------|-------| | ||
| | **Use Case ID** | DPY-002 | | ||
| | **Test Case ID** | TC-DPY-002-01 | | ||
| | **Short Description** | Search for a known DashPay profile by username | | ||
| | **Pre-Conditions** | DashPay profile created in TC-DPY-001-01 | | ||
| | **Test Steps** | 1. Click **"Dashpay"** in the left sidebar.<br>2. Click **"Search Profiles"** in the DashPay subscreen chooser.<br>3. Locate the search field (hint: *"Enter DPNS username..."*).<br>4. Type the username registered in TC-DPN-001-01.<br>5. Verify search results appear.<br>6. Verify the result shows **Display Name:** `Smoke Test Bot` and **Bio/Status:** `Automated QA testing`. | | ||
| | **Test Data** | Search query: username from TC-DPN-001-01 | |
There was a problem hiding this comment.
🟡 Suggestion: TC-DPY-001-01 step 10 and TC-DPY-002-01 step 6 assert a 'Bio/Status:' label that only exists in edit mode
Both steps tell QA to verify the profile shows 'Bio/Status: Automated QA testing'. In src/ui/dashpay/profile_screen.rs, 'Bio/Status:' is only the edit-form label (line 786); the display/view side renders the same field under 'Bio:' (line 1230). After Save Profile (step 8), the screen switches back to display mode and a literal tester will see 'Bio:', not 'Bio/Status:', and may mark the case as failed. Update the verification text in both cases to use 'Bio:' (or describe both states explicitly).
source: ['claude']
🤖 Fix this with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `docs/test-cases.md`:
- [SUGGESTION] lines 452-466: TC-DPY-001-01 step 10 and TC-DPY-002-01 step 6 assert a 'Bio/Status:' label that only exists in edit mode
Both steps tell QA to verify the profile shows 'Bio/Status: Automated QA testing'. In src/ui/dashpay/profile_screen.rs, 'Bio/Status:' is only the edit-form label (line 786); the display/view side renders the same field under 'Bio:' (line 1230). After Save Profile (step 8), the screen switches back to display mode and a literal tester will see 'Bio:', not 'Bio/Status:', and may mark the case as failed. Update the verification text in both cases to use 'Bio:' (or describe both states explicitly).
| | **Test Case ID** | TC-DPN-001-01 | | ||
| | **Short Description** | Register a unique DPNS username for the test identity | | ||
| | **Pre-Conditions** | Identity from TC-IDN-001-01 with sufficient credits | | ||
| | **Test Steps** | 1. Click **"Identities"** in the left sidebar.<br>2. Select the identity from TC-IDN-001-01.<br>3. Navigate to DPNS registration — the heading reads **"Register DPNS Name"**.<br>4. Enter a unique username: `detsmoke-{timestamp}` (e.g., `detsmoke-1710770000`).<br>5. Review the cost estimate.<br>6. Click **"Register Name"**.<br>7. Wait for Platform confirmation. | |
There was a problem hiding this comment.
🟡 Suggestion: TC-DPN-001-01 step 3 doesn't name the button used to reach the DPNS registration screen
Step 3 reads 'Navigate to DPNS registration — the heading reads "Register DPNS Name"', which is a verification rather than an action. The actual navigation is clicking the per-identity '📛 Register DPNS Name' button (src/ui/identities/identities_screen.rs:684). Every other navigation step in this suite names the literal button label (e.g., TC-IDN-008-01 step 3 says 'Click the "Keys" button'). Rewrite as: 'Click the "📛 Register DPNS Name" button for that identity and verify the heading reads "Register DPNS Name".'
source: ['claude']
🤖 Fix this with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `docs/test-cases.md`:
- [SUGGESTION] line 409: TC-DPN-001-01 step 3 doesn't name the button used to reach the DPNS registration screen
Step 3 reads 'Navigate to DPNS registration — the heading reads "Register DPNS Name"', which is a verification rather than an action. The actual navigation is clicking the per-identity '📛 Register DPNS Name' button (src/ui/identities/identities_screen.rs:684). Every other navigation step in this suite names the literal button label (e.g., TC-IDN-008-01 step 3 says 'Click the "Keys" button'). Rewrite as: 'Click the "📛 Register DPNS Name" button for that identity and verify the heading reads "Register DPNS Name".'
| | **Test Case ID** | TC-IDN-004-01 | | ||
| | **Short Description** | Add credits to an existing identity | | ||
| | **Pre-Conditions** | Identity from TC-IDN-001-01 exists; **Identity Test** wallet has remaining funds | | ||
| | **Test Steps** | 1. Click **"Identities"** in the left sidebar.<br>2. Select the identity created in TC-IDN-001-01.<br>3. Open top-up and verify the breadcrumb shows **Top Up Identity**.<br>4. Under **"Choose your funding method"**, choose **"Wallet Balance"** or **"Unused Asset Locks"**, whichever is available.<br>5. Enter amount: `0.1` tDASH worth of credits.<br>6. Confirm the top-up transaction.<br>7. Wait for Platform confirmation.<br>8. Verify the identity's credit balance increased. | |
There was a problem hiding this comment.
🟡 Suggestion: TC-IDN-004-01 step 3 'Open top-up' should name the actual '💰 Top up' button
Step 3 says 'Open top-up and verify the breadcrumb shows "Top Up Identity".' The per-identity button on the Identities screen is labelled '💰 Top up' (src/ui/identities/identities_screen.rs:648). For consistency with the rest of the suite, rewrite step 3 as: 'Click the "💰 Top up" button for that identity and verify the breadcrumb shows "Top Up Identity".'
source: ['claude']
🤖 Fix this with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `docs/test-cases.md`:
- [SUGGESTION] line 366: TC-IDN-004-01 step 3 'Open top-up' should name the actual '💰 Top up' button
Step 3 says 'Open top-up and verify the breadcrumb shows "Top Up Identity".' The per-identity button on the Identities screen is labelled '💰 Top up' (src/ui/identities/identities_screen.rs:648). For consistency with the rest of the suite, rewrite step 3 as: 'Click the "💰 Top up" button for that identity and verify the breadcrumb shows "Top Up Identity".'
| # Testnet SPV mode — DAPI addresses plus placeholder Core RPC fields | ||
| TESTNET_dapi_addresses=https://34.214.48.68:1443,https://52.12.176.90:1443,https://52.34.144.50:1443,https://44.240.98.102:1443,https://54.201.32.131:1443,https://52.10.229.11:1443,https://52.13.132.146:1443,https://52.40.219.41:1443,https://54.149.33.167:1443,https://35.164.23.245:1443,https://52.33.28.47:1443,https://52.43.13.92:1443,https://52.89.154.48:1443,https://52.24.124.162:1443,https://35.85.21.179:1443,https://54.187.14.232:1443,https://54.68.235.201:1443,https://52.13.250.182:1443 | ||
| TESTNET_show_in_ui=true | ||
|
|
||
| # Core RPC fields are structurally required by NetworkConfig even in SPV mode, | ||
| # but are not actively used at runtime. Supply placeholder values. | ||
| TESTNET_core_host=127.0.0.1 | ||
| TESTNET_core_rpc_port=19998 | ||
| TESTNET_core_rpc_user=user | ||
| TESTNET_core_rpc_password=password | ||
| TESTNET_insight_api_url=https://insight.testnet.networks.dash.org:3002/insight-api | ||
| # core_zmq_endpoint is optional and can be omitted |
There was a problem hiding this comment.
💬 Nitpick: Inlined testnet .env block duplicates and is already drifting from the bundled .env.example
The prerequisites doc inlines the full TESTNET_* endpoint block (18 hardcoded DAPI IPs and a specific insight URL) instead of pointing at the bundled .env.example that DET copies into the config dir via copy_env_file_if_not_exists() in src/app_dir.rs. The duplication is already out of sync: .env.example:19 uses TESTNET_insight_api_url=https://testnet-insight.dash.org/insight-api, while docs/test-prerequisites.md:99 hardcodes https://insight.testnet.networks.dash.org:3002/insight-api. Testnet evonode addresses also churn, so a recurring weekly smoke doc that pins them will silently rot. Point testers at .env.example (or quote only the smoke-suite-specific deltas) so the docs track the source of truth.
source: ['claude', 'codex']
54b53b0 to
790d7ad
Compare
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
Documentation-only PR adding a weekly SPV smoke-test suite. The suite is generally useful, but the shared SPV setup is missing the explicit Connect action required to start SPV sync, so fresh smoke-test runs can stall before any cases execute. A few specification gaps also make the suite harder to run repeatably: several required seed phrases are never used, and token/contract ID cases rely on out-of-band values.
Reviewed commit: 790d7ad
🔴 1 blocking | 🟡 2 suggestion(s) | 💬 2 nitpick(s)
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `docs/test-cases.md`:
- [BLOCKING] lines 32-40: SPV setup is missing the Connect click that actually starts sync
Step 6 selects **"SPV Client"** and step 7 tells the tester to wait for the status indicator to turn green, but selecting the backend only updates `core_backend_mode` — SPV does not start until the user clicks the **Connect** button that calls `start_spv()` (see `src/ui/network_chooser_screen.rs:529-541`). A fresh weekly run will hang here forever. The same omission is repeated in `docs/test-prerequisites.md` under "Select SPV Backend" (lines 117-125). Add an explicit step that clicks **"Connect"** after selecting **"SPV Client"** in both places.
- [SUGGESTION] lines 500-528: Token and contract import cases are not reproducible from the documented inputs
TC-TOK-003-01 requires "a known token contract ID" and TC-DOC-003-01 requires the DPNS contract ID, but neither `test-prerequisites.md` nor Session Setup provides concrete values or a discovery procedure for either one. These weekly smoke cases therefore depend on out-of-band tribal knowledge instead of the formal test spec. Add stable testnet IDs to the prerequisites/Test Data rows, or add explicit discovery steps before execution.
In `docs/test-prerequisites.md`:
- [SUGGESTION] lines 19-34: Six required mnemonics are declared but never referenced
The Required Variables table marks `TC_TOK_MNEMONIC`, `TC_DPY_MNEMONIC`, `TC_DOC_MNEMONIC`, `TC_DEV_MNEMONIC`, `TC_NET_MNEMONIC`, and `TC_ALK_MNEMONIC` as required, but `docs/test-cases.md` never references them. The executable setup imports the Bank, Send Test, and Identity Test wallets, and the token, DashPay, document, developer, network, and asset-lock cases all reuse those wallets/identities. This forces operators to generate and protect six unused seed phrases, increasing secret-handling overhead without making the smoke suite more isolated. Either move these to an optional/future section or add cases that genuinely need separate wallets.
| ### 1. Configure SPV Mode | ||
|
|
||
| 1. Click **"Settings"** in the left sidebar. | ||
| 2. Click the **"Advanced Settings"** header to expand it. | ||
| 3. Check the **"Developer mode"** checkbox. | ||
| 4. Stay on **Settings (Network Chooser)** and verify the **"Connection Type"** dropdown appears in **"Connection Settings"**. | ||
| 5. In **"Connection Settings"**, open the **"Network:"** dropdown and select **"Testnet"**. | ||
| 6. In **"Connection Settings"**, open the **"Connection Type"** dropdown and select **"SPV Client"**. | ||
| 7. Wait for the connection status indicator (top bar) to turn green. |
There was a problem hiding this comment.
🔴 Blocking: SPV setup is missing the Connect click that actually starts sync
Step 6 selects "SPV Client" and step 7 tells the tester to wait for the status indicator to turn green, but selecting the backend only updates core_backend_mode — SPV does not start until the user clicks the Connect button that calls start_spv() (see src/ui/network_chooser_screen.rs:529-541). A fresh weekly run will hang here forever. The same omission is repeated in docs/test-prerequisites.md under "Select SPV Backend" (lines 117-125). Add an explicit step that clicks "Connect" after selecting "SPV Client" in both places.
💡 Suggested change
| ### 1. Configure SPV Mode | |
| 1. Click **"Settings"** in the left sidebar. | |
| 2. Click the **"Advanced Settings"** header to expand it. | |
| 3. Check the **"Developer mode"** checkbox. | |
| 4. Stay on **Settings (Network Chooser)** and verify the **"Connection Type"** dropdown appears in **"Connection Settings"**. | |
| 5. In **"Connection Settings"**, open the **"Network:"** dropdown and select **"Testnet"**. | |
| 6. In **"Connection Settings"**, open the **"Connection Type"** dropdown and select **"SPV Client"**. | |
| 7. Wait for the connection status indicator (top bar) to turn green. | |
| ### 1. Configure SPV Mode | |
| 1. Click **"Settings"** in the left sidebar. | |
| 2. Click the **"Advanced Settings"** header to expand it. | |
| 3. Check the **"Developer mode"** checkbox. | |
| 4. Stay on **Settings (Network Chooser)** and verify the **"Connection Type"** dropdown appears in **"Connection Settings"**. | |
| 5. In **"Connection Settings"**, open the **"Network:"** dropdown and select **"Testnet"**. | |
| 6. In **"Connection Settings"**, open the **"Connection Type"** dropdown and select **"SPV Client"**. | |
| 7. Click the **"Connect"** button. | |
| 8. Wait for the connection status indicator (top bar) to turn green. |
source: ['codex']
🤖 Fix this with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `docs/test-cases.md`:
- [BLOCKING] lines 32-40: SPV setup is missing the Connect click that actually starts sync
Step 6 selects **"SPV Client"** and step 7 tells the tester to wait for the status indicator to turn green, but selecting the backend only updates `core_backend_mode` — SPV does not start until the user clicks the **Connect** button that calls `start_spv()` (see `src/ui/network_chooser_screen.rs:529-541`). A fresh weekly run will hang here forever. The same omission is repeated in `docs/test-prerequisites.md` under "Select SPV Backend" (lines 117-125). Add an explicit step that clicks **"Connect"** after selecting **"SPV Client"** in both places.
|
|
||
| | Variable | Words | Purpose | | ||
| |---|---|---| | ||
| | `BANK_MNEMONIC` | 24 | Bank wallet — pre-funded, password-protected | | ||
| | `BANK_PASSWORD` | — | Bank wallet encryption password | | ||
| | `TC_WAL_MNEMONIC` | 24 | Wallet management tests (import, rename, lock, remove) | | ||
| | `TC_WAL_PASSWORD` | — | Password for wallet-management test wallet | | ||
| | `TC_SND_MNEMONIC` | 21 | Send/receive tests | | ||
| | `TC_IDN_MNEMONIC` | 21 | Identity registration and top-up tests | | ||
| | `TC_TOK_MNEMONIC` | 18 | Token operation tests | | ||
| | `TC_DPY_MNEMONIC` | 18 | DashPay profile and contact tests | | ||
| | `TC_DOC_MNEMONIC` | 15 | Contract and document tests | | ||
| | `TC_DEV_MNEMONIC` | 15 | Developer tools tests | | ||
| | `TC_NET_MNEMONIC` | 12 | Network/settings tests | | ||
| | `TC_ALK_MNEMONIC` | 12 | Asset lock tests | | ||
|
|
There was a problem hiding this comment.
🟡 Suggestion: Six required mnemonics are declared but never referenced
The Required Variables table marks TC_TOK_MNEMONIC, TC_DPY_MNEMONIC, TC_DOC_MNEMONIC, TC_DEV_MNEMONIC, TC_NET_MNEMONIC, and TC_ALK_MNEMONIC as required, but docs/test-cases.md never references them. The executable setup imports the Bank, Send Test, and Identity Test wallets, and the token, DashPay, document, developer, network, and asset-lock cases all reuse those wallets/identities. This forces operators to generate and protect six unused seed phrases, increasing secret-handling overhead without making the smoke suite more isolated. Either move these to an optional/future section or add cases that genuinely need separate wallets.
source: ['claude', 'codex']
🤖 Fix this with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `docs/test-prerequisites.md`:
- [SUGGESTION] lines 19-34: Six required mnemonics are declared but never referenced
The Required Variables table marks `TC_TOK_MNEMONIC`, `TC_DPY_MNEMONIC`, `TC_DOC_MNEMONIC`, `TC_DEV_MNEMONIC`, `TC_NET_MNEMONIC`, and `TC_ALK_MNEMONIC` as required, but `docs/test-cases.md` never references them. The executable setup imports the Bank, Send Test, and Identity Test wallets, and the token, DashPay, document, developer, network, and asset-lock cases all reuse those wallets/identities. This forces operators to generate and protect six unused seed phrases, increasing secret-handling overhead without making the smoke suite more isolated. Either move these to an optional/future section or add cases that genuinely need separate wallets.
| ### TC-TOK-003-01: Add a token by contract ID | ||
|
|
||
| | Field | Value | | ||
| |-------|-------| | ||
| | **Use Case ID** | TOK-003 | | ||
| | **Test Case ID** | TC-TOK-003-01 | | ||
| | **Short Description** | Add a token using a contract or token ID | | ||
| | **Pre-Conditions** | DET running on testnet, a known token contract ID available | | ||
| | **Test Steps** | 1. Click **"Tokens"** in the left sidebar.<br>2. Click the **"Add Token"** button.<br>3. Verify the heading reads **"Add Token"**.<br>4. In the **"Contract or Token ID:"** field, enter a known testnet token contract ID.<br>5. Click **"Search"** and select the desired token if multiple results are shown.<br>6. Click **"Add Token"** and verify the **"Token Added Successfully"** screen appears.<br>7. Click **"Back to Tokens screen"**, return to **"My Tokens"**, and verify the added token appears in the local tracked token list by **Token Name** or **Token ID**. | | ||
| | **Test Data** | A valid token contract ID from testnet | | ||
| | **Expected Result** | Token is added successfully and saved as a locally tracked token in **"My Tokens"**. The tracked entry is visible by **Token Name** or **Token ID**; token balance may remain zero or absent unless a local identity holds a non-zero balance. | | ||
| | **Post-Condition** | Token available for viewing. | | ||
|
|
||
| --- | ||
|
|
||
| ## Contracts and Documents (DOC) | ||
|
|
||
| ### TC-DOC-003-01: Import a contract by ID | ||
|
|
||
| | Field | Value | | ||
| |-------|-------| | ||
| | **Use Case ID** | DOC-003 | | ||
| | **Test Case ID** | TC-DOC-003-01 | | ||
| | **Short Description** | Import the DPNS system contract by ID | | ||
| | **Pre-Conditions** | DET running on testnet | | ||
| | **Test Steps** | 1. Click **"Contracts"** in the left sidebar.<br>2. Click the **"Add Contracts"** button.<br>3. Verify the breadcrumb reads **Contracts > Add Contracts**.<br>4. Enter the DPNS contract ID in the input field.<br>5. Confirm the import.<br>6. Verify the contract appears in the contracts list with its name and available document types. | | ||
| | **Test Data** | DPNS contract ID (system contract, available on testnet) | | ||
| | **Expected Result** | Contract imported. Shows contract name and document types in the list. | | ||
| | **Post-Condition** | DPNS contract available for document browsing. | |
There was a problem hiding this comment.
🟡 Suggestion: Token and contract import cases are not reproducible from the documented inputs
TC-TOK-003-01 requires "a known token contract ID" and TC-DOC-003-01 requires the DPNS contract ID, but neither test-prerequisites.md nor Session Setup provides concrete values or a discovery procedure for either one. These weekly smoke cases therefore depend on out-of-band tribal knowledge instead of the formal test spec. Add stable testnet IDs to the prerequisites/Test Data rows, or add explicit discovery steps before execution.
source: ['codex']
🤖 Fix this with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `docs/test-cases.md`:
- [SUGGESTION] lines 500-528: Token and contract import cases are not reproducible from the documented inputs
TC-TOK-003-01 requires "a known token contract ID" and TC-DOC-003-01 requires the DPNS contract ID, but neither `test-prerequisites.md` nor Session Setup provides concrete values or a discovery procedure for either one. These weekly smoke cases therefore depend on out-of-band tribal knowledge instead of the formal test spec. Add stable testnet IDs to the prerequisites/Test Data rows, or add explicit discovery steps before execution.
| | **Test Case ID** | TC-NET-010-01 | | ||
| | **Short Description** | Verify onboarding wizard on fresh start | | ||
| | **Pre-Conditions** | DET data directory removed (fresh start — see [test-prerequisites.md](test-prerequisites.md#fresh-start-for-onboarding-tests)); `.env` file in place | | ||
| | **Test Steps** | 1. Launch DET.<br>2. Verify the welcome screen appears with three cards: **"Create Wallet"**, **"Import Wallet"**, and **"Just Explore"**.<br>3. Click **"Just Explore"**.<br>4. Verify the app loads — the left sidebar shows navigation items (**"Wallets"**, **"Identities"**, **"Contracts"**, **"Tokens"**, **"Tools"**, **"Settings"**, etc.) and no wallet is selected. | |
There was a problem hiding this comment.
💬 Nitpick: TC-NET-010-01 omits the Dashpay sidebar item from the expected navigation
The expected sidebar list reads "Wallets", "Identities", "Contracts", "Tokens", "Tools", "Settings", etc., but the app has a top-level "Dashpay" entry (src/ui/components/left_panel.rs:121-122) and later cases instruct the tester to click "Dashpay". Listing it here keeps the onboarding verification consistent with the rest of the suite.
source: ['claude']
| | **Test Case ID** | TC-ALK-001-01 | | ||
| | **Short Description** | Create an asset lock from the Identity Test wallet | | ||
| | **Pre-Conditions** | **Identity Test** wallet selected with >= 2 tDASH | | ||
| | **Test Steps** | 1. Click **"Wallets"** in the left sidebar.<br>2. Select the **Identity Test** wallet.<br>3. In the **"Asset Locks"** section, click **"Create Asset Lock"**.<br>4. Verify the breadcrumb/heading shows **Wallets > Create Asset Lock** / **"Create Asset Lock"**.<br>5. Under **"Select Asset Lock Purpose"**, click **"Registration"**.<br>6. In **"Select how much you would like to transfer?"**, enter `0.5` tDASH.<br>7. Review the generated funding address/QR code and the estimated fee.<br>8. Send funds from the **Identity Test** wallet to the displayed address if prompted, then wait through **"Waiting for funds..."** and **"Waiting for Core Chain to produce proof of asset lock..."**.<br>9. Verify **"Asset Lock Created Successfully!"** and wait for the asset lock transaction/proof to confirm. | |
There was a problem hiding this comment.
💬 Nitpick: TC-ALK-001-01 step 8 is ambiguous about when to send funds
Step 8 says to send funds from the Identity Test wallet to the displayed address "if prompted". The asset-lock flow can use existing wallet UTXOs, so a tester may not know whether this manual self-send is always required, only required when the wallet has no suitable UTXO, or optional. Clarify the condition for the manual send, or remove the step if pre-funding the Identity Test wallet is sufficient.
source: ['claude']
790d7ad to
6bdb718
Compare
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
Documentation-only PR. Most prior blockers from earlier rounds are now resolved on 6bdb718. One remaining blocking issue: TC-DPY-002-01 still describes a DashPay profile-search flow that does not match the actual UI (no explicit Enter/Search trigger, and verification asserts 'Display Name:' / 'Bio:' labels that the search result card does not render). The .env duplication issue is real and now drifting on this SHA (insight_api_url differs from the bundled .env.example). Three smaller nitpick/suggestion follow-ups remain (sidebar listing omits Dashpay, TC-ALK-001-01 step 8 still ambiguous about manual funding, TC-DOC-003-01 step 5 should name the 'Add Contracts' button).
Reviewed commit: 6bdb718
🔴 1 blocking | 🟡 1 suggestion(s)
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `docs/test-cases.md`:
- [BLOCKING] line 466: TC-DPY-002-01 search flow is non-executable: missing trigger and asserts labels the search UI doesn't render
Two issues make this case non-executable as written:
1. Step 4 only says "Type the username" and step 5 jumps straight to "Verify search results appear," but the Profile Search UI does not search on plain typing. `src/ui/dashpay/profile_search.rs:142-150` dispatches the query only when the text field loses focus with Enter pressed, or when the `Search` button is clicked. A tester following the steps literally will sit on an empty results view forever.
2. Step 6 verifies labelled fields **Display Name:** and **Bio:** on the result, but `profile_search.rs:187-216` renders the username as the primary heading, then the display name as an unlabeled secondary line, and the public_message as unlabeled italic preview text. There is no `Display Name:` or `Bio:` label on the result card. The labelled assertions belong on the My Profile view (TC-DPY-001-01 step 10), not here.
Add an explicit Enter/Search click step and re-word the verification to match what the search card actually shows.
- [SUGGESTION] line 526: TC-DOC-003-01 should name the actual 'Add Contracts' button and 'Contract 1:' input label
Step 5 reads "Confirm the import." but the submit control in `src/ui/contracts_documents/add_contracts_screen.rs` is a button labelled **Add Contracts**. Every other navigation step in this suite names the literal button label (TC-IDN-008-01 step 3, the recently-fixed TC-IDN-004-01 step 3, etc.). Step 4 says "input field" — the on-screen label is `Contract 1:` (add_contracts_screen.rs:104), under the heading `Enter Contract Identifiers:` (line 99). Naming both keeps QA from guessing.
| | **Test Case ID** | TC-DPY-002-01 | | ||
| | **Short Description** | Search for a known DashPay profile by username | | ||
| | **Pre-Conditions** | DashPay profile created in TC-DPY-001-01 | | ||
| | **Test Steps** | 1. Click **"Dashpay"** in the left sidebar.<br>2. Click **"Search Profiles"** in the DashPay subscreen chooser.<br>3. Locate the search field (hint: *"Enter DPNS username..."*).<br>4. Type the username registered in TC-DPN-001-01.<br>5. Verify search results appear.<br>6. Verify the result shows **Display Name:** `Smoke Test Bot` and **Bio:** `Automated QA testing`. | |
There was a problem hiding this comment.
🔴 Blocking: TC-DPY-002-01 search flow is non-executable: missing trigger and asserts labels the search UI doesn't render
Two issues make this case non-executable as written:
-
Step 4 only says "Type the username" and step 5 jumps straight to "Verify search results appear," but the Profile Search UI does not search on plain typing.
src/ui/dashpay/profile_search.rs:142-150dispatches the query only when the text field loses focus with Enter pressed, or when theSearchbutton is clicked. A tester following the steps literally will sit on an empty results view forever. -
Step 6 verifies labelled fields Display Name: and Bio: on the result, but
profile_search.rs:187-216renders the username as the primary heading, then the display name as an unlabeled secondary line, and the public_message as unlabeled italic preview text. There is noDisplay Name:orBio:label on the result card. The labelled assertions belong on the My Profile view (TC-DPY-001-01 step 10), not here.
Add an explicit Enter/Search click step and re-word the verification to match what the search card actually shows.
source: ['codex']
🤖 Fix this with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `docs/test-cases.md`:
- [BLOCKING] line 466: TC-DPY-002-01 search flow is non-executable: missing trigger and asserts labels the search UI doesn't render
Two issues make this case non-executable as written:
1. Step 4 only says "Type the username" and step 5 jumps straight to "Verify search results appear," but the Profile Search UI does not search on plain typing. `src/ui/dashpay/profile_search.rs:142-150` dispatches the query only when the text field loses focus with Enter pressed, or when the `Search` button is clicked. A tester following the steps literally will sit on an empty results view forever.
2. Step 6 verifies labelled fields **Display Name:** and **Bio:** on the result, but `profile_search.rs:187-216` renders the username as the primary heading, then the display name as an unlabeled secondary line, and the public_message as unlabeled italic preview text. There is no `Display Name:` or `Bio:` label on the result card. The labelled assertions belong on the My Profile view (TC-DPY-001-01 step 10), not here.
Add an explicit Enter/Search click step and re-word the verification to match what the search card actually shows.
| | **Test Case ID** | TC-DOC-003-01 | | ||
| | **Short Description** | Import the DPNS system contract by ID | | ||
| | **Pre-Conditions** | DET running on testnet | | ||
| | **Test Steps** | 1. Click **"Contracts"** in the left sidebar.<br>2. Click the **"Add Contracts"** button.<br>3. Verify the breadcrumb reads **Contracts > Add Contracts**.<br>4. Enter `${DPNS_CONTRACT_ID}` in the input field.<br>5. Confirm the import.<br>6. Verify the contract appears in the contracts list with its name and available document types. | |
There was a problem hiding this comment.
🟡 Suggestion: TC-DOC-003-01 should name the actual 'Add Contracts' button and 'Contract 1:' input label
Step 5 reads "Confirm the import." but the submit control in src/ui/contracts_documents/add_contracts_screen.rs is a button labelled Add Contracts. Every other navigation step in this suite names the literal button label (TC-IDN-008-01 step 3, the recently-fixed TC-IDN-004-01 step 3, etc.). Step 4 says "input field" — the on-screen label is Contract 1: (add_contracts_screen.rs:104), under the heading Enter Contract Identifiers: (line 99). Naming both keeps QA from guessing.
💡 Suggested change
| | **Test Steps** | 1. Click **"Contracts"** in the left sidebar.<br>2. Click the **"Add Contracts"** button.<br>3. Verify the breadcrumb reads **Contracts > Add Contracts**.<br>4. Enter `${DPNS_CONTRACT_ID}` in the input field.<br>5. Confirm the import.<br>6. Verify the contract appears in the contracts list with its name and available document types. | | |
| | **Test Steps** | 1. Click **"Contracts"** in the left sidebar.<br>2. Click the **"Add Contracts"** button.<br>3. Verify the breadcrumb reads **Contracts > Add Contracts**.<br>4. Under the **"Enter Contract Identifiers:"** heading, enter `${DPNS_CONTRACT_ID}` in the **"Contract 1:"** field.<br>5. Click the **"Add Contracts"** button to submit.<br>6. Verify the contract appears in the contracts list with its name and available document types. | |
source: ['claude']
🤖 Fix this with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `docs/test-cases.md`:
- [SUGGESTION] line 526: TC-DOC-003-01 should name the actual 'Add Contracts' button and 'Contract 1:' input label
Step 5 reads "Confirm the import." but the submit control in `src/ui/contracts_documents/add_contracts_screen.rs` is a button labelled **Add Contracts**. Every other navigation step in this suite names the literal button label (TC-IDN-008-01 step 3, the recently-fixed TC-IDN-004-01 step 3, etc.). Step 4 says "input field" — the on-screen label is `Contract 1:` (add_contracts_screen.rs:104), under the heading `Enter Contract Identifiers:` (line 99). Naming both keeps QA from guessing.
6bdb718 to
956c35d
Compare
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
Documentation-only PR adding a weekly SPV smoke-test suite. Most prior issues are resolved on 956c35d. One blocking issue remains: TC-DPY-001-01 step 10 asserts a 'Display Name:' label on the profile display view that does not exist in src/ui/dashpay/profile_screen.rs (only 'Bio:' is labeled; the display name is a bare heading). One valid suggestion: TC-ALK-001-01 step 7 asks the tester to verify an 'estimated fee' that the asset-lock screen does not render. Three nitpicks remain: the inlined TESTNET_insight_api_url has already drifted from .env.example, TC-NET-010-01's expected sidebar omits 'Dashpay', and TC-ALK-001-01 step 8's 'if prompted' qualifier is misleading because the manual self-send is always required.
Reviewed commit: 956c35d
🔴 1 blocking | 🟡 1 suggestion(s) | 💬 3 nitpick(s)
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `docs/test-cases.md`:
- [BLOCKING] line 453: TC-DPY-001-01 step 10 asserts a 'Display Name:' label that the profile display view does not render
Step 10 reads: 'Verify the profile screen shows **Display Name:** `Smoke Test Bot` and **Bio:** `Automated QA testing`.' In src/ui/dashpay/profile_screen.rs the display-mode view renders the display name as a bare heading (line 1177: `ui.label(RichText::new(&profile.display_name).heading())`) with no field label preceding it. Only the bio gets a label — 'Bio:' at line 1230. The 'Display Name:' label only appears in edit mode. A QA tester following step 10 literally will look for the 'Display Name:' label on the post-Save view, fail to find it, and mark this case as failed. This is the same class of bug as the previously-resolved DPY-002 issue, but on the My Profile display side. Re-word step 10 to verify the display name appears as the prominent heading (and the username as `@<dpns-name>` below it) and that **Bio:** `Automated QA testing` appears as a labeled field underneath.
- [SUGGESTION] line 337: TC-ALK-001-01 step 7 tells the tester to review an 'estimated fee' that the asset lock screen does not render
Step 7 reads: 'Review the generated funding address/QR code and the estimated fee.' Grepping src/ui/wallets/create_asset_lock_screen.rs for 'fee'/'Fee'/'estimated' returns zero matches — the create-asset-lock screen renders an amount input, a QR code/funding address, and waiting/success headings, but no fee summary. Either drop the 'estimated fee' verification from step 7, or move that check to TC-IDN-001-01 step 7 (which already mentions 'Verify the **Estimated Fee:** summary' on the identity-creation screen where a fee really is shown).
| | **Test Case ID** | TC-DPY-001-01 | | ||
| | **Short Description** | Create a DashPay profile for the test identity | | ||
| | **Pre-Conditions** | Identity from TC-IDN-001-01 with DPNS username and sufficient credits | | ||
| | **Test Steps** | 1. Click **"Dashpay"** in the left sidebar.<br>2. Click **"My Profile"** in the DashPay subscreen chooser.<br>3. Click **"Refresh"** in the top panel and wait for the profile view to load.<br>4. If the **"No DashPay Profile"** card appears, click **"Create Profile"**.<br>5. In the **"Display Name:"** field (hint: *"Enter your display name (required)"*), type `Smoke Test Bot`.<br>6. In the **"Bio/Status:"** field, type `Automated QA testing`.<br>7. Leave the **"Avatar URL:"** field empty.<br>8. Click **"Save Profile"**.<br>9. Wait for Platform confirmation.<br>10. Verify the profile screen shows **Display Name:** `Smoke Test Bot` and **Bio:** `Automated QA testing`. | |
There was a problem hiding this comment.
🔴 Blocking: TC-DPY-001-01 step 10 asserts a 'Display Name:' label that the profile display view does not render
Step 10 reads: 'Verify the profile screen shows Display Name: Smoke Test Bot and Bio: Automated QA testing.' In src/ui/dashpay/profile_screen.rs the display-mode view renders the display name as a bare heading (line 1177: ui.label(RichText::new(&profile.display_name).heading())) with no field label preceding it. Only the bio gets a label — 'Bio:' at line 1230. The 'Display Name:' label only appears in edit mode. A QA tester following step 10 literally will look for the 'Display Name:' label on the post-Save view, fail to find it, and mark this case as failed. This is the same class of bug as the previously-resolved DPY-002 issue, but on the My Profile display side. Re-word step 10 to verify the display name appears as the prominent heading (and the username as @<dpns-name> below it) and that Bio: Automated QA testing appears as a labeled field underneath.
source: ['claude']
🤖 Fix this with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `docs/test-cases.md`:
- [BLOCKING] line 453: TC-DPY-001-01 step 10 asserts a 'Display Name:' label that the profile display view does not render
Step 10 reads: 'Verify the profile screen shows **Display Name:** `Smoke Test Bot` and **Bio:** `Automated QA testing`.' In src/ui/dashpay/profile_screen.rs the display-mode view renders the display name as a bare heading (line 1177: `ui.label(RichText::new(&profile.display_name).heading())`) with no field label preceding it. Only the bio gets a label — 'Bio:' at line 1230. The 'Display Name:' label only appears in edit mode. A QA tester following step 10 literally will look for the 'Display Name:' label on the post-Save view, fail to find it, and mark this case as failed. This is the same class of bug as the previously-resolved DPY-002 issue, but on the My Profile display side. Re-word step 10 to verify the display name appears as the prominent heading (and the username as `@<dpns-name>` below it) and that **Bio:** `Automated QA testing` appears as a labeled field underneath.
| | **Test Case ID** | TC-ALK-001-01 | | ||
| | **Short Description** | Create an asset lock from the Identity Test wallet | | ||
| | **Pre-Conditions** | **Identity Test** wallet selected with >= 2 tDASH | | ||
| | **Test Steps** | 1. Click **"Wallets"** in the left sidebar.<br>2. Select the **Identity Test** wallet.<br>3. In the **"Asset Locks"** section, click **"Create Asset Lock"**.<br>4. Verify the breadcrumb/heading shows **Wallets > Create Asset Lock** / **"Create Asset Lock"**.<br>5. Under **"Select Asset Lock Purpose"**, click **"Registration"**.<br>6. In **"Select how much you would like to transfer?"**, enter `0.5` tDASH.<br>7. Review the generated funding address/QR code and the estimated fee.<br>8. Send funds from the **Identity Test** wallet to the displayed address if prompted, then wait through **"Waiting for funds..."** and **"Waiting for Core Chain to produce proof of asset lock..."**.<br>9. Verify **"Asset Lock Created Successfully!"** and wait for the asset lock transaction/proof to confirm. | |
There was a problem hiding this comment.
🟡 Suggestion: TC-ALK-001-01 step 7 tells the tester to review an 'estimated fee' that the asset lock screen does not render
Step 7 reads: 'Review the generated funding address/QR code and the estimated fee.' Grepping src/ui/wallets/create_asset_lock_screen.rs for 'fee'/'Fee'/'estimated' returns zero matches — the create-asset-lock screen renders an amount input, a QR code/funding address, and waiting/success headings, but no fee summary. Either drop the 'estimated fee' verification from step 7, or move that check to TC-IDN-001-01 step 7 (which already mentions 'Verify the Estimated Fee: summary' on the identity-creation screen where a fee really is shown).
source: ['claude']
🤖 Fix this with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `docs/test-cases.md`:
- [SUGGESTION] line 337: TC-ALK-001-01 step 7 tells the tester to review an 'estimated fee' that the asset lock screen does not render
Step 7 reads: 'Review the generated funding address/QR code and the estimated fee.' Grepping src/ui/wallets/create_asset_lock_screen.rs for 'fee'/'Fee'/'estimated' returns zero matches — the create-asset-lock screen renders an amount input, a QR code/funding address, and waiting/success headings, but no fee summary. Either drop the 'estimated fee' verification from step 7, or move that check to TC-IDN-001-01 step 7 (which already mentions 'Verify the **Estimated Fee:** summary' on the identity-creation screen where a fee really is shown).
| | **Test Case ID** | TC-ALK-001-01 | | ||
| | **Short Description** | Create an asset lock from the Identity Test wallet | | ||
| | **Pre-Conditions** | **Identity Test** wallet selected with >= 2 tDASH | | ||
| | **Test Steps** | 1. Click **"Wallets"** in the left sidebar.<br>2. Select the **Identity Test** wallet.<br>3. In the **"Asset Locks"** section, click **"Create Asset Lock"**.<br>4. Verify the breadcrumb/heading shows **Wallets > Create Asset Lock** / **"Create Asset Lock"**.<br>5. Under **"Select Asset Lock Purpose"**, click **"Registration"**.<br>6. In **"Select how much you would like to transfer?"**, enter `0.5` tDASH.<br>7. Review the generated funding address/QR code and the estimated fee.<br>8. Send funds from the **Identity Test** wallet to the displayed address if prompted, then wait through **"Waiting for funds..."** and **"Waiting for Core Chain to produce proof of asset lock..."**.<br>9. Verify **"Asset Lock Created Successfully!"** and wait for the asset lock transaction/proof to confirm. | |
There was a problem hiding this comment.
💬 Nitpick: TC-ALK-001-01 step 8 'if prompted' wording is misleading — the manual self-send is always required
Step 8 reads 'Send funds from the Identity Test wallet to the displayed address if prompted...'. The asset-lock flow in src/ui/wallets/create_asset_lock_screen.rs always generates a dedicated funding address/QR and sits in WalletFundedScreenStep::WaitingOnFunds ('Waiting for funds...') until a UTXO arrives at that funding address. There is no path where the flow auto-completes without the tester sending funds — the 'if prompted' qualifier implies the manual transfer might be skipped, which can leave a tester stuck on 'Waiting for funds...' indefinitely. State the action unconditionally: 'Send funds from the Identity Test wallet to the displayed funding address (this is required to advance past Waiting for funds...), then wait through Waiting for funds... and Waiting for Core Chain to produce proof of asset lock...'.
source: ['claude', 'codex']
| TESTNET_core_rpc_port=19998 | ||
| TESTNET_core_rpc_user=user | ||
| TESTNET_core_rpc_password=password | ||
| TESTNET_insight_api_url=https://insight.testnet.networks.dash.org:3002/insight-api |
There was a problem hiding this comment.
💬 Nitpick: Inlined TESTNET_insight_api_url has already drifted from the bundled .env.example
The prerequisites doc inlines TESTNET_insight_api_url=https://insight.testnet.networks.dash.org:3002/insight-api (line 122). The bundled .env.example at the repo root — which DET copies into the app data directory on first launch — uses TESTNET_insight_api_url=https://testnet-insight.dash.org/insight-api (.env.example:19). The two values already disagree on this SHA, and the 18-address TESTNET_dapi_addresses list (line 113) is duplicated as well and will silently rot the same way. Point testers at .env.example as the source of truth (or quote only smoke-suite-specific deltas) so the doc tracks the bundled config.
source: ['claude', 'codex']
| | **Test Case ID** | TC-NET-010-01 | | ||
| | **Short Description** | Verify onboarding wizard on fresh start | | ||
| | **Pre-Conditions** | DET data directory removed (fresh start — see [test-prerequisites.md](test-prerequisites.md#fresh-start-for-onboarding-tests)); `.env` file in place | | ||
| | **Test Steps** | 1. Launch DET.<br>2. Verify the welcome screen appears with three cards: **"Create Wallet"**, **"Import Wallet"**, and **"Just Explore"**.<br>3. Click **"Just Explore"**.<br>4. Verify the app loads — the left sidebar shows navigation items (**"Wallets"**, **"Identities"**, **"Contracts"**, **"Tokens"**, **"Tools"**, **"Settings"**, etc.) and no wallet is selected. | |
There was a problem hiding this comment.
💬 Nitpick: TC-NET-010-01 step 4 sidebar list omits Dashpay
Step 4 enumerates the expected left-sidebar items as Wallets, Identities, Contracts, Tokens, Tools, Settings, etc., but add_left_panel in src/ui/components/left_panel.rs:119-155 registers Dashpay as the first top-level entry — and subsequent cases in this same suite (TC-DPY-001-01, TC-DPY-002-01) tell the tester to click Dashpay. Listing it in the onboarding verification keeps the sidebar enumeration consistent with the real UI and avoids implying that a missing Dashpay item would be acceptable.
source: ['claude', 'codex']
956c35d to
f063d8c
Compare
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
I verified the reported issues against the checked-out f063d8ce5b4c2ccc851ab5390ca334a0aa56c83d tree. Two findings are real documentation defects: the token search case describes the wrong UI shape, and the asset-lock case suggests a funding step may be optional even though the flow cannot advance without a UTXO at the generated address. The other two reported mismatches are not actionable as written because one is a harmless placeholder drift in an SPV-only field and the other cites a non-exhaustive sidebar list that already uses etc..
Reviewed commit: f063d8c
🔴 1 blocking | 🟡 1 suggestion(s)
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `docs/test-cases.md`:
- [BLOCKING] line 496: TC-TOK-002-01 verifies token-level fields, but the search screen returns contract rows
Step 6 tells testers to expect each result entry to show a token name and metadata, but the `Search Tokens` screen renders a contract-oriented table with `Contract ID`, `Contract Description`, and an action column. The empty state is also contract-oriented, and the next smoke case (`TC-TOK-003-01`) explicitly depends on recording a `Contract ID` from this results table. A tester following the current wording can fail the case even when the UI is behaving correctly because the documented assertions do not match the implemented screen.
- [SUGGESTION] line 337: TC-ALK-001-01 implies the funding transfer may be optional even though the flow cannot proceed without it
Step 8 says to send funds to the displayed address "if prompted", but the asset-lock screen starts in `WaitingOnFunds` and only advances after a UTXO is detected for the generated funding address. The flow does not consume the wallet's existing balance directly, so skipping the self-send leaves the test stuck on `Waiting for funds...`. The case should instruct the tester to perform the transfer, not present it as conditional.
| | **Test Case ID** | TC-TOK-002-01 | | ||
| | **Short Description** | Search for tokens by keyword | | ||
| | **Pre-Conditions** | DET running on testnet, SPV synced | | ||
| | **Test Steps** | 1. Click **"Tokens"** in the left sidebar.<br>2. Select the **"Search Tokens"** tab.<br>3. Enter the keyword `test` in the search field.<br>4. Execute the search.<br>5. Verify results appear, or a "no results" message is shown.<br>6. If results appear, verify each entry shows a token name and metadata. | |
There was a problem hiding this comment.
🔴 Blocking: TC-TOK-002-01 verifies token-level fields, but the search screen returns contract rows
Step 6 tells testers to expect each result entry to show a token name and metadata, but the Search Tokens screen renders a contract-oriented table with Contract ID, Contract Description, and an action column. The empty state is also contract-oriented, and the next smoke case (TC-TOK-003-01) explicitly depends on recording a Contract ID from this results table. A tester following the current wording can fail the case even when the UI is behaving correctly because the documented assertions do not match the implemented screen.
source: ['codex']
🤖 Fix this with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `docs/test-cases.md`:
- [BLOCKING] line 496: TC-TOK-002-01 verifies token-level fields, but the search screen returns contract rows
Step 6 tells testers to expect each result entry to show a token name and metadata, but the `Search Tokens` screen renders a contract-oriented table with `Contract ID`, `Contract Description`, and an action column. The empty state is also contract-oriented, and the next smoke case (`TC-TOK-003-01`) explicitly depends on recording a `Contract ID` from this results table. A tester following the current wording can fail the case even when the UI is behaving correctly because the documented assertions do not match the implemented screen.
| | **Test Case ID** | TC-ALK-001-01 | | ||
| | **Short Description** | Create an asset lock from the Identity Test wallet | | ||
| | **Pre-Conditions** | **Identity Test** wallet selected with >= 2 tDASH | | ||
| | **Test Steps** | 1. Click **"Wallets"** in the left sidebar.<br>2. Select the **Identity Test** wallet.<br>3. In the **"Asset Locks"** section, click **"Create Asset Lock"**.<br>4. Verify the breadcrumb/heading shows **Wallets > Create Asset Lock** / **"Create Asset Lock"**.<br>5. Under **"Select Asset Lock Purpose"**, click **"Registration"**.<br>6. In **"Select how much you would like to transfer?"**, enter `0.5` tDASH.<br>7. Review the generated funding address/QR code.<br>8. Send funds from the **Identity Test** wallet to the displayed address if prompted, then wait through **"Waiting for funds..."** and **"Waiting for Core Chain to produce proof of asset lock..."**.<br>9. Verify **"Asset Lock Created Successfully!"** and wait for the asset lock transaction/proof to confirm. | |
There was a problem hiding this comment.
🟡 Suggestion: TC-ALK-001-01 implies the funding transfer may be optional even though the flow cannot proceed without it
Step 8 says to send funds to the displayed address "if prompted", but the asset-lock screen starts in WaitingOnFunds and only advances after a UTXO is detected for the generated funding address. The flow does not consume the wallet's existing balance directly, so skipping the self-send leaves the test stuck on Waiting for funds.... The case should instruct the tester to perform the transfer, not present it as conditional.
source: ['codex']
🤖 Fix this with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `docs/test-cases.md`:
- [SUGGESTION] line 337: TC-ALK-001-01 implies the funding transfer may be optional even though the flow cannot proceed without it
Step 8 says to send funds to the displayed address "if prompted", but the asset-lock screen starts in `WaitingOnFunds` and only advances after a UTXO is detected for the generated funding address. The flow does not consume the wallet's existing balance directly, so skipping the self-send leaves the test stuck on `Waiting for funds...`. The case should instruct the tester to perform the transfer, not present it as conditional.
f063d8c to
126aa86
Compare
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
I verified the reported issues against the checked-out source and the dependency implementation that resolves DET's config directory. One reported blocker is real and still affects the executability of the token smoke flow; the macOS .env path report is a false positive because the docs match ProjectDirs and the stale path is only in .env.example.
Reviewed commit: 126aa86
🔴 1 blocking
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `docs/test-cases.md`:
- [BLOCKING] lines 496-499: TC-TOK-002-01 leaves the suite blocked in the documented success path when the search returns no rows
This case explicitly allows the empty-state outcome in step 5 and in the expected result, but the next case still depends on a `Contract ID` produced here. The implementation matches that behavior: `ContractSearchStatus::Complete` shows either `No tokens match your keyword.` when `results.is_empty()` or a table with `Contract ID`, `Contract Description`, and a `More Info` action when rows exist (`src/ui/tokens/tokens_screen/keyword_search.rs:115-140`, `178-240`). As written, TC-TOK-002-01 can finish successfully without producing `${TC_TOK_CONTRACT_ID}`, which means the suite immediately blocks on TC-TOK-003-01. The test case needs to make that dependency part of TC-TOK-002-01's executable outcome instead of treating the no-results branch as an equivalent success state.
| | **Test Steps** | 1. Click **"Tokens"** in the left sidebar.<br>2. Select the **"Search Tokens"** tab.<br>3. Enter the keyword `test` in the search field.<br>4. Execute the search.<br>5. Verify results appear, or **"No tokens match your keyword."** is shown.<br>6. If results appear, verify the contract-results table shows **"Contract ID"**, **"Contract Description"**, and **"Action"** columns with a **"More Info"** action; record one **Contract ID** as `${TC_TOK_CONTRACT_ID}` for TC-TOK-003-01. | | ||
| | **Test Data** | Keyword: `test` | | ||
| | **Expected Result** | Search executes without error. Results or empty-state message displayed. | | ||
| | **Post-Condition** | `${TC_TOK_CONTRACT_ID}` recorded from a result row, or TC-TOK-003-01 blocked if no rows are returned. | |
There was a problem hiding this comment.
🔴 Blocking: TC-TOK-002-01 leaves the suite blocked in the documented success path when the search returns no rows
This case explicitly allows the empty-state outcome in step 5 and in the expected result, but the next case still depends on a Contract ID produced here. The implementation matches that behavior: ContractSearchStatus::Complete shows either No tokens match your keyword. when results.is_empty() or a table with Contract ID, Contract Description, and a More Info action when rows exist (src/ui/tokens/tokens_screen/keyword_search.rs:115-140, 178-240). As written, TC-TOK-002-01 can finish successfully without producing ${TC_TOK_CONTRACT_ID}, which means the suite immediately blocks on TC-TOK-003-01. The test case needs to make that dependency part of TC-TOK-002-01's executable outcome instead of treating the no-results branch as an equivalent success state.
source: ['codex']
🤖 Fix this with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `docs/test-cases.md`:
- [BLOCKING] lines 496-499: TC-TOK-002-01 leaves the suite blocked in the documented success path when the search returns no rows
This case explicitly allows the empty-state outcome in step 5 and in the expected result, but the next case still depends on a `Contract ID` produced here. The implementation matches that behavior: `ContractSearchStatus::Complete` shows either `No tokens match your keyword.` when `results.is_empty()` or a table with `Contract ID`, `Contract Description`, and a `More Info` action when rows exist (`src/ui/tokens/tokens_screen/keyword_search.rs:115-140`, `178-240`). As written, TC-TOK-002-01 can finish successfully without producing `${TC_TOK_CONTRACT_ID}`, which means the suite immediately blocks on TC-TOK-003-01. The test case needs to make that dependency part of TC-TOK-002-01's executable outcome instead of treating the no-results branch as an equivalent success state.
126aa86 to
973a409
Compare
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
I verified the checked-out worktree at 973a4098272f022060cd507aa02a8ff2a1b175fa and confirmed one real reproducibility problem in the new smoke-test docs. The Session Setup instructions spend 4 DASH from the Bank wallet, but two later wallet tests still require the Bank wallet to remain at >= 10 tDASH, which contradicts the documented minimum starting balance and can produce false failures for a correctly prepared environment.
Reviewed commit: 973a409
🔴 1 blocking
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `docs/test-cases.md`:
- [BLOCKING] lines 199-225: Bank wallet balance checks are impossible after Session Setup when using the documented minimum funding
The docs tell testers to start with a Bank wallet balance of at least `10 tDASH` in `docs/test-prerequisites.md:159`, then Session Setup spends `1` DASH to `Send Test` and `3` DASH to `Identity Test` in `docs/test-cases.md:75-90`. After that outflow, `TC-WAL-004-01` and `TC-WAL-008-01` still require the Bank wallet to show `>= 10 tDASH`, which is no longer true for a tester who followed the documented minimum. That makes these cases fail even when the product behaves correctly, so the suite is not internally reproducible as written. The docs need to either raise the required starting Bank balance enough to cover setup spending and fees, or relax these later assertions to check for a remaining positive or sufficient balance instead of the original pre-setup minimum.
| | **Test Steps** | 1. Click **"Wallets"** in the left sidebar.<br>2. Click on the **Created Wallet** entry in the wallet list.<br>3. Verify **"Core balance:"** shows `0.0000 DASH`.<br>4. Click on the **Bank** entry in the wallet list.<br>5. Verify **"Core balance:"** shows >= 10 tDASH. | | ||
| | **Test Data** | N/A | | ||
| | **Expected Result** | Selecting a different wallet updates the displayed balance immediately. No restart required. | | ||
| | **Post-Condition** | **Bank** wallet is selected. | | ||
|
|
||
| ### TC-WAL-005-01: Rename a wallet | ||
|
|
||
| | Field | Value | | ||
| |-------|-------| | ||
| | **Use Case ID** | WAL-005 | | ||
| | **Test Case ID** | TC-WAL-005-01 | | ||
| | **Short Description** | Rename the Created Wallet | | ||
| | **Pre-Conditions** | `Created Wallet` exists | | ||
| | **Test Steps** | 1. Click **"Wallets"** in the left sidebar.<br>2. Locate **Created Wallet** in the wallet list.<br>3. Click the **"Rename"** button next to it.<br>4. A **"Rename Wallet"** dialog appears with a text field (hint: *"Enter wallet name"*).<br>5. Clear the existing name and type `Renamed Wallet`.<br>6. Confirm the rename.<br>7. Verify the wallet list now shows **Renamed Wallet** instead of **Created Wallet**.<br>8. Quit and re-launch DET.<br>9. Click **"Wallets"** — verify the wallet is still named **Renamed Wallet**. | | ||
| | **Test Data** | New name: `Renamed Wallet` | | ||
| | **Expected Result** | Wallet name updates immediately in the list. Name persists after app restart. | | ||
| | **Post-Condition** | Wallet is now named `Renamed Wallet`. | | ||
|
|
||
| ### TC-WAL-008-01: View wallet balances | ||
|
|
||
| | Field | Value | | ||
| |-------|-------| | ||
| | **Use Case ID** | WAL-008 | | ||
| | **Test Case ID** | TC-WAL-008-01 | | ||
| | **Short Description** | Verify balance display for the bank wallet | | ||
| | **Pre-Conditions** | **Bank** wallet imported and synced | | ||
| | **Test Steps** | 1. Click **"Wallets"** in the left sidebar.<br>2. Select the **Bank** wallet.<br>3. Verify **"Core balance:"** is displayed with a value >= 10 tDASH (format: `X.XXXX DASH`).<br>4. Verify **"Platform balance:"** is displayed (may show `0.0000 DASH` if no credits loaded). | |
There was a problem hiding this comment.
🔴 Blocking: Bank wallet balance checks are impossible after Session Setup when using the documented minimum funding
The docs tell testers to start with a Bank wallet balance of at least 10 tDASH in docs/test-prerequisites.md:159, then Session Setup spends 1 DASH to Send Test and 3 DASH to Identity Test in docs/test-cases.md:75-90. After that outflow, TC-WAL-004-01 and TC-WAL-008-01 still require the Bank wallet to show >= 10 tDASH, which is no longer true for a tester who followed the documented minimum. That makes these cases fail even when the product behaves correctly, so the suite is not internally reproducible as written. The docs need to either raise the required starting Bank balance enough to cover setup spending and fees, or relax these later assertions to check for a remaining positive or sufficient balance instead of the original pre-setup minimum.
source: ['codex-general']
🤖 Fix this with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `docs/test-cases.md`:
- [BLOCKING] lines 199-225: Bank wallet balance checks are impossible after Session Setup when using the documented minimum funding
The docs tell testers to start with a Bank wallet balance of at least `10 tDASH` in `docs/test-prerequisites.md:159`, then Session Setup spends `1` DASH to `Send Test` and `3` DASH to `Identity Test` in `docs/test-cases.md:75-90`. After that outflow, `TC-WAL-004-01` and `TC-WAL-008-01` still require the Bank wallet to show `>= 10 tDASH`, which is no longer true for a tester who followed the documented minimum. That makes these cases fail even when the product behaves correctly, so the suite is not internally reproducible as written. The docs need to either raise the required starting Bank balance enough to cover setup spending and fees, or relax these later assertions to check for a remaining positive or sufficient balance instead of the original pre-setup minimum.
973a409 to
f4bf5d9
Compare
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
I verified the only substantive agent finding against the checked-out documentation, and it is correct. The smoke-test flow funds two other wallets from Bank during Session Setup, but later wallet tests still assert that Bank remains at >= 10 tDASH while the prerequisites claim a 15 tDASH minimum is sufficient to preserve that threshold. Those two statements cannot both be true once the documented sends and fees are applied, so the suite is not reproducible as written.
Reviewed commit: f4bf5d9
🔴 1 blocking
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `docs/test-cases.md`:
- [BLOCKING] lines 199-225: Later Bank balance checks contradict the documented setup funding flow
`TC-WAL-004-01` step 5 and `TC-WAL-008-01` step 3 require the Bank wallet to show `>= 10 tDASH`, but Session Setup already spends from that wallet before either test runs. In `docs/test-cases.md:75-90`, the documented setup sends `1 DASH` to `${TC_SND_ADDRESS_0}` and `3 DASH` to `${TC_IDN_ADDRESS_0}`, plus transaction fees. At the same time, `docs/test-prerequisites.md:159-161` says a `15 tDASH` minimum covers the setup outflow and still leaves `>= 10 tDASH` for later checks. That arithmetic does not hold: a Bank wallet starting at the documented minimum will fall below the later assertion threshold after the two sends and fees, so a tester can follow the spec exactly and still fail these cases. The docs need one consistent rule: either raise the required starting balance enough to preserve the later threshold, or weaken the later assertions so they only require a valid positive remaining balance.
| | **Test Steps** | 1. Click **"Wallets"** in the left sidebar.<br>2. Click on the **Created Wallet** entry in the wallet list.<br>3. Verify **"Core balance:"** shows `0.0000 DASH`.<br>4. Click on the **Bank** entry in the wallet list.<br>5. Verify **"Core balance:"** shows >= 10 tDASH. | | ||
| | **Test Data** | N/A | | ||
| | **Expected Result** | Selecting a different wallet updates the displayed balance immediately. No restart required. | | ||
| | **Post-Condition** | **Bank** wallet is selected. | | ||
|
|
||
| ### TC-WAL-005-01: Rename a wallet | ||
|
|
||
| | Field | Value | | ||
| |-------|-------| | ||
| | **Use Case ID** | WAL-005 | | ||
| | **Test Case ID** | TC-WAL-005-01 | | ||
| | **Short Description** | Rename the Created Wallet | | ||
| | **Pre-Conditions** | `Created Wallet` exists | | ||
| | **Test Steps** | 1. Click **"Wallets"** in the left sidebar.<br>2. Locate **Created Wallet** in the wallet list.<br>3. Click the **"Rename"** button next to it.<br>4. A **"Rename Wallet"** dialog appears with a text field (hint: *"Enter wallet name"*).<br>5. Clear the existing name and type `Renamed Wallet`.<br>6. Confirm the rename.<br>7. Verify the wallet list now shows **Renamed Wallet** instead of **Created Wallet**.<br>8. Quit and re-launch DET.<br>9. Click **"Wallets"** — verify the wallet is still named **Renamed Wallet**. | | ||
| | **Test Data** | New name: `Renamed Wallet` | | ||
| | **Expected Result** | Wallet name updates immediately in the list. Name persists after app restart. | | ||
| | **Post-Condition** | Wallet is now named `Renamed Wallet`. | | ||
|
|
||
| ### TC-WAL-008-01: View wallet balances | ||
|
|
||
| | Field | Value | | ||
| |-------|-------| | ||
| | **Use Case ID** | WAL-008 | | ||
| | **Test Case ID** | TC-WAL-008-01 | | ||
| | **Short Description** | Verify balance display for the bank wallet | | ||
| | **Pre-Conditions** | **Bank** wallet imported and synced | | ||
| | **Test Steps** | 1. Click **"Wallets"** in the left sidebar.<br>2. Select the **Bank** wallet.<br>3. Verify **"Core balance:"** is displayed with a value >= 10 tDASH (format: `X.XXXX DASH`).<br>4. Verify **"Platform balance:"** is displayed (may show `0.0000 DASH` if no credits loaded). | |
There was a problem hiding this comment.
🔴 Blocking: Later Bank balance checks contradict the documented setup funding flow
TC-WAL-004-01 step 5 and TC-WAL-008-01 step 3 require the Bank wallet to show >= 10 tDASH, but Session Setup already spends from that wallet before either test runs. In docs/test-cases.md:75-90, the documented setup sends 1 DASH to ${TC_SND_ADDRESS_0} and 3 DASH to ${TC_IDN_ADDRESS_0}, plus transaction fees. At the same time, docs/test-prerequisites.md:159-161 says a 15 tDASH minimum covers the setup outflow and still leaves >= 10 tDASH for later checks. That arithmetic does not hold: a Bank wallet starting at the documented minimum will fall below the later assertion threshold after the two sends and fees, so a tester can follow the spec exactly and still fail these cases. The docs need one consistent rule: either raise the required starting balance enough to preserve the later threshold, or weaken the later assertions so they only require a valid positive remaining balance.
source: ['codex-general']
🤖 Fix this with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `docs/test-cases.md`:
- [BLOCKING] lines 199-225: Later Bank balance checks contradict the documented setup funding flow
`TC-WAL-004-01` step 5 and `TC-WAL-008-01` step 3 require the Bank wallet to show `>= 10 tDASH`, but Session Setup already spends from that wallet before either test runs. In `docs/test-cases.md:75-90`, the documented setup sends `1 DASH` to `${TC_SND_ADDRESS_0}` and `3 DASH` to `${TC_IDN_ADDRESS_0}`, plus transaction fees. At the same time, `docs/test-prerequisites.md:159-161` says a `15 tDASH` minimum covers the setup outflow and still leaves `>= 10 tDASH` for later checks. That arithmetic does not hold: a Bank wallet starting at the documented minimum will fall below the later assertion threshold after the two sends and fees, so a tester can follow the spec exactly and still fail these cases. The docs need one consistent rule: either raise the required starting balance enough to preserve the later threshold, or weaken the later assertions so they only require a valid positive remaining balance.
e72323f to
a5c8381
Compare
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
I verified the checked-out PR at a5c8381bd4e3f8d5fc8b428b823bce8b46d0bf1f against the referenced files and found one real documentation maintainability issue. The rest of the supplied agent output does not produce additional actionable findings for this revision.
Reviewed commit: a5c8381
🟡 1 suggestion(s)
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `docs/test-prerequisites.md`:
- [SUGGESTION] lines 112-122: The embedded Testnet `.env` block duplicates the bundled config and has already drifted from it
This section hard-codes the full `TESTNET_*` endpoint block even though the application already ships a bundled `.env.example` and copies it into the app data directory on first launch via `copy_env_file_if_not_exists()` in `src/app_dir.rs`. The duplication is not harmless: the checked-in `.env.example` uses the same DAPI host list today but a different `TESTNET_insight_api_url` (`https://testnet-insight.dash.org/insight-api` in `.env.example` versus `https://insight.testnet.networks.dash.org:3002/insight-api` here). Because this smoke-test doc is now maintaining its own endpoint inventory, it can silently go stale as testnet infrastructure changes. The doc should point readers to the bundled `.env.example` as the source of truth and only document the smoke-test-specific edits or constraints.
| # Testnet SPV mode — DAPI addresses plus placeholder Core RPC fields | ||
| TESTNET_dapi_addresses=https://34.214.48.68:1443,https://52.12.176.90:1443,https://52.34.144.50:1443,https://44.240.98.102:1443,https://54.201.32.131:1443,https://52.10.229.11:1443,https://52.13.132.146:1443,https://52.40.219.41:1443,https://54.149.33.167:1443,https://35.164.23.245:1443,https://52.33.28.47:1443,https://52.43.13.92:1443,https://52.89.154.48:1443,https://52.24.124.162:1443,https://35.85.21.179:1443,https://54.187.14.232:1443,https://54.68.235.201:1443,https://52.13.250.182:1443 | ||
| TESTNET_show_in_ui=true | ||
|
|
||
| # Core RPC fields are structurally required by NetworkConfig even in SPV mode, | ||
| # but are not actively used at runtime. Supply placeholder values. | ||
| TESTNET_core_host=127.0.0.1 | ||
| TESTNET_core_rpc_port=19998 | ||
| TESTNET_core_rpc_user=user | ||
| TESTNET_core_rpc_password=password | ||
| TESTNET_insight_api_url=https://insight.testnet.networks.dash.org:3002/insight-api |
There was a problem hiding this comment.
🟡 Suggestion: The embedded Testnet .env block duplicates the bundled config and has already drifted from it
This section hard-codes the full TESTNET_* endpoint block even though the application already ships a bundled .env.example and copies it into the app data directory on first launch via copy_env_file_if_not_exists() in src/app_dir.rs. The duplication is not harmless: the checked-in .env.example uses the same DAPI host list today but a different TESTNET_insight_api_url (https://testnet-insight.dash.org/insight-api in .env.example versus https://insight.testnet.networks.dash.org:3002/insight-api here). Because this smoke-test doc is now maintaining its own endpoint inventory, it can silently go stale as testnet infrastructure changes. The doc should point readers to the bundled .env.example as the source of truth and only document the smoke-test-specific edits or constraints.
source: ['codex-general']
🤖 Fix this with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `docs/test-prerequisites.md`:
- [SUGGESTION] lines 112-122: The embedded Testnet `.env` block duplicates the bundled config and has already drifted from it
This section hard-codes the full `TESTNET_*` endpoint block even though the application already ships a bundled `.env.example` and copies it into the app data directory on first launch via `copy_env_file_if_not_exists()` in `src/app_dir.rs`. The duplication is not harmless: the checked-in `.env.example` uses the same DAPI host list today but a different `TESTNET_insight_api_url` (`https://testnet-insight.dash.org/insight-api` in `.env.example` versus `https://insight.testnet.networks.dash.org:3002/insight-api` here). Because this smoke-test doc is now maintaining its own endpoint inventory, it can silently go stale as testnet infrastructure changes. The doc should point readers to the bundled `.env.example` as the source of truth and only document the smoke-test-specific edits or constraints.
a5c8381 to
6dada97
Compare
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
I verified the review scope on head 6dada9779dfd8688dc5e6024411d0b0ddc22e4c6 against the checked-out worktree and the touched documentation files. The previously reported agent sections contain no concrete findings, and my spot checks of the referenced UI labels, network/developer-mode behavior, wallet flows, and prerequisite constants all match the current source. I did not find an evidence-backed defect in this PR.
Reviewed commit: 6dada97
Issue
Closes #771
Summary
Adds formalized weekly smoke test specification covering all implemented user stories from
docs/user-stories.md, SPV-only on testnet.New files:
docs/test-cases.md— 32 test cases across all feature areas (WAL, SND, ALK, IDN, DPN, DPY, TOK, DOC, DEV, NET)docs/test-prerequisites.md— Environment setup,.envconfiguration for SPV testnet, bank wallet concept, cleanup proceduresTest case structure:
Each test case includes: Use Case ID, Test Case ID, Short Description, Pre-Conditions, Test Steps, Test Data, Expected Result, and Post-Condition.
Coverage:
Notes:
Summary by CodeRabbit