feat: setup Playwright e2e testing infrastructure#328
feat: setup Playwright e2e testing infrastructure#328DSingh0304 wants to merge 88 commits intokeploy:mainfrom
Conversation
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
Code Review SummaryStatus: No Issues Found | Recommendation: Merge OverviewThis PR adds Playwright E2E testing infrastructure to the blog website. The implementation is well-structured and follows best practices:
Files Reviewed (7 files)
Notes
|
There was a problem hiding this comment.
Pull request overview
Sets up Playwright-based end-to-end testing for the Next.js blog app, including local developer workflows and CI execution in GitHub Actions.
Changes:
- Adds Playwright test configuration (multi-browser projects, webServer auto-start, artifact output dirs).
- Introduces initial e2e test structure with a placeholder smoke test and
.env.testtemplate. - Adds npm scripts and a GitHub Actions workflow to run Playwright tests in CI.
Reviewed changes
Copilot reviewed 5 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
tests/e2e/smoke.spec.ts |
Adds initial smoke test exercising homepage load/title. |
playwright.config.ts |
Defines Playwright config (projects, timeouts, reporters, webServer, env loading). |
package.json |
Adds Playwright/dotenv dev deps and e2e scripts; updates Node types. |
package-lock.json |
Locks new dependencies (@playwright/test, playwright, dotenv) and type updates. |
.gitignore |
Updates ignored env/test artifact patterns, adds Playwright output/cache ignores. |
.github/workflows/playwright.yml |
Adds CI workflow to run Playwright tests (Chromium matrix initially). |
.env.test.example |
Provides template env vars for local e2e runs. |
Comments suppressed due to low confidence (2)
.github/workflows/playwright.yml:74
- This workflow creates a new PR comment on every run and doesn’t declare the
issues: write/pull-requests: writepermissions needed forgithub.rest.issues.createComment(the repo already uses explicit permissions in other workflows). This can fail the job in repos/orgs with restricted default token permissions, and it will spam PR threads. Prefer using a create-or-update comment action (sticky comment) and add an explicitpermissionsblock, or remove the commenting step entirely.
- name: Comment PR with result
if: github.event_name == 'pull_request' && always()
uses: actions/github-script@v7
with:
script: |
const status = '${{ job.status }}';
const emoji = status === 'success' ? '✅' : '❌';
const message = status === 'success'
? `${emoji} Playwright tests passed on **${{ matrix.browser }}**!`
: `${emoji} Playwright tests failed on **${{ matrix.browser }}**. Check the [workflow run](https://github.com/${{ github.repository }}/actions/runs/${{ github.run_id }}) for details.`;
github.rest.issues.createComment({
issue_number: context.issue.number,
owner: context.repo.owner,
repo: context.repo.repo,
body: message
});
package.json:56
@types/nodewas bumped from v18 to v25 while the repo’s GitHub Actions workflows run Node 18. This mismatch can introduce confusing/incorrect Node API typings (and may require newer TS features) compared to the actual runtime. Consider pinning@types/nodeto the major that matches the supported runtime (e.g.,^18.x) unless there’s a specific reason to target Node 25 types.
"@playwright/test": "^1.58.2",
"@types/node": "^25.3.0",
"@types/prismjs": "^1.26.3",
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: DSingh0304 <deepshekhar0306@gmail.com>
Signed-off-by: DSingh0304 <deepshekhar0306@gmail.com>
DSingh0304
force-pushed
the
main
branch
4 times, most recently
from
d9ef83a to
0b72371
Compare
Signed-off-by: DSingh0304 <deepshekhar0306@gmail.com>
manasmanohar
left a comment
manasmanohar
left a comment
There was a problem hiding this comment.
@DSingh0304, could you please update the PR description to include the TODOs, WIP items, the current status of the PR, and the Playwright test run video?
Signed-off-by: DSingh0304 <deepshekhar0306@gmail.com>
… for PR comments Signed-off-by: DSingh0304 <deepshekhar0306@gmail.com>
Yes sir I have updated the PR description with the following requirements. |
Signed-off-by: DSingh0304 <deepshekhar0306@gmail.com>
…s component. Signed-off-by: DSingh0304 <deepshekhar0306@gmail.com>
Signed-off-by: DSingh0304 <deepshekhar0306@gmail.com>
Signed-off-by: DSingh0304 <deepshekhar0306@gmail.com>
Signed-off-by: DSingh0304 <deepshekhar0306@gmail.com>
Signed-off-by: DSingh0304 <deepshekhar0306@gmail.com>
Signed-off-by: DSingh0304 <deepshekhar0306@gmail.com>
Signed-off-by: DSingh0304 <deepshekhar0306@gmail.com>
Signed-off-by: DSingh0304 <deepshekhar0306@gmail.com>
Signed-off-by: DSingh0304 <deepshekhar0306@gmail.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 46 out of 48 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
…isibility checks for first post Signed-off-by: Deep <deep@example.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Deep Shekhar Singh <deepshekhar0306@gmail.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 46 out of 48 changed files in this pull request and generated 8 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Deep Shekhar Singh <deepshekhar0306@gmail.com>
…horizontal overflow checks Signed-off-by: Deep <deep@example.com>
…ency in mobile layout tests Signed-off-by: Deep <deep@example.com>
… clicks across multiple components Signed-off-by: Deep <deep@example.com>
… MobileNavigation for improved visibility and reliability of elements before interactions Signed-off-by: Deep <deep@example.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 46 out of 48 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Deep Shekhar Singh <deepshekhar0306@gmail.com>
…locator strategies for better reliability Signed-off-by: Deep <deep@example.com>
…rovide clearer warnings when .env.test is missing Signed-off-by: Deep <deep@example.com>
|
@manasmanohar Sir I have addressed all the comments and ran copilot review and fixed required issues, I have added screenshot in the PR description for the test report, please allow the workflows now so that I can see if all the checks are passing. |
|
@manasmanohar Hello Sir, I see all the checks are passing. Is there anything else I need to do to get this PR merged? |
|
I see one test is failing I will go through it and fix it. |
…checks and refining URL wait conditions in tests. Signed-off-by: Deep <deep@example.com>
|
HI @DSingh0304, will review the pr today once again as discussed in the email thread. Please check the failing tests once |
manasmanohar
left a comment
manasmanohar
left a comment
There was a problem hiding this comment.
Please go through the tests file by file and make sure you fully understand what each test is actually validating.
A good way to verify this is to intentionally break the behavior the test is supposed to cover and confirm the test fails. If it still passes, the test likely isn’t asserting anything meaningful.
This helps catch silent passes caused by stale selectors, incorrect scoping, or guards like if (count > 0) that skip assertions.
A few such cases are already present in this PR (flagged inline). Running this check yourself will be much faster than going through multiple review iterations.
| test('TOC should be sticky on desktop while scrolling', async ({ page }) => { | ||
| const tocContainer = page.locator('.hidden.lg\\:inline-block'); | ||
| const count = await tocContainer.count(); | ||
| if (count > 0) { |
There was a problem hiding this comment.
why are we using css class selector instead of data-testid?
There was a problem hiding this comment.
I have migrated all CSS/ID-based selectors to data-testid across the components and updated the tests. I also removed the conditional guards that were causing silent passes.
| test('TOC should be sticky on desktop while scrolling', async ({ page }) => { | ||
| const tocContainer = page.locator('.hidden.lg\\:inline-block'); | ||
| const count = await tocContainer.count(); | ||
| if (count > 0) { |
There was a problem hiding this comment.
I think we had already discussed issues with such assertions and logic. please address all such instances without any fail before proceeding.
Using if (count > 0) makes the test pass even if the TOC is missing, which defeats the purpose of the test.
- locator.count() + manual checks bypass Playwright’s built-in auto-waiting and assertions.
- Reading class via getAttribute is brittle and unnecessary.
- The Tailwind selector (.hidden.lg\:inline-block) is fragile and tightly coupled to styling.
There was a problem hiding this comment.
I have migrated all tests to removed all if (count > 0) guards, and replaced brittle .getAttribute('class') logic with Playwright's toHaveClass()
| await expect(tocToggle.first()).toBeVisible(); | ||
| }); | ||
|
|
||
| test('desktop TOC sidebar should be hidden on mobile', async ({ page }) => { |
There was a problem hiding this comment.
similar issue here.
- brittle selector
'.hidden.lg\\:inline-block'); .not.toBeVisible()assertion passes even when no element is found.
There was a problem hiding this comment.
replaced brittle .getAttribute('class') logic with Playwright's toHaveClass()
| }); | ||
|
|
||
| test.describe('API Mocking — Client-Side Search Resilience', () => { | ||
|
|
There was a problem hiding this comment.
are we testing using ssr mock data or client
There was a problem hiding this comment.
The tests use the mock data for both SSR and Client.
The configuration in
playwright.config.ts sets the GraphQL endpoint to our mock server localhost:4000 for both environments:
SSR: via the WORDPRESS_API_URL environment variable.
Client: via the NEXT_PUBLIC_WORDPRESS_API_URL environment variable.
This ensures consistency whether the content is rendered on the server or fetched dynamically in the browser.
tests/components/Tag.spec.ts
Outdated
| test('each tag should have non-empty text', async ({ page }) => { | ||
| const tagLinks = page.locator('a[href*="/tag/"]'); | ||
| const count = await tagLinks.count(); | ||
| if (count > 0) { |
There was a problem hiding this comment.
why is every test body is inside if (count > 0)?
There was a problem hiding this comment.
removed all if (count > 0) guards
tests/components/Tag.spec.ts
Outdated
| }); | ||
|
|
||
| test('each tag should have non-empty text', async ({ page }) => { | ||
| const tagLinks = page.locator('a[href*="/tag/"]'); |
There was a problem hiding this comment.
please look into the playwright selector best practices, seeing multiple other files with css based selectors
The test should verify what the user sees, not how it's styled.
There was a problem hiding this comment.
I have replaced all brittle CSS and href selectors with data-testid="tag-link" in the Tag component and its spec, ensuring all 6 tests now strictly follow Playwright best practices and pass.
| }); | ||
|
|
||
| test('post cards should be single column on mobile', async ({ page }) => { | ||
| const postGrid = page.locator('.grid.grid-cols-1').first(); |
There was a problem hiding this comment.
The test should verify what the user sees, not how it's styled.
There was a problem hiding this comment.
I have replaced the .grid.grid-cols-1 selector in MobileLayout.spec.ts with data-testid="post-grid" and updated the assertion to verify the actual layout to ensure posts are stacked vertically, making the test fully agnostic of styling
…o components and improving test selectors. Signed-off-by: Deep <deep@example.com>
… for content and data-testid locators. Signed-off-by: Deep <deep@example.com>
Related Tickets & Documents
Part of e2e testing initiative
Current Status
✅ Ready for Review - CI/CD issues resolved, workflow passing, ready for merge
Recent Updates
pull-requests: write,issues: write)Description
This PR establishes the foundation for comprehensive end-to-end testing using Playwright. It sets up the testing infrastructure with environment-based configuration, multi-browser support, and GitHub Actions CI integration, along with a working homepage E2E test.
The configuration enables automated testing across different browsers and environments without code changes, provides fast local development feedback, and ensures code quality through automated PR checks.
Changes
Type of Change
Testing
npx playwright install.env.testfrom templatenpm run test:e2e:chromium- All 11 tests passing ✅npm run test:e2e:ui✅Demo
Playwright Test Run Video:
In this video the playwright test is begin run in the CI in chromium browser only but the all the tests have passed in all 3 browsers with 1 test as flaky ( No significant errors or failed tests. )
Test Commands Available:
Browser Coverage:
Environment and Dependencies
New Dependencies:
@playwright/test(v1.58.2) - Test frameworkdotenv(v17.3.1) - Environment variable managementConfiguration Changes:
.env.testfile from.env.test.exampletemplate (optional for local development)requiredno longer needed - WordPress API URL now hardcoded in workflownpx playwright installWork in Progress (WIP)
Current Focus:
Not Included in This PR:
TODOs for Follow-up PRs 📋
Component Tests (High Priority)
Page Tests (High Priority)
Medium Priority
Low Priority
Checklist
Additional Notes
CI/CD Configuration Details:
https://wp.keploy.io/graphql) is hardcoded in workflow for security and fork compatibility