fix(e2e): rewrite performFullLogin and extend walkOnboarding for 6-st…#303
Merged
graycyrus merged 1 commit intotinyhumansai:mainfrom Apr 3, 2026
Conversation
…ep flow (tinyhumansai#200) - Extract performFullLogin to shared-flows so all specs can reuse it; remove the duplicate local copy from auth-access-control.spec.ts. - Add completeMnemonicStep helper to handle the recovery-phrase step (checkbox tick + Finish Setup click) introduced in the 6-step flow. - Extend ONBOARDING_OVERLAY_TEXTS with recovery-phrase labels so onboardingOverlayLikelyVisible detects the new step correctly. - Increase walkOnboarding loop cap from 6 to 7 passes to cover the extra step; fall back to dumpAccessibilityTree on unexpected stalls. - Update auth-access-control spec to call the shared performFullLogin with the postLoginVerifier callback for consume/profile assertions. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughExtended E2E onboarding test helpers to support a newer mnemonic/recovery-phrase confirmation step in the onboarding flow. Added recovery-phrase-related detection strings and a new Changes
Sequence DiagramsequenceDiagram
participant Test as Test Code
participant Walk as walkOnboarding()
participant Step as completeMnemonicStep()
participant Browser as Browser API
participant DOM as Onboarding UI
Test->>Walk: Execute onboarding loop
loop Main Loop (7 iterations)
Walk->>Step: Call completeMnemonicStep()
Step->>DOM: Check for mnemonic label
alt Mnemonic Label Found
Step->>Browser: browser.execute() checkbox
Browser->>DOM: Interact with recovery phrase checkbox
Step->>DOM: Click "Finish Setup" or "Continue"
DOM->>Step: Button clicked, overlay advances
Step->>Walk: Return true (success)
Walk->>Walk: Skip remainder of iteration
else No Mnemonic Label
Step->>Walk: Return false (continue)
Walk->>DOM: Attempt primary button click
end
end
Walk->>Test: Onboarding complete
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
2 tasks
4 tasks
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
app/test/e2e/specs/auth-access-control.spec.ts (1)
107-113: Capture the UI state before failing the consume-call verifier.When this trips, the request log alone does not show whether the app reused a cached session or landed on the wrong screen. Dumping the accessibility tree here will make this new failure path much faster to diagnose.
🔍 Suggested addition
if (!consumeCall) { + const tree = await dumpAccessibilityTree(); console.log( '[AuthAccess] Missing consume call. Request log:', JSON.stringify(getRequestLog(), null, 2) ); + console.log('[AuthAccess] Missing consume call. Tree:\n', tree.slice(0, 4000)); throw new Error('Auth consume call missing in performFullLogin'); }As per coding guidelines
Assert both UI outcomes and backend/mock effects when relevant in E2E specs; add failure diagnostics (request logs, \dumpAccessibilityTree()`) for faster debugging`.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/test/e2e/specs/auth-access-control.spec.ts` around lines 107 - 113, When the consume-call verifier fails in performFullLogin (i.e., when consumeCall is falsy), augment the existing failure diagnostics by calling dumpAccessibilityTree() and including its output alongside the current JSON.stringify(getRequestLog(), ...) in the console error; ensure the message still throws the Error('Auth consume call missing in performFullLogin') after logging both the request log and the accessibility tree to aid debugging the UI state and session reuse scenarios.app/test/e2e/helpers/shared-flows.ts (1)
263-266: KeepONBOARDING_OVERLAY_TEXTSto screen-title strings.The recovery-phrase step is already covered by its headings, so adding
Finish Setuphere just broadensonboardingOverlayLikelyVisible()to a generic CTA label.walkOnboarding()is also called unconditionally fromapp/test/e2e/specs/local-model-runtime.spec.ts:65-71, so this makes false positives much easier if any non-onboarding screen later reuses the same button text.✂️ Suggested simplification
export const ONBOARDING_OVERLAY_TEXTS = [ 'Skip', 'Welcome', 'Run AI Models Locally', 'Screen & Accessibility', 'Enable Tools', 'Install Skills', 'Lastly, your Recovery Phrase', 'Your Recovery Phrase', 'Import Recovery Phrase', - 'Finish Setup', ] as const;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/test/e2e/helpers/shared-flows.ts` around lines 263 - 266, ONBOARDING_OVERLAY_TEXTS presently includes a generic CTA ('Finish Setup') which broadens onboardingOverlayLikelyVisible() and can cause false positives because walkOnboarding() is invoked unconditionally; remove the 'Finish Setup' entry from ONBOARDING_OVERLAY_TEXTS so the array only contains screen-title strings (e.g., 'Lastly, your Recovery Phrase', 'Your Recovery Phrase', 'Import Recovery Phrase') and ensure onboardingOverlayLikelyVisible() and walkOnboarding() rely on those title strings only.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/test/e2e/helpers/shared-flows.ts`:
- Around line 277-310: The helper completeMnemonicStep is advancing to the CTA
even when the checkbox interaction failed or wasn't actually checked and it uses
an unscoped 'input[type="checkbox"]' which may target the wrong element; update
completeMnemonicStep to scope the checkbox lookup to the MnemonicStep container
(use a unique selector from the MnemonicStep component) and ensure the
browser.execute() returns true (checkbox.checked) before proceeding — if
browser.execute() throws or returns false, log and return false (do not continue
to clickFirstMatch); only call clickFirstMatch(['Finish Setup','Continue'], ...)
when the checkbox is confirmed checked.
---
Nitpick comments:
In `@app/test/e2e/helpers/shared-flows.ts`:
- Around line 263-266: ONBOARDING_OVERLAY_TEXTS presently includes a generic CTA
('Finish Setup') which broadens onboardingOverlayLikelyVisible() and can cause
false positives because walkOnboarding() is invoked unconditionally; remove the
'Finish Setup' entry from ONBOARDING_OVERLAY_TEXTS so the array only contains
screen-title strings (e.g., 'Lastly, your Recovery Phrase', 'Your Recovery
Phrase', 'Import Recovery Phrase') and ensure onboardingOverlayLikelyVisible()
and walkOnboarding() rely on those title strings only.
In `@app/test/e2e/specs/auth-access-control.spec.ts`:
- Around line 107-113: When the consume-call verifier fails in performFullLogin
(i.e., when consumeCall is falsy), augment the existing failure diagnostics by
calling dumpAccessibilityTree() and including its output alongside the current
JSON.stringify(getRequestLog(), ...) in the console error; ensure the message
still throws the Error('Auth consume call missing in performFullLogin') after
logging both the request log and the accessibility tree to aid debugging the UI
state and session reuse scenarios.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f7b924fa-f3e9-43c6-8760-af70faedbaa1
📒 Files selected for processing (2)
app/test/e2e/helpers/shared-flows.tsapp/test/e2e/specs/auth-access-control.spec.ts
Comment on lines
+277
to
+310
| async function completeMnemonicStep(logPrefix: string): Promise<boolean> { | ||
| const mnemonicVisible = | ||
| (await textExists('Lastly, your Recovery Phrase')) || | ||
| (await textExists('Your Recovery Phrase')) || | ||
| (await textExists('Import Recovery Phrase')); | ||
|
|
||
| if (!mnemonicVisible) { | ||
| return false; | ||
| } | ||
|
|
||
| console.log(`${logPrefix} MnemonicStep visible`); | ||
|
|
||
| try { | ||
| const checked = await browser.execute(() => { | ||
| const checkbox = document.querySelector('input[type="checkbox"]') as HTMLInputElement | null; | ||
| if (!checkbox) return false; | ||
| if (!checkbox.checked) { | ||
| checkbox.click(); | ||
| } | ||
| return checkbox.checked; | ||
| }); | ||
| console.log(`${logPrefix} MnemonicStep checkbox checked=${checked}`); | ||
| } catch (err) { | ||
| console.log(`${logPrefix} MnemonicStep checkbox interaction failed:`, err); | ||
| } | ||
|
|
||
| const clicked = await clickFirstMatch(['Finish Setup', 'Continue'], 12_000); | ||
| if (clicked) { | ||
| console.log(`${logPrefix} MnemonicStep: clicked ${clicked}`); | ||
| await browser.pause(4_000); | ||
| return true; | ||
| } | ||
|
|
||
| return false; |
Contributor
There was a problem hiding this comment.
Don't fall through to Finish Setup when the mnemonic checkbox was never confirmed.
app/src/pages/onboarding/steps/MnemonicStep.tsx only advances once confirmed is true, but this helper just logs a failed/unsupported browser.execute() and then keeps trying the CTA anyway. Combined with the unscoped input[type="checkbox"] lookup, that makes the new step flaky and can strand the onboarding loop on a disabled/no-op button.
🛠️ Suggested fix
-async function completeMnemonicStep(logPrefix: string): Promise<boolean> {
+const completeMnemonicStep = async (logPrefix: string): Promise<boolean> => {
const mnemonicVisible =
(await textExists('Lastly, your Recovery Phrase')) ||
(await textExists('Your Recovery Phrase')) ||
(await textExists('Import Recovery Phrase'));
if (!mnemonicVisible) {
return false;
}
console.log(`${logPrefix} MnemonicStep visible`);
+ if (!supportsExecuteScript()) {
+ console.log(`${logPrefix} MnemonicStep checkbox cannot be toggled on this platform`);
+ return false;
+ }
+
try {
const checked = await browser.execute(() => {
- const checkbox = document.querySelector('input[type="checkbox"]') as HTMLInputElement | null;
+ const checkbox = Array.from(document.querySelectorAll('label'))
+ .find(label =>
+ label.textContent?.includes('I have saved my recovery phrase in a safe place')
+ )
+ ?.querySelector('input[type="checkbox"]') as HTMLInputElement | null;
if (!checkbox) return false;
if (!checkbox.checked) {
checkbox.click();
}
return checkbox.checked;
});
console.log(`${logPrefix} MnemonicStep checkbox checked=${checked}`);
+ if (!checked) {
+ return false;
+ }
} catch (err) {
console.log(`${logPrefix} MnemonicStep checkbox interaction failed:`, err);
+ return false;
}
const clicked = await clickFirstMatch(['Finish Setup', 'Continue'], 12_000);
if (clicked) {
console.log(`${logPrefix} MnemonicStep: clicked ${clicked}`);
await browser.pause(4_000);
return true;
}
return false;
-}
+};
- if (await completeMnemonicStep(logPrefix)) {
+ const completedMnemonicStep = await completeMnemonicStep(logPrefix);
+ if (completedMnemonicStep) {
continue;
}
+ if (
+ (await textExists('Lastly, your Recovery Phrase')) ||
+ (await textExists('Your Recovery Phrase')) ||
+ (await textExists('Import Recovery Phrase'))
+ ) {
+ const tree = await dumpAccessibilityTree();
+ console.log(
+ `${logPrefix} MnemonicStep visible but could not be completed. Tree:\n`,
+ tree.slice(0, 4000)
+ );
+ break;
+ }Also applies to: 340-342
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/test/e2e/helpers/shared-flows.ts` around lines 277 - 310, The helper
completeMnemonicStep is advancing to the CTA even when the checkbox interaction
failed or wasn't actually checked and it uses an unscoped
'input[type="checkbox"]' which may target the wrong element; update
completeMnemonicStep to scope the checkbox lookup to the MnemonicStep container
(use a unique selector from the MnemonicStep component) and ensure the
browser.execute() returns true (checkbox.checked) before proceeding — if
browser.execute() throws or returns false, log and return false (do not continue
to clickFirstMatch); only call clickFirstMatch(['Finish Setup','Continue'], ...)
when the checkbox is confirmed checked.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
performFullLogin()to use the current 6-step onboarding flow, replacing the legacy 5-step helper that referenced UI labels no longer in the app.performFullLogintoapp/test/e2e/helpers/shared-flows.tsso all specs can import it rather than duplicating login logic per file.walkOnboardingwithcompleteMnemonicStepto handle the recovery-phrase step (checkbox tick +Finish Setupclick) introduced in the 6-step flow.ONBOARDING_OVERLAY_TEXTSso overlay detection works for all onboarding screens.Problem
auth-access-control.spec.tshad a localperformFullLogin()that clicked through legacy step names (Enable Tools,Install Skillsas the final step viaContinue) that no longer exist as the last step in the app. The current onboarding ends with a recovery-phrase confirmation screen (Finish Setup). This caused every auth test case that relied on the helper — registration, revoked session — to stall at the mnemonic step and fail.The helper was also private to the spec, so any new spec needing full login had to duplicate the logic.
Solution
completeMnemonicStep(logPrefix): detects the recovery-phrase screen by checking for any of its three possible heading texts, ticks the acknowledgement checkbox viabrowser.execute, then clicksFinish SetuporContinue. Returnstrueif it handled the step sowalkOnboardingcancontinuewithout consuming a regularContinueclick.walkOnboardingloop cap raised from 6 → 7 to cover the 6-step flow plus one async-settle retry.completeMnemonicStepis attempted at the start of each pass and in the stall-recovery path.performFullLogin(token, logPrefix, postLoginVerifier?)inshared-flows.ts: drives deep link → bootstrap →walkOnboarding→ Home assertion. OptionalpostLoginVerifiercallback runs after Home is confirmed, letting callers assert side-effects (consume call, profile fetch) without those assertions being baked into the helper itself.performFullLoginremoved fromauth-access-control.spec.ts; consume/me assertions moved into the first test's verifier callback.Submission Checklist
walkOnboardingis exercised by every spec that callsperformFullLoginorcompleteOnboardingIfVisible.walkOnboarding,completeOnboardingIfVisible, andperformFullLogininshared-flows.ts.dumpAccessibilityTreefor diagnosis.Impact
auth-access-control.spec.tsare covered by the updated helper.performFullLoginis now importable by any future spec — no duplication needed.Merge order note
Related
Summary by CodeRabbit