fix: Add npm install step for E2E test dependencies - #36
Conversation
**Root Cause**: Playwright E2E tests failed because @playwright/test package was never installed. The tests/e2e/ directory has its own package.json with @playwright/test as a dependency, but the GitHub Actions workflow only installed dependencies in web-ui/. **Error**: "Cannot find module '@playwright/test'" when loading playwright.config.ts **Changes**: 1. Add "Install E2E test dependencies" step before running Playwright tests - Runs `npm ci` in tests/e2e/ directory - Generates package-lock.json for reproducible builds 2. Fix Playwright browser installation directory - Changed from web-ui/ to tests/e2e/ - Ensures browsers install in correct location 3. Add package-lock.json to tests/e2e/ - Generated by running `npm install` in tests/e2e/ - Locks @playwright/test@^1.40.0, @types/node@^20.10.0, typescript@^5.3.0 **Impact**: - E2E Frontend Tests should now execute (currently fail at config load) - Playwright can import required modules - CI environment matches local development setup **Testing**: - Will verify in next CI run - Local testing: cd tests/e2e && npm ci && npx playwright test Closes issue with Playwright test execution in GitHub Actions.
WalkthroughThese changes fix Playwright E2E test execution failures in the CI workflow by installing dependencies and running tests from the correct directory (tests/e2e instead of web-ui). A comprehensive failure analysis document and restructured session documentation accompany the workflow fix. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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 |
Code Review - PR #36: Fix Playwright E2E Test DependenciesSummaryThis PR correctly addresses the root cause of Playwright E2E test failures in GitHub Actions by ensuring dependencies are installed in the ✅ Strengths1. Excellent Root Cause AnalysisThe
This level of documentation will be invaluable for future maintainers and demonstrates thorough engineering practice. 2. Minimal, Targeted FixThe workflow changes are precisely scoped:
3. Proper Dependency LockingIncluding
4. Clear PR DescriptionThe PR body effectively communicates:
🔍 Code Quality AssessmentWorkflow Changes (
|
| Metric | Score | Notes |
|---|---|---|
| Code Quality | ⭐⭐⭐⭐⭐ | Excellent - minimal, targeted fix |
| Documentation | ⭐⭐⭐⭐⭐ | Outstanding - thorough root cause analysis |
| Testing | ⭐⭐⭐⭐☆ | Good - tested locally, will verify post-merge |
| Security | ⭐⭐⭐⭐⭐ | No concerns |
| Performance | ⭐⭐⭐⭐⭐ | Negligible impact |
Overall: 5/5 - Excellent PR 🎉
Great work on the thorough investigation and clean fix! The documentation quality particularly stands out - this will be a valuable reference for the team.
Review generated using repository CLAUDE.md guidelines and best practices
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
.github/workflows/test.yml (1)
304-309: Update Node.js cache configuration to include tests/e2e dependencies.The cache setup only references
web-ui/package-lock.jsonbut the workflow now installs dependencies from bothweb-ui/andtests/e2e/. Update thecache-dependency-pathto cache both:- name: Set up Node.js uses: actions/setup-node@v4 with: node-version: ${{ env.NODE_VERSION }} cache: 'npm' - cache-dependency-path: 'web-ui/package-lock.json' + cache-dependency-path: | + web-ui/package-lock.json + tests/e2e/package-lock.jsonE2E_PLAYWRIGHT_FAILURE_ANALYSIS.md (1)
3-3: Fix static analysis issues: wrap bare URLs and specify code block languages.Address the following linting issues to improve documentation quality:
- Line 3 & Lines 193-194: Wrap bare URLs in markdown link syntax
- Lines 24 & 77: Specify language identifiers for fenced code blocks
Apply these fixes:
-**GitHub Actions Run**: https://github.com/frankbria/codeframe/actions/runs/19883332345 +**GitHub Actions Run**: [https://github.com/frankbria/codeframe/actions/runs/19883332345](https://github.com/frankbria/codeframe/actions/runs/19883332345) -``` +```json { "devDependencies": { -``` +``` WARNING: It looks like you are running 'npx playwright install' without firstFor lines 193-194, wrap the localhost URLs similarly:
-2. ✅ Frontend server started (PID: 5416) +2. ✅ Frontend server started (PID: 5416) - [http://localhost:3000](http://localhost:3000)Also applies to: 24-24, 77-77, 193-194
claudedocs/SESSION.md (1)
28-28: Remove the dangling reference or add the referenced plan details.Line 28 states "See full plan details below." but no plan details follow. Either remove this line (if the document is intentionally a summary-only status view) or add the referenced details.
See full plan details below. + +(Or remove this line if full details are not included.)Clarify the intended scope for this document: is it meant to be a brief status summary, or should it include detailed task-by-task plan information?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
tests/e2e/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
.github/workflows/test.yml(1 hunks)E2E_PLAYWRIGHT_FAILURE_ANALYSIS.md(1 hunks)claudedocs/SESSION.md(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.md
📄 CodeRabbit inference engine (AGENTS.md)
Documentation files must be sized to fit in a single agent context window (spec.md ~200-400 lines, plan.md ~300-600 lines, tasks.md ~400-800 lines)
Files:
E2E_PLAYWRIGHT_FAILURE_ANALYSIS.mdclaudedocs/SESSION.md
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
Repo: frankbria/codeframe PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T19:08:20.036Z
Learning: Applies to tests/e2e/**/*.py : Use TestSprite MCP for E2E test generation and Playwright for frontend E2E testing
Learnt from: CR
Repo: frankbria/codeframe PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T19:08:20.036Z
Learning: Applies to web-ui/**/*.{ts,tsx,test.ts,test.tsx} : Run frontend tests with npm test from web-ui directory
📚 Learning: 2025-11-25T19:08:20.036Z
Learnt from: CR
Repo: frankbria/codeframe PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T19:08:20.036Z
Learning: Applies to web-ui/**/*.{ts,tsx,test.ts,test.tsx} : Run frontend tests with npm test from web-ui directory
Applied to files:
.github/workflows/test.yml
📚 Learning: 2025-11-25T19:08:20.036Z
Learnt from: CR
Repo: frankbria/codeframe PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T19:08:20.036Z
Learning: Applies to tests/e2e/**/*.py : Use TestSprite MCP for E2E test generation and Playwright for frontend E2E testing
Applied to files:
.github/workflows/test.ymlE2E_PLAYWRIGHT_FAILURE_ANALYSIS.md
🪛 LanguageTool
E2E_PLAYWRIGHT_FAILURE_ANALYSIS.md
[uncategorized] ~55-~55: The official name of this software platform is spelled with a capital “H”.
Context: ...rkflow Configuration Issue** File: .github/workflows/test.yml (Lines 326-332) ``...
(GITHUB)
🪛 markdownlint-cli2 (0.18.1)
E2E_PLAYWRIGHT_FAILURE_ANALYSIS.md
3-3: Bare URL used
(MD034, no-bare-urls)
24-24: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
77-77: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
193-193: Bare URL used
(MD034, no-bare-urls)
194-194: Bare URL used
(MD034, no-bare-urls)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Frontend Unit Tests
- GitHub Check: Backend Unit Tests
- GitHub Check: claude-review
🔇 Additional comments (2)
.github/workflows/test.yml (1)
330-336: E2E dependency installation and Playwright browser setup look good.The addition of the E2E test dependencies step (lines 330–332) and the corrected
working-directoryfor Playwright browser installation (line 335) directly address the root cause. The order is correct: install npm dependencies first, then install browsers in the correct directory.Verify that
tests/e2e/package-lock.jsonis included in this PR and contains the expected dependencies (@playwright/test,@types/node,typescript).E2E_PLAYWRIGHT_FAILURE_ANALYSIS.md (1)
1-244: Excellent failure analysis document — clear root cause and well-structured remediation.The document provides clear context for the PR fix with accurate root cause analysis, quantified impact, and detailed remediation steps. The distinction between Fix #1 (recommended) and alternatives (not recommended) is well-justified. Verification steps and impact assessment add confidence in the proposed solution.
Archive completed E2E Playwright test fixing documentation from Dec 2-4, 2025 work that was merged in PRs #36, #38, #39. Changes: - Archive 13 analysis/investigation docs to docs/archive/e2e-test-fixes-2025-12/ - Add comprehensive README documenting the 18% → 54% pass rate improvement - Update CLAUDE.md with code style section - Add session documentation for skip test cleanup work Archived docs cover: - Root cause analysis of test failures - Implementation plans and investigations - React component bug analysis - Test data requirements - PR summaries The archived work achieved 200% improvement in E2E test pass rates through comprehensive test data seeding, frontend bug fixes, and test infrastructure improvements.
1 participant
Problem
Playwright E2E tests in GitHub Actions were failing with:
Root Cause
The
tests/e2e/directory has its ownpackage.jsonwith@playwright/testas a dependency, but the GitHub Actions workflow only installed dependencies inweb-ui/. When Playwright tried to loadplaywright.config.ts, it couldn't find the required@playwright/testmodule.Solution
Changes Made:
Add "Install E2E test dependencies" step (Line 330-332)
Fix Playwright browser installation directory (Line 334-336)
web-ui/totests/e2e/Add
package-lock.jsontotests/e2e/npm installintests/e2e/@playwright/test@^1.40.0,@types/node@^20.10.0,typescript@^5.3.0Investigation Details
See
E2E_PLAYWRIGHT_FAILURE_ANALYSIS.mdfor comprehensive root cause analysis with evidence chain.Impact
Testing
Note: E2E tests only run on
mainbranch (workflow condition:if: github.ref == 'refs/heads/main'), so this PR won't trigger E2E test execution. Tests will run after merge to main.Local testing:
Related
Files Changed
.github/workflows/test.yml- Added E2E dependency installationtests/e2e/package-lock.json- New file for reproducible buildsE2E_PLAYWRIGHT_FAILURE_ANALYSIS.md- Investigation documentationclaudedocs/SESSION.md- Session documentationChecklist
Summary by CodeRabbit
Bug Fixes
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.