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

Wallet 441 cw automation backup secret phrase #1075

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

xbeghers
Copy link
Collaborator

Make sure to fill in all the below sections.

Description

First pull request for Konrad's automation tests

// Provide a useful description of this PR, aimed at helping reviewers and contributors

Linked tickets

WALLET-XXX

Checklist

  • Make sure this PR title follows semantic release conventions: https://semantic-release.gitbook.io/semantic-release/#commit-message-format

  • If the PR adds any new text to the UI, make sure they are localized

  • Include a screenshot or recording if implementing significant UI or user flow change

  • When this PR affects architecture changes wait for review from Dmytro before merging

@ost-ptk
Copy link
Member

ost-ptk commented Oct 14, 2024

Please install eslint and prettier to the IDE that you use. There are many problems with code formatting.

} from '../../constants';
import { popup, popupExpect } from '../../fixtures';


Copy link
Member

Choose a reason for hiding this comment

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

please use the name-name format for file naming.

Comment on lines +48 to +53
const locators = createLocators(page);
const accountSwitcher = locators.accountSwitcher;
const firstAccount = locators.firstAccount;

// Perform actions and assertions
await accountSwitcher.click();
Copy link
Member

Choose a reason for hiding this comment

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

What does this code do?

Comment on lines +12 to +34
popup(
"should display a secret phrase after providing password and copy phrase",
async ({ popupPage, unlockVault, providePassword }) => {
await unlockVault();
await popupPage.getByTestId('menu-open-icon').click();
await popupPage.getByText('Back up your secret recovery phrase').click();
await providePassword();


await popupExpect(
popupPage.getByRole('heading', { name: 'Back up your secret recovery phrase' })
).toBeVisible();
await popupPage.getByText('Click to reveal secret recovery phrase').click();
await popupPage.getByText('Copy secret recovery phrase').click();



await popupExpect(
popupPage.getByText(
'Copied to clipboard'
)
).toBeVisible();
}
Copy link
Member

Choose a reason for hiding this comment

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

I just wanted to let you know that you didn't check if the secret phrase was displayed and if it was copied.
You test if the text changed after the user clicks on the copy text


);
popup(
'should display safety tips',
Copy link
Member

Choose a reason for hiding this comment

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

I don't think that we need this test. Here we test UI text and no functionality



popup(
"should timeout after providing wrong password 5 times",
Copy link
Member

Choose a reason for hiding this comment

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

we already have a test for locking the wallet for 5 minutes when the user types the wrong password 5 times
common/wallet.spec.ts

@ost-ptk
Copy link
Member

ost-ptk commented Oct 14, 2024

Please also update the PR name, description, provide the ticket's link, etc

image

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

Successfully merging this pull request may close these issues.

2 participants