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.

Sign up for GitHub

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

Jump to bottom

refactor(tests/e2e): simplify openPopup helper #706

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
+14 −32
Open

refactor(tests/e2e): simplify openPopup helper #706

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
ad9afb9
refactor(tests/e2e): simplify `openPopup` helper
sidvishnoi Nov 11, 2024
9460b83
Merge branch 'main' into test-popup-url
sidvishnoi Nov 13, 2024
43cb273
Merge branch 'main' into test-popup-url
sidvishnoi Nov 13, 2024
8358b8f
chore(deps): update react monorepo (#699)
renovate[bot] Nov 13, 2024
56e5ac2
fix(scripts/build): use correct asset-copy paths on Windows (#712)
DarianM Nov 14, 2024
89aca7c
fix persistent context not loading extension correctly
sidvishnoi Nov 15, 2024
3029907
Merge branch 'main' into test-popup-url
sidvishnoi Nov 15, 2024
e60e377
make userDataDir more unique?
sidvishnoi Nov 15, 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
10 changes: 2 additions & 8 deletions tests/e2e/fixtures/base.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { test as base, type BrowserContext, type Page } from '@playwright/test';
import {
getBackground,
getExtensionId,
loadContext,
BrowserIntl,
type Background,
Expand Down Expand Up @@ -51,13 +50,8 @@ export const test = base.extend<{ page: Page }, BaseScopeWorker>({
],

popup: [
async ({ background, persistentContext, browserName, channel }, use) => {
const extensionId = getExtensionId(browserName, background);
const popup = await openPopup(
persistentContext,
{ browserName, channel },
extensionId,
);
async ({ background, persistentContext }, use) => {
const popup = await openPopup(persistentContext, background);

await use(popup);
await popup.close();
Expand Down
10 changes: 9 additions & 1 deletion tests/e2e/fixtures/helpers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import net from 'node:net';
import path from 'node:path';
import { readFileSync } from 'node:fs';
import { readFile } from 'node:fs/promises';
import { tmpdir } from 'node:os';
import { format } from 'date-fns';
import {
chromium,
firefox,
Expand Down Expand Up @@ -142,10 +144,16 @@ export async function loadContext(
{ browserName, channel }: BrowserInfo,
workerInfo: WorkerInfo,
) {
const userDataDir = path.join(
tmpdir(),
'wm-extension-playwright',
`${browserName}-${channel}`,
format(new Date(), 'yyyyMMdd-HHmmss'),
);
const pathToExtension = getPathToExtension(browserName);
let context: BrowserContext | undefined;
if (browserName === 'chromium') {
context = await chromium.launchPersistentContext('', {
context = await chromium.launchPersistentContext(userDataDir, {
headless: true,
channel,
args: [
Expand Down
26 changes: 3 additions & 23 deletions tests/e2e/pages/popup.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,15 @@
import type { BrowserContext } from '@playwright/test';
import type { BrowserIntl, BrowserInfo } from '../fixtures/helpers';
import type { BrowserIntl, Background } from '../fixtures/helpers';

export type Popup = Awaited<ReturnType<typeof openPopup>>;

export { connectWallet } from '../helpers/testWallet';

export async function openPopup(
context: BrowserContext,
{ browserName, channel }: BrowserInfo,
extensionId: string,
background: Background,
) {
const url = getPopupUrl({ browserName, channel }, extensionId);
const url = await background.evaluate(() => chrome.action.getPopup({}));
Copy link
Member

Choose a reason for hiding this comment

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

Might it be worth to use browser from polyfill instead of chrome?

Copy link
Member Author

Choose a reason for hiding this comment

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

That'll require importing polyfill in each evaluate context.

Copy link
Member Author

@sidvishnoi sidvishnoi Nov 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Will check if Playwright lets us define such polyfills (like globally). But chrome is supposed to work in Edge/Chrome and Firefox.

Copy link
Member Author

Choose a reason for hiding this comment

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

In most cases, something like const browser = globalThis.browser || globalThis.chrome should be enough (we don't need full polyfill).

Copy link
Member Author

Choose a reason for hiding this comment

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

I couldn't find a way for now, but something like exposeBinding would be the way. Or we can make browser a global in background.js

Something of a general problem, I think I will check that as a followup.

const page = await context.newPage();
const popupPromise = page.waitForEvent('popup');
await page.evaluate(() => {
Expand All @@ -28,25 +27,6 @@ export async function openPopup(
return popup;
}

function getPopupUrl(
{ browserName, channel }: BrowserInfo,
extensionId: string,
) {
let url: string;
if (browserName === 'chromium') {
if (channel === 'edge') {
url = `extension://${extensionId}/popup/index.html`;
} else {
url = `chrome-extension://${extensionId}/popup/index.html`;
}
} else if (browserName === 'firefox') {
url = `moz-extension://${extensionId}/popup/index.html`;
} else {
throw new Error('Unsupported browser: ' + browserName);
}
return url;
}

export async function disconnectWallet(popup: Popup) {
await popup.locator(`[href="/settings"]`).click();
await popup.locator('button').getByText('Disconnect').click();
Expand Down