-
Notifications
You must be signed in to change notification settings - Fork 265
include tests/e2e in pnpm workspace for shared tooling #1086
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR integrates the tests/e2e folder into the pnpm workspace to share tooling (linters, Prettier, etc.) across the project. The changes remove local tool dependencies, delete the standalone ESLint config and package-lock.json, and update the GitHub Actions workflow to use pnpm instead of npm for E2E test execution.
Changes:
- Added
tests/e2eto the pnpm workspace and updated dependencies to use shared tooling - Removed redundant configuration files (
eslint.config.mjs,package-lock.json) from the e2e directory - Updated GitHub Actions workflow to use pnpm for installing and running E2E tests
Reviewed changes
Copilot reviewed 4 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
tests/e2e/package.json |
Removed TypeScript ESLint plugins, downgraded ESLint to 8.57.1, added nx configuration for task orchestration |
tests/e2e/package-lock.json |
Deleted entire package-lock.json (2726 lines) as e2e is now part of pnpm workspace |
tests/e2e/eslint.config.mjs |
Removed standalone ESLint configuration file (52 lines) |
frontend/pnpm-workspace.yaml |
Added ../tests/e2e to workspace packages |
frontend/pnpm-lock.yaml |
Added e2e dependencies to the shared lockfile |
.github/workflows/pr-builder.yml |
Updated E2E job to use pnpm instead of npm, improved logging and error handling |
Files not reviewed (2)
- frontend/pnpm-lock.yaml: Language not supported
- tests/e2e/package-lock.json: Language not supported
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1086 +/- ##
==========================================
- Coverage 87.22% 87.20% -0.03%
==========================================
Files 535 535
Lines 36114 36114
Branches 1611 1611
==========================================
- Hits 31501 31492 -9
- Misses 2978 2985 +7
- Partials 1635 1637 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 4 out of 6 changed files in this pull request and generated 2 comments.
Files not reviewed (2)
- frontend/pnpm-lock.yaml: Language not supported
- tests/e2e/package-lock.json: Language not supported
| "prettier": "3.4.2", | ||
| "typescript": "5.7.3" | ||
| "prettier": "3.6.2", | ||
| "typescript": "5.9.3" |
Copilot
AI
Jan 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The TypeScript version should be aligned with the workspace catalog. According to the pnpm-lock.yaml, the catalog specifies TypeScript 5.9.3, so this should use 'catalog:' to maintain consistency across the workspace.
| "typescript": "5.9.3" | |
| "typescript": "catalog:typescript" |
| TEST_USER_USERNAME: ${{ secrets.PLAYWRIGHT_TEST_USER_USERNAME || vars.PLAYWRIGHT_TEST_USER_USERNAME || 'testuser' }} | ||
| TEST_USER_PASSWORD: ${{ secrets.PLAYWRIGHT_TEST_USER_PASSWORD || 'admin' }} | ||
| PLAYWRIGHT_WORKERS: ${{ vars.PLAYWRIGHT_WORKERS || 1 }} | ||
| PLAYWRIGHT_WORKERS: ${{ vars.PLAYWRIGHT_WORKERS || '1' }} |
Copilot
AI
Jan 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The environment variable is being set as a string '1' instead of numeric 1. While this may work, Playwright typically expects numeric values for the workers configuration. Consider removing the quotes or ensuring Playwright handles string-to-number conversion correctly.
| PLAYWRIGHT_WORKERS: ${{ vars.PLAYWRIGHT_WORKERS || '1' }} | |
| PLAYWRIGHT_WORKERS: ${{ vars.PLAYWRIGHT_WORKERS || 1 }} |
Purpose
This PR adds the tests/e2e folder to the existing pnpm workspace so it can reuse shared tools such as linters, Prettier, and other common dependencies. This helps keep tooling consistent across the project and avoids managing duplicate configurations.
🔧 Summary of Breaking Changes
Describe what is changing
💥 Impact
What will break? Who is affected?
🔄 Migration Guide
How should users update their code/configuration to adapt to the breaking changes? Include examples if helpful
-->
Approach
Related Issues
Related PRs
Checklist
breaking changelabel added.Security checks