Fix E2E test failures: rate limiting, auth persistence, test isolation, and app bug#755
Conversation
…n, and app bug
App fixes:
- Make rate limit configurable via RATE_LIMIT_MAX env var (was 100/15min, causing 429s during test runs)
- Fix login button accepting whitespace-only email (trim check)
Test fixes:
- Use sidebar navigation (client-side) instead of page.goto() to avoid full reloads losing auth state
- Wait for MuiAppBar after login to ensure auth state settles before proceeding
- Fix login redirect assertion to use waitForFunction for SPA navigation
- Target .MuiMenuItem-root for MUI dropdown selection to avoid backdrop intercept
- Use page.once('dialog') before delete clicks for proper dialog handling
- Fix isClientInTable to check only first column (td:first-child) to avoid description substring matches
- Fix delete work entry assertion to check specific client, not total row count
- Use unique emails/clients for scenarios needing data isolation (dashboard empty, reports totals, delete entry)
- Add networkidle waits after mutation operations for proper state refresh
All 53 scenarios now pass (was 36-38 failures).
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| 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) |
There was a problem hiding this comment.
🔴 parseInt of invalid RATE_LIMIT_MAX silently disables rate limiting
If RATE_LIMIT_MAX is set to a non-numeric string (e.g., "abc"), parseInt returns NaN, and since totalHits > NaN is always false in JavaScript, no request is ever rate-limited. Similarly, if set to "0", parseInt("0", 10) returns 0—and in express-rate-limit v7 (backend/package.json), max: 0 is documented to disable rate limiting entirely. Because "0" is truthy, the || '100' fallback doesn't activate. In both cases, rate limiting is silently disabled, removing a security control against brute-force or DoS attacks. There is no input validation or bounds checking on the parsed value.
| max: parseInt(process.env.RATE_LIMIT_MAX || '100', 10) | |
| max: Math.max(1, parseInt(process.env.RATE_LIMIT_MAX || '100', 10) || 100) |
Was this helpful? React with 👍 or 👎 to provide feedback.
Summary
Fixes all 36-38 E2E test failures across the Playwright + Cucumber BDD test suite, bringing the pass rate from ~32% to 100% (53/53 scenarios passing).
Root Causes & Fixes
App bug (1 fix):
backend/src/server.js): The 100 req/15min limit caused 429 errors mid-test-run. Made configurable viaRATE_LIMIT_MAXenv var (default still 100 for production safety).frontend/src/pages/LoginPage.tsx): Changed!emailto!email.trim().Test issues (multiple fixes):
page.goto()caused full page reloads, triggering/api/auth/mewhich failed under rate limiting. Fixed by using sidebar navigation (client-side SPA routing) instead of full reloads, and waiting forMuiAppBarafter login to confirm auth state has settled.waitForURLtowaitForFunctionfor SPA pushState navigation..MuiMenuItem-root:has-text()to avoid backdrop intercept.page.once('dialog')registered before the delete click.isClientInTable()usedtd:has-text()which matched description column substrings. Fixed totd:first-child:has-text().Review & Testing Checklist for Human
RATE_LIMIT_MAX=10000 node backend/src/server.jsandnpm run dev(frontend), thencd e2e && npx cucumber-js --config cucumber.jsto verify all 53 scenarios passRATE_LIMIT_MAXenv var defaults to 100 when not set (production behavior unchanged)Notes
RATE_LIMIT_MAXis explicitly set does the limit change..MuiDrawer-dockedtargets only the permanent (desktop) drawer, avoiding false matches with the hidden mobile drawer.Link to Devin session: https://partner-workshops.devinenterprise.com/sessions/02c26fc9532f4c64b7f66c46709fb063