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: new switchToWindowWithTitle w/ Extension communication #25362

Merged
merged 25 commits into from
Jul 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
a55651a
test: new switchToWindowWithTitle w/ Extension communication
HowardBraham Jun 17, 2024
5838ca3
Otherwise it will "leak" event emitters
HowardBraham Jun 21, 2024
11fb79a
fix typo
HowardBraham Jun 21, 2024
ae10ca7
David Murdoch review
HowardBraham Jun 26, 2024
65c40e8
Merge branch 'develop' into e2e/switchToWindowWithTitle
HowardBraham Jun 26, 2024
5deb0c4
fixed merge problems
HowardBraham Jun 26, 2024
5ea1bc5
Figured out the flake in "MetaMask onboarding @no-mmi doesn't make an…
HowardBraham Jun 26, 2024
72b3222
Merge branch 'develop' into e2e/switchToWindowWithTitle
HowardBraham Jul 10, 2024
41c9eaf
Merge branch 'develop' into e2e/switchToWindowWithTitle
HowardBraham Jul 15, 2024
c9dc9f7
Merge branch 'develop' into e2e/switchToWindowWithTitle
HowardBraham Jul 15, 2024
e290160
lint fix
HowardBraham Jul 15, 2024
299ce1a
disableServerMochaToBackground flag
HowardBraham Jul 15, 2024
fbeb1a4
davidmurdoch: "Not all URLs will end with a slash..."
HowardBraham Jul 19, 2024
abaa36a
davidmurdoch: "Why is it safe to assume this is our window?"
HowardBraham Jul 19, 2024
c745464
Do not start SocketBackgroundToMocha unless `navigator.webdriver === …
HowardBraham Jul 19, 2024
0f4e1d1
davidmurdoch: "Wonder if we should guard against multiple connection …
HowardBraham Jul 19, 2024
0232641
davidmurdoch: "It'd be nice to require a specific format and command …
HowardBraham Jul 19, 2024
99754f2
Better error messages when window not found
HowardBraham Jul 19, 2024
7b675af
Merge branch 'develop' into e2e/switchToWindowWithTitle
HowardBraham Jul 19, 2024
3779eee
fix the MMI build and lint fixes
HowardBraham Jul 19, 2024
c611652
fix error reporting for 'test snap update rejected metric'
HowardBraham Jul 20, 2024
d6adad6
fixed problems with test-snap-metrics.spec.js
HowardBraham Jul 22, 2024
09533c6
Merge branch 'develop' into e2e/switchToWindowWithTitle
HowardBraham Jul 22, 2024
8b9b1a4
Merge branch 'develop' into e2e/switchToWindowWithTitle
HowardBraham Jul 23, 2024
bef19fe
Merge branch 'develop' into e2e/switchToWindowWithTitle
HowardBraham Jul 23, 2024
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
9 changes: 9 additions & 0 deletions app/scripts/background.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import { checkForLastErrorAndLog } from '../../shared/modules/browser-runtime.ut
import { isManifestV3 } from '../../shared/modules/mv3.utils';
import { maskObject } from '../../shared/modules/object.utils';
import { FIXTURE_STATE_METADATA_VERSION } from '../../test/e2e/default-fixture';
import { getSocketBackgroundToMocha } from '../../test/e2e/background-socket/socket-background-to-mocha';
import {
OffscreenCommunicationTarget,
OffscreenCommunicationEvents,
Expand Down Expand Up @@ -401,6 +402,14 @@ async function initialize() {

let isFirstMetaMaskControllerSetup;

// We only want to start this if we are running a test build, not for the release build.
// `navigator.webdriver` is true if Selenium, Puppeteer, or Playwright are running.
// In MV3, the Service Worker sees `navigator.webdriver` as `undefined`, so this will trigger from
// an Offscreen Document message instead. Because it's a singleton class, it's safe to start multiple times.
if (process.env.IN_TEST && window.navigator?.webdriver) {
getSocketBackgroundToMocha();
}

if (isManifestV3) {
// Save the timestamp immediately and then every `SAVE_TIMESTAMP_INTERVAL`
// miliseconds. This keeps the service worker alive.
Expand Down
7 changes: 7 additions & 0 deletions app/scripts/offscreen.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { captureException } from '@sentry/browser';
import { OffscreenCommunicationTarget } from '../../shared/constants/offscreen-communication';
import { getSocketBackgroundToMocha } from '../../test/e2e/background-socket/socket-background-to-mocha';

/**
* Creates an offscreen document that can be used to load additional scripts
Expand All @@ -25,6 +26,12 @@ export async function createOffscreen() {
offscreenDocumentLoadedListener,
);
resolve();

// If the Offscreen Document sees `navigator.webdriver === true` and we are in a test environment,
// start the SocketBackgroundToMocha.
if (process.env.IN_TEST && msg.webdriverPresent) {
getSocketBackgroundToMocha();
}
}
};
chrome.runtime.onMessage.addListener(offscreenDocumentLoadedListener);
Expand Down
2 changes: 2 additions & 0 deletions development/build/manifest.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ function createManifestTasks({
...manifest.permissions,
'webRequestBlocking',
'http://localhost/*',
'tabs', // test builds need tabs permission for switchToWindowWithTitle
];
});

Expand All @@ -80,6 +81,7 @@ function createManifestTasks({
...manifest.permissions,
'webRequestBlocking',
'http://localhost/*',
'tabs', // test builds need tabs permission for switchToWindowWithTitle
];
});

Expand Down
4 changes: 4 additions & 0 deletions offscreen/scripts/offscreen.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,4 +42,8 @@ if (process.env.IN_TEST) {
chrome.runtime.sendMessage({
target: OffscreenCommunicationTarget.extensionMain,
isBooted: true,

// This message is being sent from the Offscreen Document to the Service Worker.
// The Service Worker has no way to query `navigator.webdriver`, so we send it here.
webdriverPresent: navigator.webdriver === true,
});
5 changes: 5 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -492,6 +492,7 @@
"@tsconfig/node20": "^20.1.2",
"@types/babelify": "^7.3.7",
"@types/browserify": "^12.0.37",
"@types/chrome": "^0.0.268",
"@types/currency-formatter": "^1.5.1",
"@types/fs-extra": "^9.0.13",
"@types/gulp": "^4.0.9",
Expand Down Expand Up @@ -520,6 +521,7 @@
"@types/w3c-web-hid": "^1.0.3",
"@types/watchify": "^3.11.1",
"@types/webextension-polyfill": "^0.10.4",
"@types/ws": "^8.5.10",
"@types/yargs": "^17.0.32",
"@typescript-eslint/eslint-plugin": "^7.10.0",
"@typescript-eslint/parser": "^7.10.0",
Expand Down Expand Up @@ -638,6 +640,7 @@
"watchify": "^4.0.0",
"webextension-polyfill": "^0.8.0",
"webpack": "^5.91.0",
"ws": "^8.17.1",
"yaml": "^2.4.1",
"yargs": "^17.7.2"
},
Expand Down Expand Up @@ -705,6 +708,8 @@
"@trezor/connect-web>@trezor/connect>@trezor/utxo-lib>tiny-secp256k1": false,
"@storybook/test-runner>@swc/core": false,
"@lavamoat/lavadome-react>@lavamoat/preinstall-always-fail": false,
"ws>bufferutil": false,
"ws>utf-8-validate": false,
"tsx>esbuild": false,
"@metamask/eth-trezor-keyring>@trezor/connect-web>@trezor/connect>@trezor/protobuf>protobufjs": false,
"firebase>@firebase/firestore>@grpc/proto-loader>protobufjs": false,
Expand Down
17 changes: 2 additions & 15 deletions test/e2e/accounts/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import {
validateContractDetails,
multipleGanacheOptions,
regularDelayMs,
openDapp,
} from '../helpers';
import { Driver } from '../webdriver/driver';
import { TEST_SNAPS_SIMPLE_KEYRING_WEBSITE_URL } from '../constants';
Expand Down Expand Up @@ -185,20 +184,8 @@ async function switchToAccount2(driver: Driver) {
}

export async function connectAccountToTestDapp(driver: Driver) {
try {
// Do an unusually fast switchToWindowWithTitle, just 1 second
await driver.switchToWindowWithTitle(
WINDOW_TITLES.TestDApp,
null,
1000,
1000,
);
} catch {
await driver.switchToWindowWithTitle(
WINDOW_TITLES.ExtensionInFullScreenView,
);
await openDapp(driver);
}
await switchToOrOpenDapp(driver);

await driver.clickElement('#connectButton');

await driver.delay(regularDelayMs);
Expand Down
3 changes: 0 additions & 3 deletions test/e2e/accounts/snap-account-signatures.spec.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { Suite } from 'mocha';
import {
openDapp,
tempToggleSettingRedesignedConfirmations,
withFixtures,
} from '../helpers';
Expand Down Expand Up @@ -33,8 +32,6 @@ describe('Snap Account Signatures', function (this: Suite) {

await tempToggleSettingRedesignedConfirmations(driver);

await openDapp(driver);

// Run all 6 signature types
const locatorIDs = [
'#ethSign',
Expand Down
130 changes: 130 additions & 0 deletions test/e2e/background-socket/server-mocha-to-background.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,130 @@
import events from 'events';
import { WebSocketServer } from 'ws';
import {
MessageType,
ServerMochaEventEmitterType,
WindowProperties,
} from './types';

/**
* This singleton class runs on the Mocha/Selenium test.
* It's used to communicate from the Mocha/Selenium test to the Extension background script (service worker in MV3).
*/
class ServerMochaToBackground {
private server: WebSocketServer;

private ws: WebSocket | null = null;

private eventEmitter;

constructor() {
this.server = new WebSocketServer({ port: 8111 });

console.debug('ServerMochaToBackground created');

this.server.on('connection', (ws: WebSocket) => {
// Check for existing connection and close it
if (this.ws) {
console.error(
'ServerMochaToBackground got a second client connection, closing the first one',
);
this.ws.close();
Comment on lines +27 to +31
Copy link

Choose a reason for hiding this comment

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

Logic: Potential issue with handling multiple WebSocket connections. Consider adding logic to manage multiple connections more gracefully.

}

this.ws = ws;
Copy link
Contributor

Choose a reason for hiding this comment

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

Wonder if we should guard against multiple connection attempts 🤔. Maybe not, just a thought.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm so that would happen if you accidentally ran two E2E tests at the same time? Yeah we should check what happens.

Copy link
Contributor

@davidmurdoch davidmurdoch Jun 27, 2024

Choose a reason for hiding this comment

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

Or devs might just try to do some weird things with this API

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Second connection disconnects the first, and also stopped the Extension from connecting if it isn't being controlled by Selenium


console.debug('ServerMochaToBackground got a client connection');

ws.onmessage = (ev: MessageEvent) => {
let message: MessageType;

try {
message = JSON.parse(ev.data);
} catch (e) {
throw new Error(
`Error in JSON sent to ServerMochaToBackground: ${
(e as Error).message
}`,
);
}

this.receivedMessage(message);
};

ws.onclose = () => {
this.ws = null;
console.debug('ServerMochaToBackground disconnected from client');
};
});

this.eventEmitter = new events.EventEmitter<ServerMochaEventEmitterType>();
}

// This function is never explicitly called, but in the future it could be
stop() {
this.ws?.close();

this.server.close();

console.debug('ServerMochaToBackground stopped');
}

// Send a message to the Extension background script (service worker in MV3)
send(message: MessageType) {
if (!this.ws) {
throw new Error('No client connected to ServerMochaToBackground');
}

this.ws.send(JSON.stringify(message));
}

// Handle messages received from the Extension background script (service worker in MV3)
private receivedMessage(message: MessageType) {
if (message.command === 'openTabs' && message.tabs) {
this.eventEmitter.emit('openTabs', message.tabs);
} else if (message.command === 'notFound') {
throw new Error(
`No window found by background script with ${message.property}: ${message.value}`,
);
Comment on lines +86 to +88
Copy link

Choose a reason for hiding this comment

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

Logic: Throwing an error here might cause the entire test suite to fail. Consider handling this more gracefully or providing more context.

}
}

// This is not used in the current code, but could be used in the future
queryTabs(tabTitle: string) {
this.send({ command: 'queryTabs', title: tabTitle });
}

// Sends the message to the Extension, and waits for a response
async waitUntilWindowWithProperty(property: WindowProperties, value: string) {
this.send({ command: 'waitUntilWindowWithProperty', property, value });

const tabs = await this.waitForResponse();
// console.debug('ServerMochaToBackground got the response', tabs);

// The return value here is less useful than we had hoped, because the tabs
// are not in the same order as driver.getAllWindowHandles()
return tabs;
}

// This is a way to wait for an event async, without timeouts or polling
async waitForResponse() {
return new Promise((resolve) => {
this.eventEmitter.once('openTabs', resolve);
});
}
}

// Singleton setup below
let _serverMochaToBackground: ServerMochaToBackground;

export function getServerMochaToBackground() {
if (!_serverMochaToBackground) {
startServerMochaToBackground();
}

return _serverMochaToBackground;
}

function startServerMochaToBackground() {
_serverMochaToBackground = new ServerMochaToBackground();
}
Loading
Loading