Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
11 changes: 7 additions & 4 deletions apps/extension/tests/auth.setup.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import fs from 'node:fs';
import process from 'node:process';
import { chromium, expect, test as setup } from '@playwright/test';
import { TEST_TIMEOUTS } from '@bypass/shared/tests';
import wretch from 'wretch';
import QueryStringAddon from 'wretch/addons/queryString';
import { getFirebasePublicConfig } from '../../../packages/configs/firebase.config';
Expand All @@ -21,6 +22,8 @@ const identityApi = wretch('https://identitytoolkit.googleapis.com/v1')
.addon(QueryStringAddon)
.query({ key: firebaseConfig.apiKey });

setup.setTimeout(60_000);

const signInWithEmailAndPassword = async (): Promise<IAuthResponse> => {
return identityApi
.url('/accounts:signInWithPassword')
Expand Down Expand Up @@ -68,7 +71,7 @@ setup('authenticate and cache extension storage', async ({}, testInfo) => {

let [background] = browserContext.serviceWorkers();
background ||= await browserContext.waitForEvent('serviceworker', {
timeout: 20_000,
timeout: TEST_TIMEOUTS.AUTH,
});
const extensionId = background.url().split('/')[2];

Expand All @@ -87,12 +90,12 @@ setup('authenticate and cache extension storage', async ({}, testInfo) => {
await page.goto(extUrl, { waitUntil: 'domcontentloaded' });

const loginButton = page.getByRole('button', { name: 'Login' });
await loginButton.waitFor({ state: 'visible', timeout: 20_000 });
await loginButton.waitFor({ state: 'visible', timeout: TEST_TIMEOUTS.AUTH });
await loginButton.click();

const logoutButton = page.getByRole('button', { name: 'Logout' });
await expect(logoutButton).toBeVisible({ timeout: 30_000 });
await expect(logoutButton).toBeEnabled({ timeout: 30_000 });
await expect(logoutButton).toBeVisible({ timeout: TEST_TIMEOUTS.AUTH });
await expect(logoutButton).toBeEnabled({ timeout: TEST_TIMEOUTS.AUTH });

const chromeStorageData = await page.evaluate(async () =>
browser.storage.local.get(null)
Expand Down
6 changes: 5 additions & 1 deletion apps/extension/tests/fixtures/base-fixture.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
type Worker,
chromium,
} from '@playwright/test';
import { TEST_TIMEOUTS } from '@bypass/shared/tests';
import { CHROME_PROFILE_DIR, EXTENSION_STORAGE_PATH } from '../auth-constants';
import { getExtensionPath } from '../utils/extension-path';

Expand Down Expand Up @@ -131,7 +132,10 @@ export const authenticateAndNavigate = async (

// Step 5: Verify we're logged in (logout button should be visible)
const logoutButton = page.getByRole('button', { name: 'Logout' });
await logoutButton.waitFor({ state: 'visible', timeout: 10_000 });
await logoutButton.waitFor({
state: 'visible',
timeout: TEST_TIMEOUTS.AUTH,
});

// Step 6: Navigate to requested panel
if (panelName && panelName !== 'home') {
Expand Down
91 changes: 53 additions & 38 deletions apps/extension/tests/specs/bookmarks.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ import {
TEST_BOOKMARKS,
TEST_FOLDERS,
TEST_PERSONS,
TEST_TIMEOUTS,
clearSearchInput,
fillSearchInput,
openNewPageFromAction,
} from '@bypass/shared/tests';
import { test, expect } from '../fixtures/bookmark-fixture';
import { BookmarksPanel } from '../utils/bookmarks-panel';
Expand Down Expand Up @@ -58,7 +60,7 @@ test.describe.serial('Bookmarks Panel', () => {
await expect(bookmark).toBeVisible();

const title = (await bookmark.textContent()) ?? '';
expect(title).toBeTruthy();
expect(title).not.toBe('');
expect(title).toContain(TEST_BOOKMARKS.REACT_DOCS);

await bookmark.click();
Expand All @@ -79,7 +81,7 @@ test.describe.serial('Bookmarks Panel', () => {
const titleInput = dialog.getByTestId('bookmark-title-input');
await expect(titleInput).toBeVisible();
const currentTitle = await titleInput.inputValue();
expect(currentTitle).toBeTruthy();
expect(currentTitle).not.toBe('');

// Verify person select is present
const personLabel = dialog.getByText('Tagged Persons');
Expand Down Expand Up @@ -128,43 +130,60 @@ test.describe.serial('Bookmarks Panel', () => {
await panel.ensureAtRoot();
});

test('should open bookmarks by double-click and via context menu', async ({
test('should open bookmark by double-click', async ({
bookmarksPage,
context,
}) => {
const panel = new BookmarksPanel(bookmarksPage);

// Test 1: Open bookmark by double-clicking
const initialPages = context.pages().length;
const bookmarkRow = panel.getBookmarkElement(TEST_BOOKMARKS.REACT_DOCS);
await expect(bookmarkRow).toBeVisible();

const [newPage] = await Promise.all([
context.waitForEvent('page', { timeout: TEST_TIMEOUTS.PAGE_OPEN }),
bookmarkRow.dblclick(),
]);

expect(newPage).toBeTruthy();
expect(context.pages().length).toBeGreaterThan(initialPages);
const newPage = await openNewPageFromAction(context, async () => {
await bookmarkRow.dblclick();
});
await newPage.close();
});

// Test 2: Open bookmark via context menu
test('should open bookmark via context menu', async ({
bookmarksPage,
context,
}) => {
const panel = new BookmarksPanel(bookmarksPage);
await panel.ensureAtRoot();
const firstBookmark = panel.getBookmarkElement(TEST_BOOKMARKS.REACT_DOCS);
await expect(firstBookmark).toBeVisible();
await firstBookmark.click();

await firstBookmark.click({ button: 'right' });
const openOption = bookmarksPage.getByTestId('context-menu-item-open');
await expect(openOption).toBeVisible();
const openFromContextMenu = async () => {
await firstBookmark.click({ button: 'right' });
const openOption = bookmarksPage.getByTestId('context-menu-item-open');
await expect(openOption).toBeVisible();
await openOption.click();
};
Comment on lines +158 to +163

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

issue: openFromContextMenu re-executes all actions (right-click, expect, click) on each retry, but clicking the same element multiple times could cause unexpected behavior or test flakiness

Suggested change
const openFromContextMenu = async () => {
await firstBookmark.click({ button: 'right' });
const openOption = bookmarksPage.getByTestId('context-menu-item-open');
await expect(openOption).toBeVisible();
await openOption.click();
};
let contextMenuPage;
for (let attempt = 0; attempt < 2; attempt++) {
try {
await firstBookmark.click({ button: 'right' });
const openOption = bookmarksPage.getByTestId('context-menu-item-open');
await expect(openOption).toBeVisible();
contextMenuPage = await openNewPageFromAction(
context,
async () => await openOption.click(),
{ timeout: 5000 }
);
break;
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/extension/tests/specs/bookmarks.spec.ts
Line: 158-163

Comment:
issue: `openFromContextMenu` re-executes all actions (right-click, expect, click) on each retry, but clicking the same element multiple times could cause unexpected behavior or test flakiness

```suggestion
      let contextMenuPage;
      for (let attempt = 0; attempt < 2; attempt++) {
        try {
          await firstBookmark.click({ button: 'right' });
          const openOption = bookmarksPage.getByTestId('context-menu-item-open');
          await expect(openOption).toBeVisible();
          
          contextMenuPage = await openNewPageFromAction(
            context,
            async () => await openOption.click(),
            { timeout: 5000 }
          );
          break;
```

How can I resolve this? If you propose a fix, please make it concise.


let contextMenuPage;
for (let attempt = 0; attempt < 2; attempt++) {
try {
contextMenuPage = await openNewPageFromAction(
context,
openFromContextMenu,
{
timeout: 5000,
}
);
break;
} catch (error) {
if (attempt === 1) {
throw error;
}
}
}

if (!contextMenuPage) {
throw new Error('Expected context menu action to open a new page');
}

const [contextMenuPage] = await Promise.all([
context.waitForEvent('page', { timeout: TEST_TIMEOUTS.PAGE_OPEN }),
openOption.click(),
]);

expect(contextMenuPage).toBeTruthy();
expect(context.pages().length).toBeGreaterThan(initialPages);
await contextMenuPage.close();
});

Expand All @@ -175,8 +194,6 @@ test.describe.serial('Bookmarks Panel', () => {
await panel.selectBookmark(TEST_BOOKMARKS.REACT_DOCS);

await bookmarksPage.keyboard.press('Meta+x');
await bookmarksPage.waitForTimeout(TEST_TIMEOUTS.DEBOUNCE);

await bookmarksPage.keyboard.press('Meta+v');

await panel.verifyBookmarkExists(TEST_BOOKMARKS.REACT_DOCS);
Expand Down Expand Up @@ -243,11 +260,9 @@ test.describe.serial('Bookmarks Panel', () => {
const newUrl = 'https://www.google.com/';
await panel.editBookmarkUrl(TEST_BOOKMARKS.REACT_DOCS, newUrl);

const [newPage] = await Promise.all([
context.waitForEvent('page'),
panel.openBookmarkByDoubleClick(TEST_BOOKMARKS.REACT_DOCS),
]);
await newPage.waitForLoadState();
const newPage = await openNewPageFromAction(context, async () => {
await panel.openBookmarkByDoubleClick(TEST_BOOKMARKS.REACT_DOCS);
});
expect(newPage.url()).toContain('google.com');
await newPage.close();

Expand Down Expand Up @@ -312,28 +327,28 @@ test.describe.serial('Bookmarks Panel', () => {
await panel.openFolder(TEST_FOLDERS.MAIN);

// Test search by title
await panel.search('ButtonGroup');
await fillSearchInput(bookmarksPage, 'ButtonGroup');
const titleFilteredBookmark = panel.getBookmarkElement(
TEST_BOOKMARKS.GITHUB
);
await expect(titleFilteredBookmark).toBeVisible();
await panel.clearSearch();
await clearSearchInput(bookmarksPage);

// Test search by URL
await panel.search('material');
await fillSearchInput(bookmarksPage, 'material');
const urlFilteredBookmark = panel.getBookmarkElement(
TEST_BOOKMARKS.REACT_DOCS
);
await expect(urlFilteredBookmark).toBeVisible();
await panel.clearSearch();
await clearSearchInput(bookmarksPage);

// Test folders remain visible during search
await panel.ensureAtRoot();
const folder = panel.getFolderElement(TEST_FOLDERS.MAIN);
await expect(folder).toBeVisible();
await panel.search('nonexistentterm');
await fillSearchInput(bookmarksPage, 'nonexistentterm');
await expect(folder).toBeVisible();
await panel.clearSearch();
await clearSearchInput(bookmarksPage);
});

test('should move bookmark using cut from context menu and paste', async ({
Expand Down Expand Up @@ -386,7 +401,7 @@ test.describe.serial('Bookmarks Panel', () => {

const folderName = 'Persistence Test Folder';
const folderRow = panel.getFolderElement(folderName);
await expect(folderRow).toBeVisible({ timeout: TEST_TIMEOUTS.LONG_WAIT });
await expect(folderRow).toBeVisible();

await folderRow.click({ button: 'right' });

Expand Down
25 changes: 12 additions & 13 deletions apps/extension/tests/specs/last-visited-button.spec.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
import { TEST_TIMEOUTS } from '@bypass/shared/tests';
import { test, expect as homeExpect } from '../fixtures/home-popup-fixture';
import { waitForDebounce } from '../utils/test-utils';

/**
* LastVisitedButton E2E Tests
Expand All @@ -24,39 +22,40 @@ test.describe.serial('LastVisitedButton', () => {

// Click to set initial timestamp
await lastVisitedButton.click();
await homePage.waitForTimeout(TEST_TIMEOUTS.NAVIGATION);

// Move away then hover to trigger tooltip fresh
await homePage.mouse.move(0, 0);
await waitForDebounce(homePage);
await lastVisitedButton.hover();
await homePage.waitForTimeout(TEST_TIMEOUTS.PAGE_LOAD);

// Get the tooltip element and capture its text
const tooltip = homePage
.locator('[data-slot="tooltip-content"]')
.filter({ hasText: /,/ });
await homeExpect(tooltip).toBeVisible();
const initialTooltipText = await tooltip.textContent();
homeExpect(initialTooltipText).toBeTruthy();
homeExpect(initialTooltipText).not.toBeNull();
if (!initialTooltipText) {
throw new Error('Expected last visited tooltip text to be present');
}

// Move away from tooltip
await homePage.mouse.move(0, 0);

// Wait 1.1s to ensure timestamps differ (second-precision)
await homePage.waitForTimeout(1100);
// Wait until the next second tick to ensure timestamp precision changes.
const firstClickTime = Date.now();
await homeExpect
.poll(() => Date.now())
.toBeGreaterThanOrEqual(firstClickTime + 1000);

// Click again to update timestamp
await lastVisitedButton.click();
await homePage.waitForTimeout(TEST_TIMEOUTS.NAVIGATION);

// Move away then hover to trigger tooltip fresh
await homePage.mouse.move(0, 0);
await waitForDebounce(homePage);
await homeExpect(tooltip).toBeHidden();
await lastVisitedButton.hover();

// Wait for tooltip text to change from initial value (auto-retrying)
await homeExpect(tooltip).not.toHaveText(initialTooltipText!, {
timeout: 5000,
});
await homeExpect(tooltip).not.toHaveText(initialTooltipText);
});
});
48 changes: 32 additions & 16 deletions apps/extension/tests/specs/open-defaults-button.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { TEST_TIMEOUTS } from '@bypass/shared/tests';
import { test, expect as homeExpect } from '../fixtures/home-popup-fixture';

/**
Expand Down Expand Up @@ -36,29 +35,46 @@ test.describe.serial('Signed In', () => {
// Poll for the expected number of pages to be opened (initial + 2 new tabs)
await homeExpect
.poll(() => context.pages().length, {
timeout: TEST_TIMEOUTS.PAGE_OPEN,
message: 'Should open 2 new tabs',
})
.toBe(initialPageCount + 2);

const allPages = context.pages();
const newPages = allPages.filter((p) => p !== homePage);

// Get actual website URLs (filter out internal pages like about:blank)
const urls = newPages
.map((p) => p.url())
.filter((url) => !url.startsWith('about:'));

// Get base URLs without query params or hashes
const baseUrls = urls.map((url) => {
const parsed = new URL(url);
return `${parsed.protocol}//${parsed.host}${parsed.pathname}`;
});
// Verify default tabs were opened (Google and Mantine)
await homeExpect
.poll(
() => {
const currentPages = context
.pages()
.filter((page) => page !== homePage);
return currentPages
.map((page) => page.url())
.filter((url) => url.startsWith('http'))
.map((url) => {
const parsed = new URL(url);
return `${parsed.protocol}//${parsed.host}${parsed.pathname}`;
});
},
{
timeout: 15_000,
}
)
.toEqual(homeExpect.arrayContaining(['https://www.google.com/']));
Comment on lines +46 to +64

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The refactoring to use expect.poll for verifying opened pages is a significant improvement. It makes the test more resilient to timing issues by continuously checking the condition until it's met or the timeout is reached, rather than relying on fixed waits. This is particularly useful for asynchronous operations like opening new tabs.

Suggested change
await homeExpect
.poll(
() => {
const currentPages = context
.pages()
.filter((page) => page !== homePage);
return currentPages
.map((page) => page.url())
.filter((url) => url.startsWith('http'))
.map((url) => {
const parsed = new URL(url);
return `${parsed.protocol}//${parsed.host}${parsed.pathname}`;
});
},
{
timeout: 15_000,
}
)
.toEqual(homeExpect.arrayContaining(['https://www.google.com/']));
// Verify default tabs were opened (Google and Mantine)
await homeExpect
.poll(
() => {
const currentPages = context
.pages()
.filter((page) => page !== homePage);
return currentPages
.map((page) => page.url())
.filter((url) => url.startsWith('http'))
.map((url) => {
const parsed = new URL(url);
return `${parsed.protocol}//${parsed.host}${parsed.pathname}`;
});
},
{
timeout: 15_000,
}
)
.toEqual(homeExpect.arrayContaining(['https://www.google.com/']));


// Verify 2 default tabs were opened (Google and Mantine)
homeExpect(urls).toHaveLength(2);
homeExpect(baseUrls).toContain('https://www.google.com/');
homeExpect(baseUrls).toContain('https://mantine.dev/');
const baseUrls = context
.pages()
.filter((page) => page !== homePage)
.map((page) => page.url())
.filter(
(url) => url.startsWith('http') || url.startsWith('chrome-error://')
);
homeExpect(baseUrls).toEqual(
homeExpect.arrayContaining([
homeExpect.stringMatching(/mantine\.dev|^chrome-error:\/\//),
])
);
Comment on lines +66 to +77

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The updated assertion for baseUrls is more robust by using homeExpect.stringMatching with a regular expression. This allows for more flexible matching of URLs, accounting for potential variations like chrome-error:// pages, which can occur during navigation or extension loading. This makes the test less brittle.

    const baseUrls = context
      .pages()
      .filter((page) => page !== homePage)
      .map((page) => page.url())
      .filter(
        (url) => url.startsWith('http') || url.startsWith('chrome-error://')
      );
    homeExpect(baseUrls).toEqual(
      homeExpect.arrayContaining([
        homeExpect.stringMatching(/mantine\.dev|^chrome-error:\/\/),
      ])
    );


// Clean up: close new tabs
for (const newPage of newPages) {
Expand Down
Loading
Loading