Skip to content

feat: Add Playwright E2E test suite + fix backend rate limiting bug#754

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

feat: Add Playwright E2E test suite + fix backend rate limiting bug#754
devin-ai-integration[bot] wants to merge 4 commits into
mainfrom
devin/1778606320-playwright-e2e-tests

Conversation

@devin-ai-integration
Copy link
Copy Markdown
Contributor

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

Summary

Adds a comprehensive Playwright E2E test suite (23 tests across 5 spec files) covering all core user workflows, plus fixes two backend bugs discovered during testing.

Tests added:

  • Login flow (5 tests): valid/invalid credentials, button states, session persistence
  • Client management (4 tests): create, edit, delete, empty state
  • Work entry lifecycle (4 tests): create, verify in list, edit hours, delete
  • Reporting (3 tests): correct totals (3+5+2=10h), individual entries, empty state
  • Edge cases (7 tests): empty form submission, special characters (日本語, HTML tags, quotes), long text (500 chars), zero/negative hours, >24 hours, missing client

Bug fixes:

  1. Backend did not load .env file (missing dotenv dependency), so NODE_ENV was always undefined
  2. Rate limiter was too restrictive (100 req/15min) even in development mode — increased to 1000 for NODE_ENV=development

Review & Testing Checklist for Human

  • Run npx playwright test from the repo root with backend and frontend running to verify all 23 tests pass
  • Verify the dotenv addition in backend/src/server.js doesn't conflict with any existing env loading mechanism
  • Confirm rate limit of 1000 req/15min in development mode is acceptable for your use case

Notes

  • Tests use an in-memory SQLite database (fresh state on backend restart)
  • Each test is independent — uses unique client names with timestamps to avoid conflicts
  • Playwright config includes webServer entries to auto-start backend/frontend if not already running

Link to Devin session: https://partner-workshops.devinenterprise.com/sessions/9486b6bc87c2455da954d5a4f822be4a


Open in Devin Review

- Added 'dotenv' dependency to load .env configuration
- Rate limit increased to 1000 requests/15min in development mode
- Prevents test failures due to restrictive rate limiting
23 tests covering:
- Login flow (5 tests): valid/invalid credentials, button states, persistence
- Client management (4 tests): create, edit, delete, empty state
- Work entry lifecycle (4 tests): create, verify, edit hours, delete
- Reporting (3 tests): correct totals, individual entries, empty state
- Edge cases (7 tests): empty forms, special chars, long text, validation
@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: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 3 additional findings.

Open in Devin Review

- Added ensureClientExists, openWorkEntryDialog, createWorkEntry helpers
- Refactored work-entries, reports, and edge-cases tests to use shared helpers
- Reduces code duplication flagged by SonarCloud quality gate
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 new potential issue.

View 4 additional findings in Devin Review.

Open in Devin Review

Comment thread e2e/helpers.ts
await page.goto('/clients');
await page.getByRole('button', { name: 'Add Client' }).waitFor({ timeout: 5000 });
const tableText = await page.locator('table').textContent().catch(() => '');
if (tableText?.includes('No clients found')) {
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.

🟡 ensureClientExists checks for any client instead of the specific named client

The ensureClientExists helper at e2e/helpers.ts:29 checks tableText?.includes('No clients found') — it only creates the named client when zero clients exist. If the table already has other clients but not the one requested by name, the function silently skips creation. This breaks downstream callers like createWorkEntry (e2e/helpers.ts:46) which filters the dropdown by clientName via .filter({ hasText: clientName }). For example, e2e/03-work-entries.spec.ts:28 calls createWorkEntry(page, 'Work Entry Test Client', ...) after ensureClientExists(page, 'Work Entry Test Client') — if the database has leftover clients from other tests but not 'Work Entry Test Client', the ensureClientExists check passes (some clients exist), but createWorkEntry fails because the specific option isn't in the dropdown. The check should be !tableText?.includes(name) instead of tableText?.includes('No clients found').

Suggested change
if (tableText?.includes('No clients found')) {
if (!tableText?.includes(name)) {
Open in Devin Review

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

- Added fillAndSubmitClientForm helper for inline client creation
- Added selectClientAndFillHours local helper in edge cases
- Unified client name 'Validation Client' across edge case tests
@sonarqubecloud
Copy link
Copy Markdown

❌ The last analysis has failed.

See analysis details on SonarQube Cloud

@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