Skip to content

Add Playwright TypeScript smoke tests for all major pages#753

Open
devin-ai-integration[bot] wants to merge 4 commits into
mainfrom
devin/1778603038-playwright-smoke-tests
Open

Add Playwright TypeScript smoke tests for all major pages#753
devin-ai-integration[bot] wants to merge 4 commits into
mainfrom
devin/1778603038-playwright-smoke-tests

Conversation

@devin-ai-integration
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot commented May 12, 2026

Summary

Adds a complete Playwright TypeScript E2E smoke test suite covering all major pages and workflows in the timesheet application.

Test coverage (28 tests):

  • Login (5 tests): form rendering, validation, successful login, post-login state
  • Dashboard (4 tests): stats cards, recent entries, quick actions, add entry button
  • Navigation (4 tests): sidebar navigation to all pages (Dashboard, Clients, Work Entries, Reports)
  • Clients (5 tests): page rendering, CRUD operations (create/delete)
  • Work Entries (4 tests): page rendering, dialog, create entry with client selection
  • Reports (4 tests): empty state, client selector, report data display
  • Logout (1 test): logout flow and redirect

Architecture:

  • Global auth setup via Playwright's storageState mechanism (login once, reuse across all tests)
  • Shared fixtures (auth.ts) with reusable helpers: login(), navigateViaSidebar(), createClientViaUI(), resetAndCreateClient(), deleteAllClients()
  • API-based cleanup ensures test isolation between suites
  • Configurable backend rate limit via RATE_LIMIT_MAX env var (set to 10000 during tests)

Backend change: One line in backend/src/server.js — makes the rate limit configurable via RATE_LIMIT_MAX environment variable (defaults to 100, unchanged for production).

Review & Testing Checklist for Human

  • Run cd e2e && npm install && npx playwright install chromium && npx playwright test to verify all 28 tests pass locally
  • Verify the RATE_LIMIT_MAX env var change in backend/src/server.js is acceptable (defaults to 100 if not set)

Notes

  • Tests run sequentially with a single worker to avoid database contention (in-memory SQLite)
  • The playwright.config.ts includes webServer config to auto-start both backend and frontend for CI
  • MUI Select interactions use .MuiSelect-select CSS selector since MUI doesn't expose proper aria-labelledby on native Selects

Link to Devin session: https://partner-workshops.devinenterprise.com/sessions/57f5105fe5cd4ac4affb71eb77cf3c2c


Open in Devin Review

- Set up Playwright with TypeScript in e2e/ directory
- Add global auth setup with storage state sharing
- Add smoke tests for: login, dashboard, navigation, clients, work entries, reports, logout
- 28 tests covering CRUD operations, navigation, and page rendering
- Make backend rate limit configurable via RATE_LIMIT_MAX env var for testing
- Add API-based cleanup helpers for test isolation
@devin-ai-integration
Copy link
Copy Markdown
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

Copy link
Copy Markdown
Contributor Author

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 potential issue.

View 2 additional findings in Devin Review.

Open in Devin Review

Comment thread backend/src/server.js
const limiter = rateLimit({
windowMs: 15 * 60 * 1000, // 15 minutes
max: 100 // limit each IP to 100 requests per windowMs
max: parseInt(process.env.RATE_LIMIT_MAX || '100', 10)
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Non-numeric RATE_LIMIT_MAX silently disables rate limiting

parseInt(process.env.RATE_LIMIT_MAX || '100', 10) returns NaN when RATE_LIMIT_MAX is set to a non-numeric string (e.g., "abc"). Since 'abc' is truthy, the || '100' fallback doesn't activate. In express-rate-limit, when max is NaN, the internal check hits > max is always false (any comparison with NaN yields false), so rate limiting is silently and completely disabled. This is a security-sensitive silent failure — a typo or misconfiguration in the environment variable would remove all rate-limit protection without any error or warning.

Suggested change
max: parseInt(process.env.RATE_LIMIT_MAX || '100', 10)
max: parseInt(process.env.RATE_LIMIT_MAX, 10) || 100
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

- Extract createClientViaUI() and resetAndCreateClient() into fixtures/auth.ts
- Use login() helper in login.spec.ts instead of duplicated login steps
- Use CLIENT_NAME constant in work-entries.spec.ts
@sonarqubecloud
Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants