-
Notifications
You must be signed in to change notification settings - Fork 0
Add comprehensive E2E testing with Playwright #58
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
Closed
danielway
wants to merge
13
commits into
master
from
claude/plan-e2e-testing-01FzWKPogX5RZchnXuThX5UP
Closed
Add comprehensive E2E testing with Playwright #58
danielway
wants to merge
13
commits into
master
from
claude/plan-e2e-testing-01FzWKPogX5RZchnXuThX5UP
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Implemented end-to-end functional testing framework to validate the full user experience from a browser perspective. This complements existing unit and component tests with real-world user interaction scenarios. Infrastructure: - Installed and configured Playwright for E2E testing - Created page object model (TaskTimerPage) for clean test APIs - Set up test fixtures with automatic localStorage cleanup - Added helper utilities for common test operations - Created comprehensive test directory structure Test Coverage: - Task management: create, edit, delete, keyboard shortcuts - Time tracking: add/remove time increments, totals calculation - Date navigation: switching dates, data isolation - Data persistence: localStorage, page reloads, multi-session workflows CI/CD Integration: - Added E2E test job to GitHub Actions workflow - Configured Playwright browser installation in CI - Set up test artifact uploads for debugging failures - Tests run automatically on all PRs and pushes Scripts Added: - npm run test:e2e - Run all E2E tests - npm run test:e2e:ui - Interactive UI mode for development - npm run test:e2e:headed - Run with visible browser - npm run test:e2e:debug - Debug mode with Playwright inspector - npm run test:e2e:report - View HTML test report Documentation: - Created comprehensive E2E test README with examples - Updated CONTRIBUTING.md with E2E testing guidelines - Documented test organization and best practices - Added instructions for local development vs CI environments Note: Tests are configured for GitHub Pages path (/task-timer/) which matches the CI environment. Local development requires changing the path to / as documented in e2e/README.md.
- Apply Prettier formatting to all E2E test files - Add eslint-disable comment for Playwright fixture's use() callback - Configure Vitest to exclude e2e directory from unit test runs - Ensure all pre-commit checks pass (format, lint, build, tests)
…ting-01FzWKPogX5RZchnXuThX5UP
The E2E test files were being included in coverage reports but not executed, pulling down function coverage from 70% to 68.35%. Added 'e2e/' and 'playwright.config.ts' to coverage exclude list. Coverage now passing: - Functions: 70.66% (threshold 70%) - Branches: 90.54% (threshold 80%) - Statements: 60.82% (threshold 45%) - Lines: 60.82% (threshold 45%)
- Add environment variable E2E_TEST to switch base path between '/' (local) and '/task-timer/' (CI) - Update vite.config.ts to use conditional base path based on E2E_TEST - Update playwright.config.ts to set E2E_TEST env var for local tests - Update TaskTimerPage and persistence test to use conditional paths - Install @types/node for TypeScript process.env support - Configure webServer with proper env variables for local E2E testing This allows E2E tests to run successfully both locally (using / base path) and in CI (using /task-timer/ for GitHub Pages deployment).
Remove complex conditional logic that caused CI timeouts: - vite.config.ts: Always use /task-timer/ base path (no E2E_TEST env var) - playwright.config.ts: Remove conditional webServer config and env variables - TaskTimerPage.goto(): Always navigate to /task-timer/ (no process.env.CI checks) - persistence.spec.ts: Remove conditional path logic - e2e/README.md: Update docs to reflect simplified approach Benefits: - Tests run identically in local and CI environments - Configuration matches production deployment - Eliminates race conditions and timeout issues - Easier to debug and maintain - No environment variable juggling
Root cause: Page Object Model was calculating incorrect indices for time increments.
- Assumed 24-hour range starting at midnight (0-23)
- App actually displays 7am-6pm only (START_HOUR=7, 11 hours total)
- Tests trying to click hour 9 calculated index 36, but hour 9 is actually index 8
Solution: Use accessibility attributes instead of index calculations
- Calculate segment based on hour offset from START_HOUR (hour - 7)
- Use getByRole('button') with aria-label pattern matching
- Leverage existing aria-label on TimeIncrement components
- More robust and follows Playwright best practices
Benefits:
- All time tracking tests should now pass
- Edit tests should now pass (they create tasks with time)
- Uses semantic/accessible selectors (aria-labels)
- Self-documenting with clear segment calculations
- No more fragile nth() index lookups
This should fix ~21 failing E2E tests and reduce CI time from 30 minutes to ~3 minutes.
Bug: The regex 'Time segment N.*logged' would match both: - 'Time segment N, logged' ✓ - 'Time segment N, not logged' ✗ (incorrectly matched) Fix: Use 'Time segment N, logged$' to ensure we only match the logged state, not the 'not logged' state. The comma-space before 'logged' and the end anchor ensure precise matching.
Changed from getByRole with regex matching to simpler locator + nth() approach:
- Still fixes the core bug: calculate segment as (hour - START_HOUR) * 4 + offset
- More reliable: locator('[role="button"][aria-label*="Time segment"]').nth(segment)
- Avoids potential issues with getByRole after filter chains
- Same correctness, better compatibility
This hybrid approach:
✓ Uses accessible selectors (role, aria-label)
✓ Simple and predictable (.nth() instead of regex matching)
✓ Properly accounts for START_HOUR=7
Root issue: Complex aria-label selectors weren't working reliably after filter chains. Solution: Revert to simple, proven .increment class selector but keep the critical START_HOUR calculation fix: - segment = (hour - START_HOUR) * 4 + minuteSegment/15 - For 9am: segment = (9-7)*4 = 8 (correct index into 11-hour range) This combines: ✓ Simple, reliable selector (.increment class) ✓ Correct index calculation (accounts for 7am start) ✓ Proven assertion pattern (toHaveClass instead of aria attributes) Should fix all 21 failing time-tracking tests.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Implemented end-to-end functional testing framework to validate the full
user experience from a browser perspective. This complements existing unit
and component tests with real-world user interaction scenarios.
Infrastructure:
Test Coverage:
CI/CD Integration:
Scripts Added:
Documentation:
Note: Tests are configured for GitHub Pages path (/task-timer/) which
matches the CI environment. Local development requires changing the path
to / as documented in e2e/README.md.