docs: document playwright-admin.js as manual-only (PL-034) - #139
Conversation
PL-034: the script hardcoded an absolute path to one specific sandbox's
global playwright install (/opt/node22/...), assumed a test-admin.html
fixture that isn't committed anywhere in the repo, assumed a dev server
already running on port 9999, and had no pass/fail assertions (a run
with console errors still exited 0). None of that is fixable without
committing a fixture, starting a server in CI, and rewriting it as real
assertions — out of scope here.
Instead: replace the hardcoded path with a normal `import('playwright')`
that fails with clear install instructions instead of a cryptic
ERR_MODULE_NOT_FOUND, and document in the file header and CLAUDE.md that
this is a manual local tool, not part of `npm test`/CI.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
PR Summary by QodoDocument Playwright admin walkthrough as manual-only; fix admin-app sync/lucide/codec
AI Description
Diagram
High-Level Assessment
Files changed (4)
|
Code Review by Qodo
Context used✅ Compliance rules (platform):
1 rule 1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
87a2d84
into
claude/pr-469-audit-rebase-ggp0e4
2 participants
Summary
Fourth PR from SLASHED-Plugins' technical-debt audit remediation (Wave 0).
tests/playwright-admin.jsis a manual visual-QA walkthrough of the admin SPA (screenshots + a console-error report), but as committed it was effectively unusable outside whatever one sandbox it was authored in:import { chromium } from '/opt/node22/lib/node_modules/playwright/index.mjs'— a hardcoded absolute path to one specific machine's global install, not a real dependency of this repo (confirmed:playwrightisn't declared in anypackage.jsonhere, and isn't present in this repo's ownnode_modules).test-admin.htmlfixture served locally athttp://localhost:9999— that file isn't committed anywhere in the repo.Turning this into a real, CI-wired test would mean committing the missing HTML fixture, starting a dev server in CI, and rewriting the walkthrough as actual assertions — a much bigger change than an audit-remediation pass warrants. Instead:
import('playwright')(normal module resolution), so a missing install now fails with a clear one-line fix instead of a crypticERR_MODULE_NOT_FOUNDpointing at a path that only ever existed on one machine.playwrightyourself, havetest-admin.htmlserved locally) and inCLAUDE.md'stests/section that this is a manual, local-only dev tool — not part ofnpm testor CI.Verification
node --check tests/playwright-admin.js— valid syntax.playwrightinstalled in this sandbox's project) — confirmed it now fails with the intended clear instructions and exit code 1, instead of the old cryptic module-resolution error.npm test: 67/67 passing (unaffected — this script was never part of the suite).npm run lint: clean.Type
Checklist
npm testpassesnpm run lintpassesnpm run verifypassesCHANGELOG.mdupdated — not user-facing (dev-tooling doc fix only)Generated by Claude Code