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

test: Added e2e for switch network #27967

Merged
merged 15 commits into from
Oct 24, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion privacy-snapshot.json
Original file line number Diff line number Diff line change
Expand Up @@ -62,5 +62,6 @@
"unresponsive-rpc.test",
"unresponsive-rpc.url",
"user-storage.api.cx.metamask.io",
"www.4byte.directory"
"www.4byte.directory",
"sepolia.infura.io"
]
19 changes: 19 additions & 0 deletions test/e2e/page-objects/pages/dialog/notification.ts
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice, the test looks good 🔥 Just added a couple of suggestions.

I would suggest some naming changes:

  • notification.ts seems quite a vague name -> this component represents the Network Confirmations modal for switching networks, so I would name something like network-switch-modal-confirmation or similar, so it's easier to know what it refers to (see pic)

Screenshot from 2024-10-23 10-58-38

  • dialog/... I think the folder name is again quite vague -> here both components refer to the network picker and modal, so we could have some folder name more explicit

What do you think?

Copy link
Contributor Author

@hjetpoluru hjetpoluru Oct 23, 2024

Choose a reason for hiding this comment

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

Thanks @seaona for the detailed and valuable feedback.

Name Change

  • I agree with you that notification.ts is generic and a more accurate name as you suggested, will be network-switch-modal-confirmation and will make the change.
  • Since Dialog is the component, anything related to it should come under this folder. I can't think of any other options. Could you suggest some alternatives? It would be helpful.

Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import { Driver } from '../../../webdriver/driver';

class Notification {
private driver: Driver;

private submitButton: string;

constructor(driver: Driver) {
this.driver = driver;
this.submitButton = '[data-testid="confirmation-submit-button"]';
}

async clickApproveButton(): Promise<void> {
console.log('Click Approve Button');
await this.driver.clickElement(this.submitButton);
Copy link
Contributor

Choose a reason for hiding this comment

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

to avoid race conditions:

Suggested change
await this.driver.clickElement(this.submitButton);
await this.driver.clickElementAndWaitToDisappear(
this.submitButton,
);

Copy link
Contributor Author

@hjetpoluru hjetpoluru Oct 23, 2024

Choose a reason for hiding this comment

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

I actually tried using clickElementAndWaitToDisappear but it was unable to click properly. Since this is a dialog and I used clickElement instead. I will try again and check locally, and I will keep you updated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok I have updated clickElementAndWaitToDisappear and it worked locally and pushed up the changes.

}
}

export default Notification;
59 changes: 59 additions & 0 deletions test/e2e/page-objects/pages/dialog/select-network.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
import { Driver } from '../../../webdriver/driver';

class SelectNetwork {
private driver: Driver;

private networkName: string | undefined;

private addNetworkButton: object;

private closeButton: string;

private toggleButton: string;

private searchInput: string;

constructor(driver: Driver) {
this.driver = driver;
this.addNetworkButton = {
tag: 'button',
text: 'Add a custom network',
};
this.closeButton = 'button[aria-label="Close"]';
this.toggleButton = '.toggle-button > div';
this.searchInput = '[data-testid="network-redesign-modal-search-input"]';
}

async clickNetworkName(networkName: string): Promise<void> {
console.log(`Click ${networkName}`);
this.networkName = `[data-testid="${networkName}"]`;
await this.driver.clickElement(this.networkName);
}

async addNewNetwork(): Promise<void> {
console.log('Click Add network');
await this.driver.clickElement(this.addNetworkButton);
}

async clickCloseButton(): Promise<void> {
console.log('Click Close Button');
await this.driver.clickElement(this.closeButton);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
await this.driver.clickElement(this.closeButton);
await this.driver.clickElementAndWaitToDisappear(
this.closeButton,
);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have updated this change.

}

async clickToggleButton(): Promise<void> {
console.log('Click Toggle Button');
await this.driver.clickElement(this.toggleButton);
}

async fillNetworkSearchInput(networkName: string): Promise<void> {
console.log(`Fill network search input with ${networkName}`);
await this.driver.fill(this.searchInput, networkName);
}

async clickAddButton(): Promise<void> {
console.log('Click Add Button');
await this.driver.clickElement('[data-testid="test-add-button"]');
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
await this.driver.clickElement('[data-testid="test-add-button"]');
await this.driver.clickElementAndWaitToDisappear(
'[data-testid="test-add-button"]',
);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have updated this change.

}
}

