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

E2e test fixtures #16061

Merged
merged 44 commits into from
Oct 28, 2022
Merged

E2e test fixtures #16061

merged 44 commits into from
Oct 28, 2022

Conversation

PeterYinusa
Copy link
Contributor

@PeterYinusa PeterYinusa commented Oct 3, 2022

Explanation

Fixes: #14340

Previously multiple e2e tests were coupled to a single state.json file.

fixtures

This PR decouples the state files, and provides an easy to use api to create fixture state, enabling more granular control of each individual tests state.

There are two initial state/fixture objects, default fixture and onboarding fixture.
The onboarding fixture provides the state of a new user pre onboarding
The default fixture provides the state of an existing user post onboarding

More Information

Screenshots/Screencaps

Before

After

Manual Testing Steps

Pre-Merge Checklist

  • PR template is filled out
  • IF this PR fixes a bug, a test that would have caught the bug has been added
  • PR is linked to the appropriate GitHub issue
  • PR has been added to the appropriate release Milestone

+ If there are functional changes:

  • Manual testing complete & passed
  • "Extension QA Board" label has been applied

@github-actions
Copy link
Contributor

github-actions bot commented Oct 3, 2022

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

metrics = await driver.collectMetrics();
});
await withFixtures(
{ fixtures: new FixtureBuilder().build() },
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Only the fixture has changed, the rest is just lint formatting

@@ -198,26 +198,6 @@ const getWindowHandles = async (driver, handlesCount) => {
return { extension, dapp, popup };
};

const connectDappWithExtensionPopup = async (driver) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We use the FixtureBuilder to add the required state for dapp connections

metrics = parsedLogs.map((pl) => JSON.parse(pl));
});
await withFixtures(
{ fixtures: new FixtureBuilder().build() },
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Only the fixture has changed, the rest is just lint formatting

await driver.delay(1000);
const logs = await driver.checkBrowserForLavamoatLogs();
await withFixtures(
{ fixtures: new FixtureBuilder().build() },
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Only the fixture has changed, the rest is just lint formatting

await driver.delay(1000);
const logs = await driver.checkBrowserForLavamoatLogs();
await withFixtures(
{ fixtures: new FixtureBuilder().build() },
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Only the fixture has changed, the rest is just lint formatting

@@ -34,8 +31,7 @@ describe('Dapp interactions', function () {
await driver.fill('#password', 'correct horse battery staple');
await driver.press('#password', driver.Key.ENTER);

// Connect to Dapp0
await connectDappWithExtensionPopup(driver, 0);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We do not actually need to connect to the dapp to click the add ethereum chain button

@@ -78,8 +76,7 @@ describe('Dapp interactions', function () {
await driver.fill('#password', 'correct horse battery staple');
await driver.press('#password', driver.Key.ENTER);

// Connect to Dapp0
await connectDappWithExtensionPopup(driver, 0);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can use the fixture to connect to the dapp

@@ -34,7 +39,7 @@ describe('Editing Confirm Transaction', function () {
'.currency-display-component__text',
);
const transactionAmount = transactionAmounts[0];
assert.equal(await transactionAmount.getText(), '2.2');
assert.equal(await transactionAmount.getText(), '1');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The value has changed as we are now reusing an existing transaction

@@ -112,7 +122,7 @@ describe('Editing Confirm Transaction', function () {
'.currency-display-component__text',
);
const transactionAmount = transactionAmounts[0];
assert.equal(await transactionAmount.getText(), '2.2');
assert.equal(await transactionAmount.getText(), '1');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The value has changed as we are now reusing an existing transaction

@@ -194,7 +209,7 @@ describe('Editing Confirm Transaction', function () {
await driver.press('#password', driver.Key.ENTER);

// open dapp and connect
await connectDappWithExtensionPopup(driver);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can use the fixture to connect to the dapp

@@ -315,7 +344,6 @@ describe('MetaMask Import UI', function () {
const importJsonFile = path.join(
__dirname,
'..',
'fixtures',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

moved this file out of the fixture folder as its not a fixture

@@ -39,26 +42,9 @@ describe('Test Snap bip-44', function () {
// connect the snap
await driver.clickElement('#connectBip44');

// switch to metamask extension and click connect
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Connect to the snap test dapp in the fixture

@@ -31,26 +34,9 @@ describe('Test Snap Confirm', function () {
await driver.fill('#snapId1', 'npm:@metamask/test-snap-confirm');
await driver.clickElement('#connectHello');

// switch to metamask extension and click connect
await driver.waitUntilXWindowHandles(2, 5000, 10000);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Connect to the snap test dapp in the fixture

@@ -32,25 +35,8 @@ describe('Test Snap Error', function () {
await driver.fill('#snapId2', 'npm:@metamask/test-snap-error');
await driver.clickElement('#connectError');

// switch to metamask extension and click connect
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Connect to the snap test dapp in the fixture

@@ -40,25 +43,9 @@ describe('Test Snap manageState', function () {
// connect the snap
await driver.clickElement('#connectManageState');

// switch to metamask extension and click connect
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Connect to the snap test dapp in the fixture

@@ -40,25 +43,9 @@ describe('Test Snap Notification', function () {
// connect the snap
await driver.clickElement('#connectNotification');

// switch to metamask extension and click connect
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Connect to the snap test dapp in the fixture

@metamaskbot
Copy link
Collaborator

Builds ready [be3c4d7]
Page Load Metrics (2506 ± 145 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint1021861242311
domContentLoaded191030292485303146
load191030542506302145
domInteractive191030292485303146

@PeterYinusa PeterYinusa marked this pull request as ready for review October 4, 2022 11:39
@PeterYinusa PeterYinusa requested a review from a team as a code owner October 4, 2022 11:39
@PeterYinusa PeterYinusa requested a review from NidhiKJha October 4, 2022 11:39
@PeterYinusa PeterYinusa added the needs-qa Label will automate into QA workspace label Oct 4, 2022
@metamaskbot
Copy link
Collaborator

Builds ready [3f7e590]
Page Load Metrics (2357 ± 116 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint93149117178
domContentLoaded167727242328247119
load174427472357242116
domInteractive167727242328247119

test/e2e/fixture-builder.js Outdated Show resolved Hide resolved
test/e2e/tests/from-import-ui.spec.js Outdated Show resolved Hide resolved
test/e2e/tests/from-import-ui.spec.js Outdated Show resolved Hide resolved
@PeterYinusa PeterYinusa marked this pull request as draft October 7, 2022 10:12
@PeterYinusa PeterYinusa mentioned this pull request Oct 7, 2022
53 tasks
let windowHandles = await driver.getAllWindowHandles();
const extensionPage = windowHandles[0];
await driver.switchToWindowWithTitle(
'MetaMask Notification',
windowHandles,
);
await driver.clickElement(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Connect to the snap test dapp in the fixture

@@ -18,7 +19,19 @@ describe('Failing contract interaction ', function () {
await withFixtures(
{
dapp: true,
fixtures: 'connected-state',
fixtures: new FixtureBuilder()
.withNetworkController({
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The send failing transaction is a type 2 transaction

@PeterYinusa PeterYinusa dismissed stale reviews from seaona, Gudahtt, and jpuri via ce88224 October 27, 2022 15:53
PeterYinusa added 2 commits October 27, 2022 17:38
const networkDisplay = await driver.findElement('.network-display');
const networkDisplay = await driver.findElement(
'[data-testid="network-display"]',
);
Copy link
Contributor Author

@PeterYinusa PeterYinusa Oct 27, 2022

Choose a reason for hiding this comment

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

Nothings changed here, this is the same element as the original test, just using a different selector

Copy link
Member

@Gudahtt Gudahtt left a comment

Choose a reason for hiding this comment

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

LGTM!

@metamaskbot
Copy link
Collaborator

Builds ready [2327778]
Page Load Metrics (2143 ± 99 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint962115215436209
domContentLoaded18062641212220096
load18062641214320699
domInteractive18062641212220096

@PeterYinusa PeterYinusa merged commit 0b4532e into develop Oct 28, 2022
@PeterYinusa PeterYinusa deleted the e2e-test-fixtures branch October 28, 2022 08:42
@github-actions github-actions bot locked and limited conversation to collaborators Oct 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
needs-qa Label will automate into QA workspace
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Clean up e2e fixture data
5 participants