fix(e2e): replace false-passing assertions and hard-coded waits in test suite by voidmatcha · Pull Request #28486 · calcom/cal.diy

voidmatcha · 2026-03-18T13:28:20Z

What does this PR do?

Fixes silent test failures caused by always-passing assertions and arbitrary waitForTimeout() calls in the Playwright E2E test suite.

This is a scoped subset of the changes from #28466, split to stay within the 10-file / 500-line PR guidelines. Files with outstanding review feedback have been excluded and will be addressed in follow-up PRs.

Problem 1: `toBeTruthy()` on Locator objects (always passes)

page.locator() and page.getByTestId() return a Locator object, which is always truthy regardless of whether the element exists in the DOM.

Before:

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

After:

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

Affected files: fixtures/apps.ts, payment-apps.e2e.ts

Problem 2: `isDisabled()` result not asserted (no-op)

isDisabled() returns a Promise<boolean>. Without asserting the result, the call is a complete no-op.

Before:

expect(await editButton.isDisabled()).toBe(true);  // non-retrying snapshot

After:

await expect(editButton).toBeDisabled();  // Playwright auto-retry
await expect(deleteButton).toBeDisabled();

Affected files: fixtures/workflows.ts

Problem 3: `toBeTruthy()` on booleans and nullable values

Before:

expect(v1 === "" || /^\+\d{1,3}$/.test(v1)).toBeTruthy();
expect(cookiesMap.has("next-auth.csrf-token")).toBeTruthy();
expect(href).toBeTruthy();

After:

expect(v1 === "" || /^\+\d{1,3}$/.test(v1)).toBe(true);
expect(cookiesMap.has("next-auth.csrf-token")).toBe(true);
expect(href).not.toBeNull();

Affected files: booking-phone-autofill.e2e.ts, login.api.e2e.ts, event-types.e2e.ts, signup.e2e.ts

Problem 4: `waitForTimeout()` — brittle arbitrary waits

File	Before	After
`eventType/limit-tab.e2e.ts`	`waitForTimeout(10000)`	`getByTestId("time").first().waitFor({ state: "visible" })`
`fixtures/apps.ts`	`waitForTimeout(1000)` ×2	element visibility waits
`signup.e2e.ts`	`waitForTimeout(500)`, `waitForTimeout(1000)`	element / member visibility waits

Problem 5: Strict mode violation and toast assertion in `out-of-office.e2e.ts`

getByTestId("away-emoji") matched 6 elements → added .first()
Infinite redirect error is displayed as a toast, not inline text → getByTestId("toast-error")

Problem 6: Alby setup page navigation in `payment-apps.e2e.ts`

Setup page returns 500 without API keys configured, making "Connect with Alby" text unverifiable. Navigation to the setup page via waitForURL() is sufficient verification.

Also replaced no-op isDisabled() on stripe switch (pre-existing UI bug where switch never becomes disabled) with toBeChecked() on the paypal switch to verify the actual enabled state.

Mandatory Tasks (DO NOT REMOVE)

I have self-reviewed the code (A decent size PR without self-review might be rejected).
I have updated the developer docs in /docs if this PR makes changes that would require a documentation change. N/A — test-only changes.
I confirm automated tests are in place that prove my fix is effective or that my feature works.

How should this be tested?

PLAYWRIGHT_HEADLESS=1 yarn e2e \
  apps/web/playwright/booking-phone-autofill.e2e.ts \
  apps/web/playwright/event-types.e2e.ts \
  apps/web/playwright/eventType/limit-tab.e2e.ts \
  apps/web/playwright/fixtures/apps.ts \
  apps/web/playwright/fixtures/workflows.ts \
  apps/web/playwright/login.api.e2e.ts \
  apps/web/playwright/out-of-office.e2e.ts \
  apps/web/playwright/payment-apps.e2e.ts \
  apps/web/playwright/signup.e2e.ts

Checklist

My code follows the style guidelines of this project
I have checked that my changes generate no new warnings
These changes are pure test quality improvements — no production code modified

cubic-dev-ai

5 issues found across 9 files

Prompt for AI agents (unresolved issues)


Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="apps/web/playwright/fixtures/workflows.ts">

<violation number="1" location="apps/web/playwright/fixtures/workflows.ts:130">
P2: Disabled-state checks use non-retrying snapshot assertions instead of Playwright’s auto-retrying locator assertions, which can cause flaky E2E failures.</violation>
</file>

<file name="apps/web/playwright/out-of-office.e2e.ts">

<violation number="1" location="apps/web/playwright/out-of-office.e2e.ts:203">
P2: `getByTestId("away-emoji")` can resolve to multiple elements (one per away day), and Playwright strict mode throws when assertions target a non-unique locator. This can make the test flaky/fail even when the UI is correct.</violation>

<violation number="2" location="apps/web/playwright/out-of-office.e2e.ts:370">
P1: Custom agent: **E2E Tests Best Practices**

Rule 1 violation: these updated assertions still rely on `page.locator('text=...')` instead of resilient `getByTestId`-based selectors.</violation>
</file>

<file name="apps/web/playwright/payment-apps.e2e.ts">

<violation number="1" location="apps/web/playwright/payment-apps.e2e.ts:127">
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.</violation>

<violation number="2" location="apps/web/playwright/payment-apps.e2e.ts:210">
P1: Custom agent: **E2E Tests Best Practices**

Use `expect(page).toHaveURL()` here instead of `waitForURL()`. Rule 1 requires a URL assertion so this test fails if the setup page immediately redirects somewhere unexpected.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

cubic-dev-ai

2 issues found across 1 file (changes from recent commits).

Prompt for AI agents (unresolved issues)


Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="apps/web/playwright/payment-apps.e2e.ts">

<violation number="1">
P2: Custom agent: **E2E Tests Best Practices**

Avoid page-level text locators in E2E tests per E2E Tests Best Practices; use a stable selector (e.g., data-testid) or verify navigation with expect(page).toHaveURL instead.</violation>

<violation number="2">
P2: The new assertion only checks that a Locator object exists, not that the Alby setup page content appears. Because locators are always truthy, this test can pass even if navigation fails or the text never appears.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

romitg2

makes sense to me, let's run ci

- Replace toBeTruthy() on Locator objects with toBeVisible() (always-passing) - Replace toBeTruthy() on boolean expressions with toBe(true) for clarity - Replace hardcoded waitForTimeout() with element-based waits (waitFor/toBeVisible) - Fix strict mode violation: getByTestId("away-emoji").first() - Fix error assertion: infinite redirect error shown as toast → getByTestId("toast-error") - Fix isDisabled() no-op → await expect().toBeDisabled() in workflows fixture - Fix alby setup: add waitForURL(), remove unverifiable "Connect with Alby" assertion - Fix stripe/paypal: replace no-op isDisabled() with toBeChecked() on paypal switch Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

- workflows.ts: replace non-retrying isDisabled() snapshot with toBeDisabled() - out-of-office.e2e.ts: add .first() for away-emoji strict mode, use toast-error testid, remove unused const t - payment-apps.e2e.ts: replace always-passing toBeTruthy() with toHaveURL() assertion

voidmatcha · 2026-03-22T22:28:59Z

Sorry about the noise — CI failed on the previous run, so I rebased and force-pushed. Could you re-trigger CI?

The failures were in the API v2 roles/permissions tests (run link) — organizations-roles-permissions.controller.e2e-spec.ts returning 403 instead of 201. This PR only changes Playwright specs under apps/web/playwright/, so it shouldn't be related.

romitg2

lgtm, thanks @dididy

…st suite (calcom#28486) * fix(e2e): replace always-passing assertions and hard-coded waits - Replace toBeTruthy() on Locator objects with toBeVisible() (always-passing) - Replace toBeTruthy() on boolean expressions with toBe(true) for clarity - Replace hardcoded waitForTimeout() with element-based waits (waitFor/toBeVisible) - Fix strict mode violation: getByTestId("away-emoji").first() - Fix error assertion: infinite redirect error shown as toast → getByTestId("toast-error") - Fix isDisabled() no-op → await expect().toBeDisabled() in workflows fixture - Fix alby setup: add waitForURL(), remove unverifiable "Connect with Alby" assertion - Fix stripe/paypal: replace no-op isDisabled() with toBeChecked() on paypal switch Co-Authored-By: Claude Sonnet 4.6 <[email protected]> * fix(e2e): fix stale comment in stripe price test Co-Authored-By: Claude Sonnet 4.6 <[email protected]> * fix(e2e): address cubic review feedback on assertion patterns - workflows.ts: replace non-retrying isDisabled() snapshot with toBeDisabled() - out-of-office.e2e.ts: add .first() for away-emoji strict mode, use toast-error testid, remove unused const t - payment-apps.e2e.ts: replace always-passing toBeTruthy() with toHaveURL() assertion * fix --------- Co-authored-by: Claude Sonnet 4.6 <[email protected]> Co-authored-by: Romit <[email protected]> Co-authored-by: Romit <[email protected]>

When the flagged assertion was already awaited (`await expect(locator).toBeTruthy()` — the common real-world shape, e.g. calcom/cal.diy#28486), the autofix prepended a second `await`, producing `await await expect(...).toBeVisible()`. Detect an AwaitExpression parent and reuse that await instead of adding one. Constraint: ESLint --fix must emit clean, valid code unattended. Rejected: replacing the whole AwaitExpression node — reusing the await is a smaller touch. Confidence: high Scope-risk: none — detection unchanged; only the fix's await handling. Not-tested: eslint --fix across a large real repo (covered by RuleTester await/return/soft cases). Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

pull-request-size Bot added the size/M label Mar 18, 2026

cubic-dev-ai Bot reviewed Mar 18, 2026

View reviewed changes

Comment thread apps/web/playwright/payment-apps.e2e.ts

Comment thread apps/web/playwright/payment-apps.e2e.ts

voidmatcha mentioned this pull request Mar 18, 2026

fix(e2e): fix false-passing assertions and CI test failures #28485

Closed

romitg2 marked this pull request as ready for review March 22, 2026 15:56

romitg2 added ready-for-e2e run-ci Approve CI to run for external contributors labels Mar 22, 2026

romitg2 previously approved these changes Mar 22, 2026

View reviewed changes

voidmatcha and others added 3 commits March 23, 2026 07:19

fix(e2e): fix stale comment in stripe price test

5b073db

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

voidmatcha dismissed romitg2’s stale review via 6d32905 March 22, 2026 22:21

voidmatcha force-pushed the fix/e2e-clean-assertions branch from 0df41f3 to 6d32905 Compare March 22, 2026 22:21

Merge branch 'main' into fix/e2e-clean-assertions

bbcd73e

romitg2 added run-ci Approve CI to run for external contributors and removed run-ci Approve CI to run for external contributors labels Mar 25, 2026

fix

eed95ac

romitg2 approved these changes Mar 25, 2026

View reviewed changes

romitg2 previously approved these changes Mar 25, 2026

View reviewed changes

romitg2 added ready-for-e2e run-ci Approve CI to run for external contributors and removed ready-for-e2e run-ci Approve CI to run for external contributors labels Mar 25, 2026

romitg2 enabled auto-merge (squash) March 25, 2026 04:20

romitg2 added community Created by Linear-GitHub Sync run-ci Approve CI to run for external contributors and removed run-ci Approve CI to run for external contributors labels Mar 25, 2026

fix

fbf8bcf

romitg2 dismissed their stale review via fbf8bcf March 25, 2026 05:00

romitg2 and others added 2 commits March 25, 2026 10:37

fix

7dc4597

Merge branch 'main' into fix/e2e-clean-assertions

f0758e7

romitg2 added ready-for-e2e run-ci Approve CI to run for external contributors and removed ready-for-e2e run-ci Approve CI to run for external contributors labels Mar 25, 2026

romitg2 approved these changes Mar 25, 2026

View reviewed changes

romitg2 merged commit ecc5e66 into calcom:main Mar 25, 2026
93 of 97 checks passed

voidmatcha mentioned this pull request Jun 27, 2026

feat(no-silent-pass): flag always-passing assertions on a Locator mskelton/eslint-plugin-playwright#470

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

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

fix(e2e): replace false-passing assertions and hard-coded waits in test suite#28486
romitg2 merged 8 commits into
calcom:mainfrom
voidmatcha:fix/e2e-clean-assertions

voidmatcha commented Mar 18, 2026

Uh oh!

cubic-dev-ai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cubic-dev-ai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

romitg2 left a comment

Uh oh!

voidmatcha commented Mar 22, 2026 •

edited

Loading

Uh oh!

romitg2 left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

voidmatcha commented Mar 18, 2026

What does this PR do?

Problem 1: toBeTruthy() on Locator objects (always passes)

Problem 2: isDisabled() result not asserted (no-op)

Problem 3: toBeTruthy() on booleans and nullable values

Problem 4: waitForTimeout() — brittle arbitrary waits

Problem 5: Strict mode violation and toast assertion in out-of-office.e2e.ts

Problem 6: Alby setup page navigation in payment-apps.e2e.ts

Mandatory Tasks (DO NOT REMOVE)

How should this be tested?

Checklist

Uh oh!

cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

romitg2 left a comment

Choose a reason for hiding this comment

Uh oh!

voidmatcha commented Mar 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

romitg2 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Problem 1: `toBeTruthy()` on Locator objects (always passes)

Problem 2: `isDisabled()` result not asserted (no-op)

Problem 3: `toBeTruthy()` on booleans and nullable values

Problem 4: `waitForTimeout()` — brittle arbitrary waits

Problem 5: Strict mode violation and toast assertion in `out-of-office.e2e.ts`

Problem 6: Alby setup page navigation in `payment-apps.e2e.ts`

voidmatcha commented Mar 22, 2026 •

edited

Loading