test: fix always-passing e2e assertions and missing awaits#7616
test: fix always-passing e2e assertions and missing awaits#7616voidmatcha wants to merge 1 commit into
Conversation
- basic-auth: expect(locator).toBeTruthy() passes for any Locator object, so all three auth-failure tests verified nothing; converted to await expect(locator).toBeVisible(). The signup test asserted 'test@gmail.com' while signing up 'test2@gmail.com' - latent expected- value typo that the dead assertion never caught; corrected to the signed-up address. - custom-basepath + params: one-shot isVisible()/innerText()/page.url() reads converted to auto-retrying toBeVisible/toHaveText/toHaveURL (9 one-shot URL reads in params.spec.ts swept in the same pass) - solid transition: await six floating increase-button click() statements Same families exist in the solid/vue mirror suites; left untouched to keep this change reviewable. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR modernizes Playwright e2e test assertions across four independent test suites by replacing manual DOM queries and page checks with Playwright's built-in matchers, and improves async click handling in transition tests. ChangesPlaywright e2e test assertion modernization
🎯 2 (Simple) | ⏱️ ~10 minutes
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This PR strengthens existing Playwright E2E assertions. It does not add behavior or rename tests.
What changed:
expect(locator).toBeTruthy()checks in the basic auth fixture withawait expect(locator).toBeVisible(). A PlaywrightLocatorobject is always truthy, so the previous checks did not verify the visible error text.test@gmail.comtotest2@gmail.com, matching the address passed tologin(page, 'test2@gmail.com', ...). The dead truthy assertion was hiding that expected-value typo.page.url(),innerText(), andisVisible()reads to web-first Playwright matchers (toHaveURL,toHaveText,toBeVisible).awaitto sixincrease-button.click()calls in the Solid transition fixture so the following assertions do not race the click actions.Scope note:
I intentionally kept this to 4 files and one assertion-family migration. The same broad families still appear in sibling Solid, Vue, and React fixture suites. I can follow up separately if maintainers want the mirror suites swept too.
Blame check:
Verification:
origin/mainon 2026-06-13.origin/mainand ran:bash scripts/pr-preflight.sh tmp/preflight-tanstack-router e2e/react-router/basic-file-based/tests/params.spec.ts e2e/react-start/basic-auth/tests/app.spec.ts e2e/react-start/custom-basepath/tests/navigation.spec.ts e2e/solid-router/basic-file-based/tests/transition.spec.tsPREFLIGHT 0 fail(s)with scanner deltatotal from 51 to 26, p0 from 26 to 1, ast from 10 to 0; AST artifacts clean; diff hygiene clean; authoring clean with 0 added comments and no test renames.node_modules, so preflight skipped tsc, eslint, and Playwright because local repo binaries were absent. The documented full path requirespnpm install,pnpm exec playwright install, and the repo build/dev setup.Found while reviewing the test suite with
e2e-skills/e2e-reviewer.