Skip to content

Creates tests for new network page feature WIP #33

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

burtonemily
Copy link
Contributor

Network pages

@burtonemily burtonemily requested a review from Bilb May 20, 2025 05:07
@@ -23,5 +23,6 @@ export default defineConfig({
reportSlowTests: null,
fullyParallel: true, // otherwise, tests in the same file are not run in parallel
globalSetup: './global.setup', // clean leftovers of previous test runs on start, runs only once
snapshotPathTemplate: screenshotFolder + '/{testName}/{arg}-{platform}{ext}',
// eslint-disable-next-line prefer-template
snapshotPathTemplate: screenshotFolder + '/{testName}/{arg}-{platform}{text}',
Copy link
Collaborator

Choose a reason for hiding this comment

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

why not make this screenshotFolder part of a templated string (with backtick?) I guess ts complains about the testName and others being there, but without the $ sign?

@@ -1,3 +1,4 @@
// eslint-disable-next-line import/no-cycle
Copy link
Collaborator

Choose a reason for hiding this comment

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

this will need to be addressed, eventually

Comment on lines +10 to +14
export async function recoverFromSeed(
window: Page,
recoveryPhrase: string,
createDisplayName?: boolean,
) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

is this createDisplayname used anywhere currently? I feel like recoverFromSeed should fail, if the displayName isn't found, and not blindly offer the option to use "Alice" as one. Why was this needed?

@@ -43,7 +43,7 @@ export async function waitForTestIdWithText(
const found = await window.waitForSelector(builtSelector, {
timeout: maxWait,
});
// console.info('found selector', builtSelector);
console.info('found selector', builtSelector);
Copy link
Collaborator

Choose a reason for hiding this comment

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

do we want this commited?

'open-url-confirm-button',
);
const href = await el.getAttribute('href');
console.log('href', href);
Copy link
Collaborator

Choose a reason for hiding this comment

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

do we want to log this?

Comment on lines +42 to +45
// TODO Add string in once crowdin has been updated
// const expectedText = englishStrippedStr('updated')
// .withArgs({ relative_time: '0m' })
// .toString();
Copy link
Collaborator

Choose a reason for hiding this comment

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

have those been added yet?

Comment on lines +12 to +17
/* Waiting for test id to be fixed
// Links to test are:
- Learn more Network link
- Learn more staking link
- How to create wallet link
*/
Copy link
Collaborator

Choose a reason for hiding this comment

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

where are we about those test ids?

@burtonemily burtonemily changed the title Creates tests for new network page feature Creates tests for new network page feature WIP May 27, 2025
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