ENG-217: Add Playwright E2E testing skill for Copilot#16480
Conversation
✅ Deploy Preview for care-ohc ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a Playwright skill document for CARE that covers E2E scope, setup and run commands, test layout and naming, execution rules, debugging patterns, advanced interaction techniques, and test independence guidance. ChangesPlaywright skill documentation
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
🎭 Playwright Test ResultsStatus: ⏭️ Skipped No test-related files were changed in this PR. Tests are skipped when changes don't affect:
Run: #9774 |
Deploying care-preview with
|
| Latest commit: |
d85a708
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://4ed273af.care-preview-a7w.pages.dev |
| Branch Preview URL: | https://eng-217-playwright-skill.care-preview-a7w.pages.dev |
Greptile SummaryThis PR adds a new Copilot Playwright skill at
Confidence Score: 5/5This is a documentation-only change adding a Copilot skill guide; it ships no runnable application code and cannot break the product. All previously-reviewed code-example bugs (toPass callback, infinite combobox loop, portal-scoped option query, missing facilityId) have been fixed. The only remaining note is a hardcoded name string in an API-setup snippet that contradicts the skill's own faker rule, but this does not affect production behaviour. No files require special attention. Important Files Changed
Reviews (10): Last reviewed commit: "docs: invoke API setup helper inside bef..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
This PR introduces a new GitHub Copilot skill (playwright) intended to guide contributors (and agents) in writing, debugging, and running Playwright E2E tests for the CARE frontend, including conventions, rules, and copy/paste patterns.
Changes:
- Adds a comprehensive Playwright E2E testing skill document with setup commands, organization conventions, and rules.
- Provides reusable patterns for API verification, selectors, form testing, flakiness triage, and multi-role workflows.
nihal467
left a comment
nihal467
left a comment
There was a problem hiding this comment.
Addressed the AI review comments in follow-up commit 0938810.
What was updated in .github/skills/playwright/SKILL.md:
- Clarified faker rule to allow fixture-backed constants for known values.
- Made serial example align with repo style using
test.describe.configure({ mode: "serial" }). - Fixed undefined placeholders in copy/paste snippets:
- Added
getFacilityId()usage in direct API setup and multi-role examples. - Defined
scopein the data-slot command-input snippet.
- Added
- Fixed
expect.toPass()pattern to useexpect(...).toBeVisible()so retries work correctly. - Added iteration guard (
MAX_CASCADES) to cascading-combobox pattern to prevent infinite loops. - Updated file upload example to real fixture paths that exist in repo.
- Updated “Keep Tests Independent” wording to match serial configuration style.
Thanks for the review — these were good catches and are now reflected in the doc.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/skills/playwright/SKILL.md:
- Line 190: Remove the self-edit guidance from the Playwright skill text in
SKILL.md: the “Continuously improve this skill” item should not tell the agent
to update its own instruction file during normal use. Edit that section to keep
improvement suggestions external, and make the wording in the skill’s numbered
guidance explicit that updates to SKILL.md must come through a separate
human-reviewed PR or maintainer-approved process rather than self-modification.
- Around line 173-185: Update the Playwright skill guidance to narrow the
`faker` recommendation: the current rule in the skills doc overstates “always
use faker,” which conflicts with deterministic assertions and the existing
constants guidance. Revise the bullet tied to `faker` so it says to use `faker`
only for generated inputs where the exact value does not matter, while keeping
fixed literals/constants for cases that need stable assertions; adjust the
wording in the same section alongside the deterministic fixture ID and constants
rules for consistency.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5d4c0332-d07b-4b2f-9bb2-7061b7820160
📒 Files selected for processing (1)
.github/skills/playwright/SKILL.md
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.github/skills/playwright/SKILL.md (1)
190-190: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winRemove the self-edit instruction.
Still present: this tells the agent to update
SKILL.mdfrom within the skill itself, recreating the self-modification risk flagged earlier. Keep updates external and human-reviewed.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/skills/playwright/SKILL.md at line 190, Remove the self-edit guidance from the Playwright skill instructions in SKILL.md so the skill no longer tells the agent to update itself; specifically, delete the “Continuously improve this skill” item and keep any future skill updates outside the skill file, with human review. Preserve the rest of the Playwright workflow content unchanged.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/skills/playwright/SKILL.md:
- Around line 399-405: The option lookup in the cascade loop is scoped
incorrectly: `combobox.getByRole("option")` only searches inside the combobox
subtree and can miss the popup. Update the `MAX_CASCADES` iteration logic in the
`comboboxes` loop to click the combobox and then query the opened `listbox` for
the option, using the existing `region.getByRole("combobox")` flow and replacing
the option search on the `combobox` with a popup-scoped query.
---
Duplicate comments:
In @.github/skills/playwright/SKILL.md:
- Line 190: Remove the self-edit guidance from the Playwright skill instructions
in SKILL.md so the skill no longer tells the agent to update itself;
specifically, delete the “Continuously improve this skill” item and keep any
future skill updates outside the skill file, with human review. Preserve the
rest of the Playwright workflow content unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 470207b6-8d2e-481c-a978-464d0e317a51
📒 Files selected for processing (1)
.github/skills/playwright/SKILL.md
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/skills/playwright/SKILL.md:
- Around line 336-345: The createAccountViaApi helper currently returns parsed
JSON without verifying the fetch result, so failed setup calls can be mistaken
for valid data. Update createAccountViaApi to inspect the Response before
calling res.json(), and fail immediately on non-OK responses with a clear error
that includes the status and any response text. Keep the check localized to
createAccountViaApi so the setup path in SKILL.md cannot continue with bad seed
data.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 751cdc75-2a04-4068-874c-86384bd32af3
📒 Files selected for processing (1)
.github/skills/playwright/SKILL.md
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/skills/playwright/SKILL.md (1)
525-558: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winClose the contexts in
finally.If any step/assertion throws, both
adminContext.close()andnurseContext.close()are skipped, which can leave browser contexts hanging and make later tests flaky.♻️ Proposed fix
test("Admin assigns task, nurse sees it", async ({ browser }) => { const facilityId = getFacilityId(); // Admin context const adminContext = await browser.newContext({ storageState: "tests/.auth/user.json", }); - const adminPage = await adminContext.newPage(); - - await test.step("Admin creates assignment", async () => { - await adminPage.goto(`/facility/${facilityId}/...`); - // ... admin actions - }); - await adminContext.close(); + try { + const adminPage = await adminContext.newPage(); + await test.step("Admin creates assignment", async () => { + await adminPage.goto(`/facility/${facilityId}/...`); + // ... admin actions + }); + } finally { + await adminContext.close(); + } // Nurse context const nurseContext = await browser.newContext({ storageState: "tests/.auth/nurse.json", }); - const nursePage = await nurseContext.newPage(); - - await test.step("Nurse verifies assignment", async () => { - await nursePage.goto(`/facility/${facilityId}/...`); - await expect(nursePage.getByText("Assigned Task")).toBeVisible(); - }); - await nurseContext.close(); + try { + const nursePage = await nurseContext.newPage(); + await test.step("Nurse verifies assignment", async () => { + await nursePage.goto(`/facility/${facilityId}/...`); + await expect(nursePage.getByText("Assigned Task")).toBeVisible(); + }); + } finally { + await nurseContext.close(); + } });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/skills/playwright/SKILL.md around lines 525 - 558, The multi-user Playwright example leaves adminContext and nurseContext open if any step fails, because the close calls happen after the assertions. Wrap the admin and nurse context usage in try/finally blocks (or a single outer finally) so both browser contexts are always closed even when a step in the test using browser.newContext(), adminPage, or nursePage throws.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In @.github/skills/playwright/SKILL.md:
- Around line 525-558: The multi-user Playwright example leaves adminContext and
nurseContext open if any step fails, because the close calls happen after the
assertions. Wrap the admin and nurse context usage in try/finally blocks (or a
single outer finally) so both browser contexts are always closed even when a
step in the test using browser.newContext(), adminPage, or nursePage throws.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1f0827a0-7c8d-4a59-9840-ce7becc44977
📒 Files selected for processing (1)
.github/skills/playwright/SKILL.md
|
Relocated this skill to the central skills repo: ohcnetwork/skills#2 This PR is left open for now in case we want the skill auto-loaded from |
Summary
Adds a Copilot Playwright skill at
.github/skills/playwright/SKILL.md— a single reference for writing E2E tests in CARE.Contents
storageState, no.skip(), no random selection from lists, faker for created data, etc.)Promise.alllistener-before-action ordering, API setup viabeforeEachwith fail-fast, multi-role tests withtry/finallycontext cleanup, file uploads with real fixtures, delete with double-confirmCloses
ENG-217