Skip to content

fix(e2e): replace false-passing assertions and hard-coded waits in test suite #28486

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

Open
dididy wants to merge 3 commits into
base: main
Choose a base branch
from
+34 −36
Open

fix(e2e): replace false-passing assertions and hard-coded waits in test suite #28486

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
fff56b2
fix(e2e): replace always-passing assertions and hard-coded waits
dididy Mar 18, 2026
5b073db
fix(e2e): fix stale comment in stripe price test
dididy Mar 18, 2026
6d32905
fix(e2e): address cubic review feedback on assertion patterns
dididy Mar 18, 2026
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
4 changes: 2 additions & 2 deletions apps/web/playwright/booking-phone-autofill.e2e.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ test.describe("Phone Location Auto-fill Feature", () => {
// Verify custom phone fields start empty or country prefix (e.g., "+1")
const v1 = await page.locator('[name="phone-1"]').inputValue();
const v2 = await page.locator('[name="phone-2"]').inputValue();
expect(v1 === "" || /^\+\d{1,3}$/.test(v1)).toBeTruthy();
expect(v2 === "" || /^\+\d{1,3}$/.test(v2)).toBeTruthy();
expect(v1 === "" || /^\+\d{1,3}$/.test(v1)).toBe(true);
expect(v2 === "" || /^\+\d{1,3}$/.test(v2)).toBe(true);

// Select phone location and enter phone number
const phoneNumber = "+14155551234";
Expand Down
2 changes: 1 addition & 1 deletion apps/web/playwright/event-types.e2e.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ test.describe("Event Types tests", () => {
'[data-testid="event-types"] a[href^="/event-types/"] >> nth=0'
);
const href = await firstElement.getAttribute("href");
expect(href).toBeTruthy();
expect(href).not.toBeNull();
const [eventTypeId] = new URL(WEBAPP_URL + href).pathname.split("/").reverse();
const firstTitle = await page.locator(`[data-testid=event-type-title-${eventTypeId}]`).innerText();
const firstFullSlug = await page.locator(`[data-testid=event-type-slug-${eventTypeId}]`).innerText();
Expand Down
2 changes: 1 addition & 1 deletion apps/web/playwright/eventType/limit-tab.e2e.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ test.describe("Limits Tab - Event Type", () => {
await bookingPage.updateEventType();
const eventTypePage = await bookingPage.previewEventType();

await eventTypePage.waitForTimeout(10000);
await eventTypePage.getByTestId("time").first().waitFor({ state: "visible" });

const counter = await eventTypePage.getByTestId("time").count();
await bookingPage.checkTimeSlotsCount(eventTypePage, counter);
Expand Down
6 changes: 2 additions & 4 deletions apps/web/playwright/fixtures/apps.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,7 @@ export function createAppsFixture(page: Page) {
await page.waitForURL(`apps/installation/event-types?slug=${app}`);
}

// eslint-disable-next-line playwright/no-wait-for-timeout
await page.waitForTimeout(1000);
await page.locator(`[data-testid="select-event-type-${eventTypeIds[0]}"]`).waitFor({ state: "visible" });
for (const id of eventTypeIds) {
await page.click(`[data-testid="select-event-type-${id}"]`);
}
Expand Down Expand Up @@ -88,8 +87,7 @@ export function createAppsFixture(page: Page) {
await page.getByTestId("install-app-button").click();
await page.waitForURL(`apps/installation/event-types?slug=${app.slug}`);

// eslint-disable-next-line playwright/no-wait-for-timeout
await page.waitForTimeout(1000);
await page.locator(`[data-testid="select-event-type-${eventTypeIds[0]}"]`).waitFor({ state: "visible" });
for (const id of eventTypeIds) {
await page.click(`[data-testid="select-event-type-${id}"]`);
}
Expand Down
4 changes: 2 additions & 2 deletions apps/web/playwright/fixtures/workflows.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,8 +127,8 @@ export function createWorkflowPageFixture(page: Page) {
getWorkflowButton("delete-button"),
]);

expect(editButton.isDisabled()).toBeTruthy();
expect(deleteButton.isDisabled()).toBeTruthy();
await expect(editButton).toBeDisabled();
await expect(deleteButton).toBeDisabled();
};

const assertWorkflowWasTriggered = async (emails: Fixtures["emails"], emailsToBeReceived: string[]) => {
Expand Down
6 changes: 3 additions & 3 deletions apps/web/playwright/login.api.e2e.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ test.describe("Login with api request", () => {
const cookiesMap = new Map(contextCookies.map(({ name, value }) => [name, value]));

// The browser context will already contain all the cookies from the API response.
expect(cookiesMap.has("next-auth.csrf-token")).toBeTruthy();
expect(cookiesMap.has("next-auth.callback-url")).toBeTruthy();
expect(cookiesMap.has("next-auth.session-token")).toBeTruthy();
expect(cookiesMap.has("next-auth.csrf-token")).toBe(true);
expect(cookiesMap.has("next-auth.callback-url")).toBe(true);
expect(cookiesMap.has("next-auth.session-token")).toBe(true);
});
});
9 changes: 4 additions & 5 deletions apps/web/playwright/out-of-office.e2e.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ test.describe("Out of office", () => {
const eventTypeLink = page.locator('[data-testid="event-type-link"]').first();
await eventTypeLink.click();

await expect(page.getByTestId("away-emoji")).toBeTruthy();
await expect(page.getByTestId("away-emoji").first()).toBeVisible();
});

test("User can create Entry for past", async ({ page, users }) => {
Expand Down Expand Up @@ -338,7 +338,6 @@ test.describe("Out of office", () => {
});

test("User cannot create infinite or overlapping reverse redirect OOOs", async ({ page, users }) => {
const t = await localize("en");
const teamMatesObj = [{ name: "member-1" }, { name: "member-2" }];
const owner = await users.create(
{ name: "owner" },
Expand Down Expand Up @@ -367,7 +366,7 @@ test.describe("Out of office", () => {
await goToOOOPage(page);
await openOOODialog(page);
await selectDateAndCreateOOO(page, "2", "5", owner.id, 400);
await expect(page.locator(`text=${t("booking_redirect_infinite_not_allowed")}`)).toBeTruthy();
await expect(page.getByTestId("toast-error")).toBeVisible();
});
});

Expand Down Expand Up @@ -410,7 +409,7 @@ test.describe("Out of office", () => {
await page.getByTestId("ooofor_username_select").click();
await page.getByTestId(`select-option-${member2User?.id}`).click();
await selectDateAndCreateOOO(page, "1", "3", member1User?.id, 400, true);
expect(page.locator(`text=${t("booking_redirect_infinite_not_allowed")}`)).toBeTruthy();
await expect(page.getByTestId("toast-error")).toBeVisible();
await page.locator(`text=${t("cancel")}`).click();
});

Expand All @@ -428,7 +427,7 @@ test.describe("Out of office", () => {

await test.step("Delete OOO successfully", async () => {
await page.getByTestId(`ooo-delete-${member3User?.username}`).click();
expect(page.locator(`text=${t("success_deleted_entry_out_of_office")}`)).toBeTruthy();
await expect(page.getByTestId("toast-success")).toBeVisible();
});
});
test("Non-Admin has read-only access to team mate's OOO", async ({ page, users }) => {
Expand Down
29 changes: 15 additions & 14 deletions apps/web/playwright/payment-apps.e2e.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,15 +123,15 @@ test.describe("Payment app", () => {

Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Mar 18, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

P1: Custom agent: E2E Tests Best Practices

Rule 1 forbids page.locator('text=...'), but this change adds that brittle selector pattern across several payment-app assertions. Replace these page-level text locators with stable test ids (or another scoped stable locator) instead.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/playwright/payment-apps.e2e.ts, line 127:

<comment>Rule 1 forbids `page.locator('text=...')`, but this change adds that brittle selector pattern across several payment-app assertions. Replace these page-level text locators with stable test ids (or another scoped stable locator) instead.</comment>

<file context>
@@ -123,15 +123,15 @@ test.describe("Payment app", () => {
-    // expect 200 sats to be displayed in page
-    expect(await page.locator("text=350").first()).toBeTruthy();
+    // expect 350 USD to be displayed in page
+    await expect(page.locator("text=350").first()).toBeVisible();
 
     await selectFirstAvailableTimeSlotNextMonth(page);
</file context>
Fix with Cubic

Copy link
Author

@dididy dididy Mar 18, 2026

Choose a reason for hiding this comment

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

Two reasons these can't be replaced with data-testid selectors. AlbyPriceComponent, AppCard, and the paypal setup page have no data-testid attributes - that would need a separate PR on the production side. Also, some of these are checking the actual text on purpose: 200 sats and MX$150.00 exist to verify the price format is correct. A testid would only tell you the element is there, not what it says.

await page.goto(`${user.username}/${paymentEvent?.slug}`);

// expect 200 sats to be displayed in page
expect(await page.locator("text=350").first()).toBeTruthy();
// expect 350 USD to be displayed in page
await expect(page.locator("text=350").first()).toBeVisible();

await selectFirstAvailableTimeSlotNextMonth(page);
expect(await page.locator("text=350").first()).toBeTruthy();
await expect(page.locator("text=350").first()).toBeVisible();

// go to /event-types and check if the price is 200 sats
// go to /event-types and check if the price is 350 USD
await page.goto(`event-types/`);
expect(await page.locator("text=350").first()).toBeTruthy();
await expect(page.locator("text=350").first()).toBeVisible();
});

test("Should be able to edit paypal price, currency", async ({ page, users }) => {
Expand Down Expand Up @@ -170,15 +170,15 @@ test.describe("Payment app", () => {
await page.goto(`${user.username}/${paymentEvent?.slug}`);

// expect 150 to be displayed in page
expect(await page.locator("text=MX$150.00").first()).toBeTruthy();
await expect(page.locator("text=MX$150.00").first()).toBeVisible();

await selectFirstAvailableTimeSlotNextMonth(page);
// expect 150 to be displayed in page
expect(await page.locator("text=MX$150.00").first()).toBeTruthy();
await expect(page.locator("text=MX$150.00").first()).toBeVisible();

// go to /event-types and check if the price is 150
await page.goto(`event-types/`);
expect(await page.locator("text=MX$150.00").first()).toBeTruthy();
await expect(page.locator("text=MX$150.00").first()).toBeVisible();
});

test("Should display App is not setup already for alby", async ({ page, users }) => {
Expand All @@ -203,12 +203,12 @@ test.describe("Payment app", () => {
await page.locator("#event-type-form").getByRole("switch").click();

// expect text "This app has not been setup yet" to be displayed
expect(await page.locator("text=This app has not been setup yet").first()).toBeTruthy();
await expect(page.locator("text=This app has not been setup yet").first()).toBeVisible();

await page.getByRole("button", { name: "Setup" }).click();

// Expect "Connect with Alby" to be displayed
expect(await page.locator("text=Connect with Alby").first()).toBeTruthy();
await expect(page).toHaveURL(/\/apps\/alby\/setup/);
} finally {
await cleanupAlbyApp();
}
Expand All @@ -233,12 +233,12 @@ test.describe("Payment app", () => {
await page.locator("#event-type-form").getByRole("switch").click();

// expect text "This app has not been setup yet" to be displayed
expect(await page.locator("text=This app has not been setup yet").first()).toBeTruthy();
await expect(page.locator("text=This app has not been setup yet").first()).toBeVisible();

await page.getByRole("button", { name: "Setup" }).click();

// Expect "Getting started with Paypal APP" to be displayed
expect(await page.locator("text=Getting started with Paypal APP").first()).toBeTruthy();
await expect(page.locator("text=Getting started with Paypal APP").first()).toBeVisible();
});

/**
Expand Down Expand Up @@ -267,7 +267,7 @@ test.describe("Payment app", () => {

await page.locator("#event-type-form").getByRole("switch").click();
// make sure Tracking ID is displayed
expect(await page.locator("text=Tracking ID").first()).toBeTruthy();
await expect(page.locator("text=Tracking ID").first()).toBeVisible();
await page.getByLabel("Tracking ID").click();
await page.getByLabel("Tracking ID").fill("demo");
await page.getByTestId("update-eventtype").click();
Expand Down Expand Up @@ -313,7 +313,8 @@ test.describe("Payment app", () => {
await goToAppsTab(page, paymentEvent?.id);

await page.locator("[data-testid='paypal-app-switch']").click();
await page.locator("[data-testid='stripe-app-switch']").isDisabled();
// After enabling paypal, the paypal switch should be checked
await expect(page.locator("[data-testid='paypal-app-switch']")).toBeChecked();
});

test("when more than one payment app is installed the price should be updated when changing settings", async ({
Expand Down
8 changes: 4 additions & 4 deletions apps/web/playwright/signup.e2e.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -467,7 +467,7 @@ test.describe("Email Signup Flow Test", async () => {

await page.goto(`/settings/teams/${subTeam.id}/members`);
await page.waitForLoadState("domcontentloaded");
await page.waitForTimeout(500);
await expect(page.getByTestId("new-member-button")).toBeVisible();
await page.getByTestId("new-member-button").click();
const inviteLink = await getInviteLink(page);

Expand Down Expand Up @@ -546,9 +546,9 @@ test.describe("Email Signup Flow Test", async () => {
},
});

expect(createdUser).toBeTruthy();
expect(createdUser).not.toBeNull();
const membership = createdUser?.teams.find((m) => m.teamId === emailToken.teamId);
expect(membership).toBeTruthy();
expect(membership).not.toBeUndefined();
expect(membership?.accepted).toBe(true);

await prisma.user.delete({ where: { id: createdUser!.id } });
Expand Down Expand Up @@ -601,7 +601,7 @@ async function expectUserToBeAMemberOfTeam({
}) {
await page.goto(`/settings/teams/${teamId}/members`);
await page.waitForLoadState("domcontentloaded");
await page.waitForTimeout(1000);
await expect(page.locator(`[data-testid="member-${username}"]`)).toBeVisible();
expect(
(
await page
Expand Down
Loading