Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multichain API E2E Test: wallet_createSession called with an existing active session #29672

Open
wants to merge 3 commits into
base: jl/caip-multichain-migrate-core
Choose a base branch
from

Conversation

ffmcgee725
Copy link
Member

@ffmcgee725 ffmcgee725 commented Jan 13, 2025

-- test: e2e test for wallet_createSession called with an existing active session;

Description

Open in GitHub Codespaces

Related issues

Fixes:

Manual testing steps

  1. Go to this page...

Screenshots/Recordings

Before

After

Pre-merge author checklist

Pre-merge reviewer checklist

  • I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
  • I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

Comment on lines +575 to +577
...optionalScopes,
},
isMultichainOrigin: false,
isMultichainOrigin,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To match description of the desired e2e test

When a dapp has an existing active session with scopes A, B & C and accounts W & X
And the dapp calls wallet_createSession with scopes C, D, E and accounts Y & Z
And the user confirms in the wallet (without editing permissions)
The old session permissions should be entirely overwritten by those requested in the new createSession request

I need to add these optional params for using multichain dapp, and more optional scopes

Even so, I thought for the sake of not bloating test code to keep new sessions to 2 new scopes instead of 3, and 1 new account instead of 2. Let me know if this should be religiously done for 3 new scopes and 2 new accounts.

Comment on lines +444 to +450
/**
* Then we make sure to deselect the existing session scopes, and create session with new scopes
*/
OLD_SCOPES.forEach(
async (scope) =>
await driver.clickElement(`input[name="${scope}"]`),
);
Copy link
Member Author

@ffmcgee725 ffmcgee725 Jan 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because of how multichain test dapp is working with previously existing sessions, I decided to document the flow of this specific test mainly for this part where we explain why we need to click checkboxes for these old existing scopes (as they come pre-selected in the UI).

Thinking of providing clarity and context for the next person touching this if needed.
You guys think we should keep these or remove the comments ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants