Skip to content

test: fix non-asserting e2e checks (missing awaits, no-op assertions)#8727

Open
voidmatcha wants to merge 1 commit into
QwikDev:mainfrom
voidmatcha:fix/e2e-non-asserting-checks
Open

test: fix non-asserting e2e checks (missing awaits, no-op assertions)#8727
voidmatcha wants to merge 1 commit into
QwikDev:mainfrom
voidmatcha:fix/e2e-non-asserting-checks

Conversation

@voidmatcha

@voidmatcha voidmatcha commented Jun 12, 2026

Copy link
Copy Markdown

What is it?

  • Docs / tests / types / typos

Description

This PR strengthens existing Playwright checks in the Qwik starter and docs E2E suites. It does not add product behavior or rename tests.

The fixes are focused on assertions that currently do not enforce the behavior they describe:

  • Prefix await on 7 web-first assertions whose promises were previously discarded (toHaveURL and toHaveClass).
  • Replace expect(await input.getAttribute("prevented")).toBeDefined() with the repo's existing custom matcher idiom, await expect(input).hasAttribute("prevented"). The old form passes for null, so it did not verify that the attribute exists.
  • Replace a discarded isVisible() boolean and a bare getByText() locator statement with await expect(...).toBeVisible().

Blame check:

  • The docs mobile menu class checks trace to 1230712 from 2025-07-15.
  • The docs resource locator check traces to dd80d5e from 2025-01-17.
  • The affected starter checks trace at least as far back as 7e742eb from 2024-06-25.

Verification:

  • Re-fetched origin/main on 2026-06-13.
  • Applied commit 4d25770 to a temp worktree based on current origin/main and ran:
    bash scripts/pr-preflight.sh tmp/preflight-qwik e2e/docs-e2e/tests/Docs/navBarOnMobile.spec.ts starters/e2e/e2e.sync-qrl.spec.ts starters/e2e/e2e.use-id.spec.ts starters/e2e/qwikcity/nav.spec.ts starters/e2e/qwikcity/resource.spec.ts
  • Result: PREFLIGHT 0 fail(s) with scanner delta total from 16 to 9, p0 from 9 to 2, ast from 5 to 0; AST artifacts clean; diff hygiene clean; authoring clean with 0 added comments and no test renames.
  • Not run locally: repo TypeScript, lint, or Playwright E2E. This clone has no node_modules, and the Qwik E2E path requires the repo dependency and app setup. The preflight therefore skipped tsc, eslint, and Playwright because local repo binaries were absent.

Checklist:

  • My code follows the developer guidelines of this project
  • I performed a self-review of my own code
  • Changeset not needed: test-only assertion fixes, no package release note
  • Docs not needed: no public docs behavior changes
  • Existing tests updated by strengthening their assertions

Found while reviewing the test suite with e2e-skills/e2e-reviewer.

- prefix await on 7 floating expect(...).toHaveURL/.toHaveClass web-first
  assertions (nav.spec.ts, navBarOnMobile.spec.ts) that were never enforced
- replace expect(await input.getAttribute("prevented")).toBeDefined() with
  await expect(input).toHaveAttribute("prevented") - toBeDefined() passes
  for null so the preventdefault behavior was never asserted
- replace a bare getByText() locator statement and a discarded isVisible()
  boolean with web-first toBeVisible() assertions

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@voidmatcha voidmatcha requested a review from a team as a code owner June 12, 2026 15:48
@changeset-bot

changeset-bot Bot commented Jun 12, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 4d25770

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant