Tamdang/e2e/multi users connection #588
Conversation
|
Looks like LNA is a feature flag introduced in Chrome 138, requiring the user to grant permission when making a request to a "local network". Not sure via Playwright would look like, but have you considered
|
| - name: Install Playwright browsers | ||
| run: pnpm exec playwright install --with-deps | ||
|
|
||
| - name: Start Firebase emulator |
extension/tests/fixture.ts
Outdated
| throw new Error(`Invalid path ${extension}`); | ||
| } | ||
|
|
||
| async function createExtensionContext(): Promise<BrowserContext> { |
There was a problem hiding this comment.
Extract to tests/utils.ts?
extension/tests/utils/room.ts
Outdated
| await page.getByRole("button", { name: "Create" }).click(); | ||
| await page.getByRole("img", { name: "Copy room ID" }).click(); | ||
|
|
||
| const roomId = await page.evaluate(async () => { |
There was a problem hiding this comment.
I think can be just as simple as const id = await page.evaluate(navigator.clipboard.readText);?
extension/tests/utils/room.ts
Outdated
| if (!roomId) { | ||
| throw new Error("Failed to extract room ID after creating room. "); | ||
| } |
There was a problem hiding this comment.
Clipboard returns string so this check is probably redundant (unless typescript def is wrong)
extension/tests/fixture.ts
Outdated
| return serviceWorker.url().split("/")[2]; | ||
| } | ||
|
|
||
| export const test = base.extend<{ |
There was a problem hiding this comment.
We want this to be a "factory" to create default fixture (unauthenticated) right?
extension/tests/utils/auth.ts
Outdated
| import type { Page } from "@playwright/test"; | ||
|
|
||
| export async function signIn(page: Page, email: string): Promise<void> { | ||
| await page.goto("https://leetcode.com/problems/two-sum", { |
There was a problem hiding this comment.
Might be wrong but since twoRoomTest extends from default fixture, this would be redudnant? https://github.com/nickbar01234/codebuddy/pull/588/changes#diff-48e511c5cb8dc56e989baa2ae1c74d1104a3057420c888bac6a22ed4ff106ca4R57
extension/tests/fixture.ts
Outdated
| await use(room); | ||
| }, | ||
| user2: async ({ room }, use) => { | ||
| const user2Context = await createExtensionContext(); |
There was a problem hiding this comment.
A "factory" would make sense here
extension/tests/fixture.ts
Outdated
| room: async ({ user1 }, use) => { | ||
| const room = await createRoom(user1.page); | ||
| await use(room); | ||
| }, |
There was a problem hiding this comment.
Is there no way for user1 to both createRoom and expose room information for user2?
| @@ -0,0 +1,21 @@ | |||
| import { expect, twoUserRoomTest } from "../fixture"; | |||
There was a problem hiding this comment.
Probably just move this into content.spec.ts. I don't see a compelling reason to have them separate yet
| };`; | ||
|
|
||
| twoUserRoomTest("User1 can copy code from User2", async ({ user1, user2 }) => { | ||
| await user1.page.getByRole("tab", { name: /Code/i }).click(); |
|
|
||
| await use({ instantiate }); | ||
|
|
||
| await Promise.all(contexts.map((ctx) => ctx.close())); |
There was a problem hiding this comment.
For my understanding, why is contexts a list?
| import { test as base, BrowserContext, chromium, Page } from "@playwright/test"; | ||
| import { signIn } from "@tests/utils/auth"; | ||
| import { getExtensionId, getExtensionPath, setupPage } from "../utils/page"; | ||
| export interface UserPage { |
extension/tests/fixture/factory.ts
Outdated
| @@ -0,0 +1,48 @@ | |||
| import { test as base, BrowserContext, chromium, Page } from "@playwright/test"; | |||
| import { signIn } from "@tests/utils/auth"; | |||
| import { getExtensionId, getExtensionPath, setupPage } from "../utils/page"; | |||
extension/tests/fixture.ts
Outdated
| }, | ||
| }); | ||
|
|
||
| export const expect = test.expect; |
There was a problem hiding this comment.
We can just delete this file right?
Description
Screenshots
Test
Checklist
If you're making changes to the extension, please run through the following checklist to make sure that we don't have
any regressions. Note that we plan to add integration tests in the future!
Possible Downsides
Additional Documentations