export default SelectNetwork;
16 changes: 16 additions & 0 deletions test/e2e/page-objects/pages/header-navbar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ class HeaderNavbar {

private readonly settingsButton = '[data-testid="global-menu-settings"]';

private readonly switchNetworkDropDown = '[data-testid="network-display"]';

constructor(driver: Driver) {
this.driver = driver;
}
Expand Down Expand Up @@ -63,6 +65,20 @@ class HeaderNavbar {
await this.driver.clickElement(this.settingsButton);
}

async clickSwitchNetworkDropDown(): Promise<void> {
console.log(`Click switch network menu`);
await this.driver.clickElement(this.switchNetworkDropDown);
}

async check_networkNameSwitchDropDown(networkName: string): Promise<boolean> {
console.log(`Validate the Switch network to ${networkName}`);
const switchNetworkName = await this.driver.findElements({
tag: 'span',
text: networkName,
});
return switchNetworkName.length === 1;
chloeYue marked this conversation as resolved.
Show resolved Hide resolved
}

/**
* Verifies that the displayed account label in header matches the expected label.
*
Expand Down
66 changes: 66 additions & 0 deletions test/e2e/tests/network/switch-network.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
import { Suite } from 'mocha';
import { Driver } from '../../webdriver/driver';
import { withFixtures, defaultGanacheOptions } from '../../helpers';
import FixtureBuilder from '../../fixture-builder';
import { Ganache } from '../../seeder/ganache';
import { loginWithBalanceValidation } from '../../page-objects/flows/login.flow';
import HomePage from '../../page-objects/pages/homepage';
import Notification from '../../page-objects/pages/dialog/notification';
import HeaderNavbar from '../../page-objects/pages/header-navbar';
import SelectNetwork from '../../page-objects/pages/dialog/select-network';

describe('Switch network - ', function (this: Suite) {
it('Ethereum Mainnet and Sepolia', async function () {
await withFixtures(
{
fixtures: new FixtureBuilder().build(),
ganacheOptions: defaultGanacheOptions,
title: this.test?.fullTitle(),
},
async ({
driver,
ganacheServer,
}: {
driver: Driver;
ganacheServer?: Ganache;
}) => {
await loginWithBalanceValidation(driver, ganacheServer);
const homePage = new HomePage(driver);
const headerNavbar = new HeaderNavbar(driver);
const selectNetwork = new SelectNetwork(driver);
const notification = new Notification(driver);

// Validate the default network is Localhost 8545
await headerNavbar.check_networkNameSwitchDropDown('Localhost 8545');

// Validate the switch network functionality to Ethereum Mainnet
await headerNavbar.clickSwitchNetworkDropDown();
await selectNetwork.clickNetworkName('Ethereum Mainnet');
await homePage.check_expectedBalanceIsDisplayed('25');
await headerNavbar.check_networkNameSwitchDropDown('Ethereum Mainnet');
// Validate the switch network functionality to test network Sepolia
await headerNavbar.clickSwitchNetworkDropDown();
await selectNetwork.clickToggleButton();
await selectNetwork.clickNetworkName('Sepolia');
await homePage.check_expectedBalanceIsDisplayed('25 Sepolia');
await headerNavbar.check_networkNameSwitchDropDown('Sepolia');

// Add Aribtrum network and perform the switch network functionality
Copy link
Contributor

@seaona seaona Oct 23, 2024

Choose a reason for hiding this comment

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

Typo: Aribtrum should be Arbitrum

The test is called: Switch network - Ethereum Mainnet and Sepolia, however here we are adding Arbitrum network and switching to it too.
I would suggest either split this into 2 different tests and add more precise test naming, or change the test naming so it represents the complete test

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, I will change this..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@seaona, I believe this test scenario is a simple one, so I have simplified the test name and provided comments. This should be sufficient. Please let me know if anything else needs to be updated

await headerNavbar.clickSwitchNetworkDropDown();
await selectNetwork.fillNetworkSearchInput('Arbitrum One');
await selectNetwork.clickAddButton();
await notification.clickApproveButton();
await headerNavbar.clickSwitchNetworkDropDown();
await selectNetwork.clickNetworkName('Arbitrum One');
await homePage.check_expectedBalanceIsDisplayed('25');
await headerNavbar.check_networkNameSwitchDropDown('Arbitrum One');

// Validate the switch network functionality back to Ethereum Mainnet
await headerNavbar.clickSwitchNetworkDropDown();
await selectNetwork.clickNetworkName('Ethereum Mainnet');
await homePage.check_expectedBalanceIsDisplayed('25');
await headerNavbar.check_networkNameSwitchDropDown('Ethereum Mainnet');
},
);
});
});