feat: enhance e2e tests #2242
feat: enhance e2e tests #2242
Conversation
WalkthroughActivate CI e2e services and concrete workflow steps; add Playwright scenarios and API-backed test flows; introduce modular MySQL and OpenSearch test utilities with guarded init/cleanup; replace monolithic setup/teardown hooks; relocate DB helpers; and make AuthService skip sending verification emails outside production while logging. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant GH as GitHub Actions
participant Compose as Docker Compose
participant Setup as global.setup
participant DB as DB utils
participant OS as OpenSearch utils
participant Runner as Playwright Runner
participant API as Backend (axios)
participant Browser as Browser UI
participant Teardown as global.teardown
GH->>Compose: Start services (mysql, opensearch, smtp)
GH->>Setup: Run setup steps (pnpm install, Playwright install)
Setup->>DB: initializeDatabaseForTest()
DB-->>Setup: DB ready / tables cleared
Setup->>OS: initializeOpenSearchForTest()
OS-->>Setup: OpenSearch ready / indices cleared
GH->>Runner: Run test list (import scenarios)
Runner->>API: POST seed data (axios)
API-->>Runner: Respond created
Runner->>Browser: Playwright navigates & interacts
Browser-->>Runner: Render / events
Runner-->>GH: Tests complete
GH->>Teardown: Run teardown hook
Teardown->>DB: cleanupDatabaseAfterTest()
Teardown->>OS: cleanupOpenSearchAfterTest()
DB-->>Teardown: DB cleanup done
OS-->>Teardown: OpenSearch cleanup done
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45–75 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
🧹 Nitpick comments (15)
apps/e2e/scenarios/create-project.spec.ts (1)
12-29: Replace fixed sleeps with state-based waits.Multiple
waitForTimeoutcalls (5s, 1.5s, …) will make this scenario flaky under slow CI or fast local runs. Prefer waiting for concrete signals—e.g.await expect(nextButton).toBeEnabled(),await page.waitForSelector(...), orawait page.waitForURL(...)—before proceeding. That also shortens the happy path when the UI responds quickly..github/workflows/e2e-test.yml (2)
35-37: Verify services are healthy before running tests.The workflow doesn't verify that services started successfully, which could lead to cryptic test failures if docker-compose fails.
Consider adding a health check step:
- name: Build and run run: | docker compose -f "./docker/docker-compose.e2e.yml" up -d + # Wait for services to be healthy + timeout 60 bash -c 'until docker compose -f "./docker/docker-compose.e2e.yml" ps | grep -q "healthy"; do sleep 2; done'
47-50: Add artifact upload for test failures and results reporting.Currently, test failures won't preserve Playwright traces, screenshots, or videos, making debugging difficult. Additionally, test results aren't reported in the GitHub UI.
Add these steps after the test run:
- name: Run e2e tests run: | pnpm build pnpm test:e2e - name: Upload test artifacts on failure uses: actions/upload-artifact@v4 if: failure() with: name: playwright-report path: apps/e2e/playwright-report/ retention-days: 7apps/e2e/global.teardown.ts (1)
6-24: Simplify error handling - outer try-catch is redundant.The outer try-catch block (lines 7-23) will never catch errors since both inner try-catch blocks handle all exceptions. Additionally, "Tearing down succeeds." logs even when cleanups fail.
Consider simplifying:
teardown('teardown', async () => { - try { - try { - await cleanupDatabaseAfterTest(); - } catch (error) { - console.warn('Database cleanup failed:', error); - } - - try { - await cleanupOpenSearchAfterTest(); - } catch (error) { - console.warn('OpenSearch cleanup failed:', error); - } - + let hasErrors = false; + + try { + await cleanupDatabaseAfterTest(); + } catch (error) { + console.warn('Database cleanup failed:', error); + hasErrors = true; + } + + try { + await cleanupOpenSearchAfterTest(); + } catch (error) { + console.warn('OpenSearch cleanup failed:', error); + hasErrors = true; + } + + if (hasErrors) { + console.log('Tearing down completed with errors.'); + } else { console.log('Tearing down succeeds.'); - } catch (e) { - console.log('Tearing down fails.', e); } });apps/e2e/scenarios/create-channel.spec.ts (1)
20-26: Remove unnecessary waitForTimeout calls.Playwright automatically waits for elements to be actionable before interacting. The explicit
waitForTimeout(500)calls add unnecessary delays and reduce test reliability.await page.getByText('Add Field').first().click(); - await page.waitForTimeout(500); await page.waitForSelector('input[name="key"]', { state: 'visible' }); await page.locator('input[name="key"]').click();Apply similar changes throughout the file (lines 29, 39, etc.).
apps/e2e/scenarios/create-feedback-with-image.spec.ts (1)
6-16: Add error handling to afterEach cleanup.The cleanup logic assumes the test data exists and will fail if the test itself failed before creating data. Consider adding guards or error handling.
test.afterEach(async ({ page }) => { + // Check if feedback exists before attempting cleanup + const feedbackExists = await page.locator('tbody').getByText('test text').count() > 0; + if (!feedbackExists) { + console.log('No test data to clean up'); + return; + } + await page.getByText('test text').first().click(); - await page.waitForTimeout(2000); await page.getByRole('button', { name: 'Delete' }).click();apps/e2e/scenarios/create-issue.spec.ts (2)
6-14: Clarify cleanup logic and magic numbers.The cleanup clicks "Delete" without first selecting an item, and uses
nth(3)without explanation. This makes the test harder to understand and maintain.Consider adding comments or extracting to a helper:
test.afterEach(async ({ page }) => { + // Delete the test issue created in the test await page.getByRole('button', { name: 'Delete' }).click(); + // Confirm deletion in the modal (4th "Delete" text on page) await expect(page.getByText('Delete').nth(3)).toBeVisible(); await page.getByText('Delete').nth(3).click();
65-75: Extract repeated popover selector to a helper.The complex locator pattern is repeated and could be extracted to improve maintainability.
+ // Helper to click popover trigger + const clickPopoverTrigger = (index: number) => + page.locator('button[data-slot="popover-trigger"]:has-text("+")').nth(index).click(); + - await page - .locator('button[data-slot="popover-trigger"]:has-text("+")') - .first() - .click(); + await clickPopoverTrigger(0); await page.getByText('test text').first().click(); - await page - .locator('button[data-slot="popover-trigger"]:has-text("+")') - .nth(1) - .click(); + await clickPopoverTrigger(1);apps/e2e/global.setup.ts (1)
9-27: Consider tracking initialization state for better error messages.While allowing tests to continue when initialization fails is good for resilience, tests that depend on these services will fail with unclear errors.
Consider exporting initialization state:
+export const initState = { + databaseReady: false, + openSearchReady: false, +}; + setup('tenant create and authenticate', async ({ page }) => { try { await initializeDatabaseForTest(); console.log('Database initialized for test'); + initState.databaseReady = true; } catch (error) { console.warn( 'Database initialization failed, continuing without database cleanup:', error, ); }apps/e2e/scenarios/search-feedback.spec.ts (1)
23-37: Extract duplicate API calls to a helper function.The two feedback creation calls are nearly identical and could be refactored to reduce duplication.
+ const createFeedback = async (message: string) => { + await axios.post( + `http://localhost:4000/api/projects/${projectId}/channels/${channelId}/feedbacks`, + { message }, + { headers: { 'x-api-key': 'MASTER_API_KEY' } }, + ); + }; + - await axios.post( - `http://localhost:4000/api/projects/${projectId}/channels/${channelId}/feedbacks`, - { - message: 'test text1', - }, - { headers: { 'x-api-key': 'MASTER_API_KEY' } }, - ); - - await axios.post( - `http://localhost:4000/api/projects/${projectId}/channels/${channelId}/feedbacks`, - { - message: 'test text2', - }, - { headers: { 'x-api-key': 'MASTER_API_KEY' } }, - ); + await createFeedback('test text1'); + await createFeedback('test text2');apps/e2e/scenarios/create-multiple-feedbacks.spec.ts (1)
12-12: Consider replacing hard waits with deterministic waiting strategies.Multiple
waitForTimeout(2000)calls make tests slower and potentially flaky. Playwright provides better alternatives like waiting for specific UI states or network requests.Example improvements:
// Instead of: await page.getByText('Delete Feedback').click(); await page.waitForTimeout(2000); // Use: await page.getByText('Delete Feedback').click(); await page.waitForLoadState('networkidle'); // or wait for a specific element to disappear/appear: await expect(page.getByText('Deleting...')).not.toBeVisible();Also applies to: 15-15, 20-20, 23-23, 58-58, 65-65, 68-68
apps/e2e/scenarios/create-feedback.spec.ts (1)
13-13: Consider replacing hard waits with deterministic waiting strategies.Same as in
create-multiple-feedbacks.spec.ts, prefer Playwright's built-in waiting mechanisms overwaitForTimeoutfor more reliable tests.Also applies to: 48-48, 60-60, 70-70
apps/e2e/utils/opensearch-utils.ts (3)
63-82: Validate response structure and consider error handling strategy.Two concerns:
Type assertion without validation (line 66): The response is cast to
Array<{ index: string }>without validating the structure. If the API response format changes, this could fail silently.Error swallowing (lines 79-81): Errors are logged but not rethrown. Consider whether cleanup failures should be silent or should fail the test.
Consider adding validation:
try { const response = await client.cat.indices({ format: 'json' }); - const indices = response.body as Array<{ index: string }>; + const rawIndices = response.body; + if (!Array.isArray(rawIndices)) { + throw new Error('Unexpected response format from OpenSearch cat.indices'); + } + const indices = rawIndices as Array<{ index: string }>; const channelIndices = indices .filter((index) => index.index.startsWith('channel_')) .map((index) => index.index);
84-99: Make OpenSearch endpoint configurable via environment variable.The hardcoded
localhost:9200limits flexibility for different test environments (CI, Docker, remote instances).Apply this diff:
export async function initializeOpenSearchForTest(): Promise<void> { const config: OpenSearchConfig = { - node: 'http://localhost:9200', + node: process.env.OPENSEARCH_URL || 'http://localhost:9200', }; const client = await createOpenSearchClient(config);
101-114: Make OpenSearch endpoint configurable via environment variable.Same as in
initializeOpenSearchForTest, the hardcoded endpoint should be configurable.Apply this diff:
export async function cleanupOpenSearchAfterTest(): Promise<void> { const config: OpenSearchConfig = { - node: 'http://localhost:9200', + node: process.env.OPENSEARCH_URL || 'http://localhost:9200', }; const client = await createOpenSearchClient(config);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (22)
.github/workflows/e2e-test.yml(1 hunks)apps/api/src/domains/admin/auth/auth.service.spec.ts(0 hunks)apps/api/src/domains/admin/auth/auth.service.ts(3 hunks)apps/e2e/database-utils.ts(0 hunks)apps/e2e/global.setup.ts(1 hunks)apps/e2e/global.teardown.ts(1 hunks)apps/e2e/package.json(1 hunks)apps/e2e/playwright.config.ts(5 hunks)apps/e2e/scenarios/create-channel.spec.ts(1 hunks)apps/e2e/scenarios/create-feedback-with-image.spec.ts(1 hunks)apps/e2e/scenarios/create-feedback.spec.ts(1 hunks)apps/e2e/scenarios/create-issue.spec.ts(1 hunks)apps/e2e/scenarios/create-multiple-feedbacks.spec.ts(1 hunks)apps/e2e/scenarios/create-project.spec.ts(1 hunks)apps/e2e/scenarios/no-database-seed/create-project.spec.ts(0 hunks)apps/e2e/scenarios/search-feedback.spec.ts(1 hunks)apps/e2e/scenarios/with-database-seed/create-channel.spec.ts(0 hunks)apps/e2e/scenarios/with-database-seed/create-feedback.spec.ts(0 hunks)apps/e2e/test.list.ts(1 hunks)apps/e2e/utils/database-utils.ts(1 hunks)apps/e2e/utils/opensearch-utils.ts(1 hunks)docker/docker-compose.e2e.yml(4 hunks)
💤 Files with no reviewable changes (5)
- apps/api/src/domains/admin/auth/auth.service.spec.ts
- apps/e2e/scenarios/with-database-seed/create-channel.spec.ts
- apps/e2e/scenarios/no-database-seed/create-project.spec.ts
- apps/e2e/database-utils.ts
- apps/e2e/scenarios/with-database-seed/create-feedback.spec.ts
🧰 Additional context used
🧬 Code graph analysis (8)
apps/e2e/scenarios/create-channel.spec.ts (2)
apps/e2e/scenarios/create-feedback.spec.ts (1)
test(5-73)apps/e2e/scenarios/create-project.spec.ts (1)
test(3-39)
apps/e2e/scenarios/create-project.spec.ts (1)
apps/e2e/scenarios/create-channel.spec.ts (1)
test(3-52)
apps/e2e/global.setup.ts (2)
apps/e2e/utils/database-utils.ts (1)
initializeDatabaseForTest(83-92)apps/e2e/utils/opensearch-utils.ts (1)
initializeOpenSearchForTest(84-99)
apps/e2e/scenarios/create-feedback.spec.ts (2)
apps/e2e/scenarios/search-feedback.spec.ts (1)
test(4-102)apps/e2e/scenarios/create-feedback-with-image.spec.ts (1)
test(4-59)
apps/e2e/test.list.ts (7)
apps/e2e/scenarios/create-issue.spec.ts (1)
test(4-87)apps/e2e/scenarios/search-feedback.spec.ts (1)
test(4-102)apps/e2e/scenarios/create-feedback.spec.ts (1)
test(5-73)apps/e2e/scenarios/create-multiple-feedbacks.spec.ts (1)
test(6-71)apps/e2e/scenarios/create-project.spec.ts (1)
test(3-39)apps/e2e/scenarios/create-channel.spec.ts (1)
test(3-52)apps/e2e/scenarios/create-feedback-with-image.spec.ts (1)
test(4-59)
apps/e2e/scenarios/create-feedback-with-image.spec.ts (2)
apps/e2e/scenarios/create-feedback.spec.ts (1)
test(5-73)apps/e2e/scenarios/create-channel.spec.ts (1)
test(3-52)
apps/e2e/global.teardown.ts (2)
apps/e2e/utils/database-utils.ts (1)
cleanupDatabaseAfterTest(94-101)apps/e2e/utils/opensearch-utils.ts (1)
cleanupOpenSearchAfterTest(101-114)
apps/e2e/scenarios/search-feedback.spec.ts (1)
apps/e2e/scenarios/create-feedback.spec.ts (1)
test(5-73)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: e2e-test
- GitHub Check: integration-test
🔇 Additional comments (8)
apps/api/src/domains/admin/auth/auth.service.ts (1)
22-22: LGTM! Standard NestJS logger setup.The Logger import and initialization follow NestJS best practices.
Also applies to: 72-72
apps/e2e/package.json (1)
10-14: Dependency addition looks correct.Pulling in
@opensearch-project/opensearchhere keeps the e2e workspace self-contained with the new OpenSearch helpers. Looks good.apps/e2e/playwright.config.ts (1)
87-108: Playwright web servers wired up nicely.The API/web
webServerentries now pass all the OpenSearch, SMTP, and DB env flags the scenarios rely on—thanks for keeping the e2e launcher self-contained.docker/docker-compose.e2e.yml (1)
41-42: OpenSearch env wiring makes sense.Mirroring the OpenSearch settings here keeps the compose stack aligned with the new test utilities; looks good.
apps/e2e/test.list.ts (1)
11-19: LGTM! Clean test orchestration.The modular structure with default-exported test functions is clean and maintainable. The test ordering appears intentional (project → channel → feedback flows).
However, ensure that test dependencies are intentional. If
createProject()orcreateChannel()fail, subsequent tests will likely fail. Consider whether tests should be truly independent or if this dependency chain is the desired behavior.apps/e2e/global.setup.ts (1)
42-45: Document that email verification is mocked in e2e environment.The 60-second timeout and fixed code '000000' suggest email verification is bypassed. This should be documented for clarity.
Ensure that the e2e environment is configured to bypass or mock email verification. Consider adding a comment:
await page.getByRole('button', { name: 'Request Code', exact: true }).click(); + // Wait for code input (email verification is bypassed in e2e with fixed code) await page.waitForSelector('input[name="code"]', { state: 'visible', timeout: 60000, });apps/e2e/scenarios/create-channel.spec.ts (1)
35-38: The review comment does not apply to E2E test code.E2E tests interact with implementation details, including hidden elements. Selecting
aria-hidden="true"elements is standard practice in E2E testing when testing components with custom styling that hide native controls. This pattern is intentional and necessary.The accessibility concern should be raised against the production code if the underlying UI component is inaccessible—not against the test that works around it. Since this is the only
selectOption()call in all E2E tests and no alternative selector was found, this approach is appropriate for testing the implementation as it exists.Likely an incorrect or invalid review comment.
apps/e2e/utils/opensearch-utils.ts (1)
41-61: LGTM: Well-implemented retry logic.The
waitForOpenSearchfunction properly implements a polling strategy with clear logging, appropriate error messages, and fails explicitly when retries are exhausted.
| await page.locator("input[name='siteName']").click(); | ||
| await page.locator("input[name='siteName']").fill("TestTenant"); | ||
| await page.getByRole("button", { name: "Next", exact: true }).click(); | ||
| await page.locator("input[name='siteName']").fill('TestTenant'); | ||
| await page.getByRole('button', { name: 'Next', exact: true }).click(); | ||
| await page.waitForTimeout(1000); | ||
|
|
||
| await page.locator("input[name='email']").click(); | ||
| await page.locator("input[name='email']").fill("[email protected]"); | ||
| await page.locator("input[name='email']").fill('[email protected]'); | ||
|
|
||
| await page.getByRole("button", { name: "Request Code", exact: true }).click(); | ||
| await page.getByRole('button', { name: 'Request Code', exact: true }).click(); | ||
|
|
||
| await page.waitForSelector('input[name="code"]', { state: "visible" }); | ||
| await page.waitForSelector('input[name="code"]', { | ||
| state: 'visible', | ||
| timeout: 60000, | ||
| }); | ||
| await page.locator("input[name='code']").click(); | ||
| await page.locator("input[name='code']").fill("000000"); | ||
| await page.locator("input[name='code']").fill('000000'); | ||
|
|
||
| await page.getByRole("button", { name: "Verify Code", exact: true }).click(); | ||
| await page.getByRole('button', { name: 'Verify Code', exact: true }).click(); | ||
|
|
||
| await page.locator("input[name='password']").click(); | ||
| await page.locator("input[name='password']").fill("12345678!"); | ||
| await page.locator("input[name='password']").fill('Abcd1234!'); | ||
|
|
||
| await page.locator("input[name='confirmPassword']").click(); | ||
| await page.locator("input[name='confirmPassword']").fill("12345678!"); | ||
| await page.locator("input[name='confirmPassword']").fill('Abcd1234!'); | ||
|
|
||
| await page.getByRole("button", { name: "Next", exact: true }).click(); | ||
| await page.getByRole('button', { name: 'Next', exact: true }).click(); | ||
| await page.waitForTimeout(1000); | ||
|
|
||
| await page.getByRole("button", { name: "Confirm", exact: true }).click(); | ||
| await page.getByRole('button', { name: 'Confirm', exact: true }).click(); |
There was a problem hiding this comment.
Replace hard-coded credentials with environment variables.
Hard-coded credentials and a fixed verification code ('000000') pose security risks and reduce test portability. The fixed code suggests email verification is bypassed in tests.
Refactor to use environment variables:
await page.locator("input[name='siteName']").click();
- await page.locator("input[name='siteName']").fill('TestTenant');
+ await page.locator("input[name='siteName']").fill(process.env.TEST_TENANT_NAME || 'TestTenant');
await page.locator("input[name='email']").click();
- await page.locator("input[name='email']").fill('[email protected]');
+ await page.locator("input[name='email']").fill(process.env.TEST_USER_EMAIL || '[email protected]');
await page.locator("input[name='code']").click();
- await page.locator("input[name='code']").fill('000000');
+ await page.locator("input[name='code']").fill(process.env.TEST_VERIFICATION_CODE || '000000');
await page.locator("input[name='password']").click();
- await page.locator("input[name='password']").fill('Abcd1234!');
+ await page.locator("input[name='password']").fill(process.env.TEST_USER_PASSWORD || 'Abcd1234!');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| await page.locator("input[name='siteName']").click(); | |
| await page.locator("input[name='siteName']").fill("TestTenant"); | |
| await page.getByRole("button", { name: "Next", exact: true }).click(); | |
| await page.locator("input[name='siteName']").fill('TestTenant'); | |
| await page.getByRole('button', { name: 'Next', exact: true }).click(); | |
| await page.waitForTimeout(1000); | |
| await page.locator("input[name='email']").click(); | |
| await page.locator("input[name='email']").fill("[email protected]"); | |
| await page.locator("input[name='email']").fill('[email protected]'); | |
| await page.getByRole("button", { name: "Request Code", exact: true }).click(); | |
| await page.getByRole('button', { name: 'Request Code', exact: true }).click(); | |
| await page.waitForSelector('input[name="code"]', { state: "visible" }); | |
| await page.waitForSelector('input[name="code"]', { | |
| state: 'visible', | |
| timeout: 60000, | |
| }); | |
| await page.locator("input[name='code']").click(); | |
| await page.locator("input[name='code']").fill("000000"); | |
| await page.locator("input[name='code']").fill('000000'); | |
| await page.getByRole("button", { name: "Verify Code", exact: true }).click(); | |
| await page.getByRole('button', { name: 'Verify Code', exact: true }).click(); | |
| await page.locator("input[name='password']").click(); | |
| await page.locator("input[name='password']").fill("12345678!"); | |
| await page.locator("input[name='password']").fill('Abcd1234!'); | |
| await page.locator("input[name='confirmPassword']").click(); | |
| await page.locator("input[name='confirmPassword']").fill("12345678!"); | |
| await page.locator("input[name='confirmPassword']").fill('Abcd1234!'); | |
| await page.getByRole("button", { name: "Next", exact: true }).click(); | |
| await page.getByRole('button', { name: 'Next', exact: true }).click(); | |
| await page.waitForTimeout(1000); | |
| await page.getByRole("button", { name: "Confirm", exact: true }).click(); | |
| await page.getByRole('button', { name: 'Confirm', exact: true }).click(); | |
| await page.locator("input[name='siteName']").click(); | |
| await page.locator("input[name='siteName']").fill(process.env.TEST_TENANT_NAME || 'TestTenant'); | |
| await page.getByRole('button', { name: 'Next', exact: true }).click(); | |
| await page.waitForTimeout(1000); | |
| await page.locator("input[name='email']").click(); | |
| await page.locator("input[name='email']").fill(process.env.TEST_USER_EMAIL || '[email protected]'); | |
| await page.getByRole('button', { name: 'Request Code', exact: true }).click(); | |
| await page.waitForSelector('input[name="code"]', { | |
| state: 'visible', | |
| timeout: 60000, | |
| }); | |
| await page.locator("input[name='code']").click(); | |
| await page.locator("input[name='code']").fill(process.env.TEST_VERIFICATION_CODE || '000000'); | |
| await page.getByRole('button', { name: 'Verify Code', exact: true }).click(); | |
| await page.locator("input[name='password']").click(); | |
| await page.locator("input[name='password']").fill(process.env.TEST_USER_PASSWORD || 'Abcd1234!'); | |
| await page.locator("input[name='confirmPassword']").click(); | |
| await page.locator("input[name='confirmPassword']").fill('Abcd1234!'); | |
| await page.getByRole('button', { name: 'Next', exact: true }).click(); | |
| await page.waitForTimeout(1000); | |
| await page.getByRole('button', { name: 'Confirm', exact: true }).click(); |
| await page.goto('http://localhost:3000', { | ||
| waitUntil: 'domcontentloaded', | ||
| }); | ||
| await page.waitForLoadState('domcontentloaded'); | ||
| await page.waitForTimeout(5000); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Replace hard-coded URLs with environment variables and remove redundant waits.
Hard-coded URLs make tests less portable. The double wait (waitForLoadState + waitForTimeout) is redundant—Playwright auto-waits for most actions.
Apply this approach:
- await page.goto('http://localhost:3000', {
+ await page.goto(process.env.BASE_URL || 'http://localhost:3000', {
waitUntil: 'domcontentloaded',
});
- await page.waitForLoadState('domcontentloaded');
- await page.waitForTimeout(5000);
+ // Playwright auto-waits for elements to be actionable📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| await page.goto('http://localhost:3000', { | |
| waitUntil: 'domcontentloaded', | |
| }); | |
| await page.waitForLoadState('domcontentloaded'); | |
| await page.waitForTimeout(5000); | |
| await page.goto(process.env.BASE_URL || 'http://localhost:3000', { | |
| waitUntil: 'domcontentloaded', | |
| }); | |
| // Playwright auto-waits for elements to be actionable |
🤖 Prompt for AI Agents
In apps/e2e/scenarios/create-channel.spec.ts around lines 6 to 10, replace the
hard-coded 'http://localhost:3000' with a configurable environment variable
(e.g. process.env.E2E_BASE_URL or use Playwright test config baseURL) and remove
the redundant waits; keep a single reliable wait strategy (rely on page.goto's
auto-wait or use waitUntil: 'networkidle' if needed) and delete the explicit
page.waitForLoadState and page.waitForTimeout calls.
| await axios.post( | ||
| `http://localhost:4000/api/projects/${projectId}/channels/${channelId}/feedbacks`, | ||
| { | ||
| message: 'test text', | ||
| image: [ | ||
| 'https://lps-editor-2050.landpress.line.me/logo/logo_line_color.svg', | ||
| ], | ||
| }, | ||
| { headers: { 'x-api-key': 'MASTER_API_KEY' } }, | ||
| ); |
There was a problem hiding this comment.
Move hard-coded credentials and URLs to environment variables.
Hard-coding API keys and endpoints is a security risk and makes tests less portable. The external image URL creates a dependency on an external service.
Apply this refactor:
await axios.post(
- `http://localhost:4000/api/projects/${projectId}/channels/${channelId}/feedbacks`,
+ `${process.env.API_BASE_URL || 'http://localhost:4000'}/api/projects/${projectId}/channels/${channelId}/feedbacks`,
{
message: 'test text',
image: [
- 'https://lps-editor-2050.landpress.line.me/logo/logo_line_color.svg',
+ process.env.TEST_IMAGE_URL || 'https://via.placeholder.com/150',
],
},
- { headers: { 'x-api-key': 'MASTER_API_KEY' } },
+ { headers: { 'x-api-key': process.env.MASTER_API_KEY } },
);🤖 Prompt for AI Agents
In apps/e2e/scenarios/create-feedback-with-image.spec.ts around lines 35 to 44,
the test hard-codes the API key and external image URL; replace those with
environment variables and a local test asset or mocked endpoint: read the API
key from process.env.MASTER_API_KEY (with a sensible fallback for CI), read the
base API URL from process.env.TEST_BASE_URL (or build from localhost:4000 if
unset), and replace the external image URL with either a project-local test
image served by the test server or a mock HTTP handler so the test does not
depend on an external service; update the axios call to use the env vars and the
local/mocked image reference.
| const url = new URL(page.url()); | ||
| const pathname = url.pathname; | ||
| const segments = pathname.split('/'); | ||
| const projectId = segments[3]; | ||
| const params = new URLSearchParams(url.search); | ||
| const channelId = params.get('channelId'); |
There was a problem hiding this comment.
Add validation for extracted URL parameters.
The code assumes projectId (from segments[3]) and channelId (from query params) are always present. If the URL structure is unexpected, these could be undefined, leading to unhelpful error messages in subsequent API calls.
Apply this diff to add validation:
const url = new URL(page.url());
const pathname = url.pathname;
const segments = pathname.split('/');
const projectId = segments[3];
const params = new URLSearchParams(url.search);
const channelId = params.get('channelId');
+
+ if (!projectId || !channelId) {
+ throw new Error(`Invalid URL structure: projectId=${projectId}, channelId=${channelId}`);
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const url = new URL(page.url()); | |
| const pathname = url.pathname; | |
| const segments = pathname.split('/'); | |
| const projectId = segments[3]; | |
| const params = new URLSearchParams(url.search); | |
| const channelId = params.get('channelId'); | |
| const url = new URL(page.url()); | |
| const pathname = url.pathname; | |
| const segments = pathname.split('/'); | |
| const projectId = segments[3]; | |
| const params = new URLSearchParams(url.search); | |
| const channelId = params.get('channelId'); | |
| if (!projectId || !channelId) { | |
| throw new Error(`Invalid URL structure: projectId=${projectId}, channelId=${channelId}`); | |
| } |
🤖 Prompt for AI Agents
In apps/e2e/scenarios/create-multiple-feedbacks.spec.ts around lines 36 to 41,
the code reads projectId from pathname segments[3] and channelId from
URLSearchParams without checks; add validation after extraction to ensure
projectId and channelId are non-empty strings (not undefined/null), and if
either is missing throw or fail the test with a clear error message (e.g.,
`throw new Error("Missing projectId from URL: ...")` or use the test framework's
assert/expect to fail) so downstream API calls get meaningful diagnostics.
| for (let i = 0; i < NUMBER_OF_FEEDBACKS; i++) { | ||
| await axios.post( | ||
| `http://localhost:4000/api/projects/${projectId}/channels/${channelId}/feedbacks`, | ||
| { | ||
| message: `test text ${i}`, | ||
| }, | ||
| { headers: { 'x-api-key': 'MASTER_API_KEY' } }, | ||
| ); | ||
| } |
There was a problem hiding this comment.
Security and reliability concerns in data seeding.
Issues identified:
- Security: Hardcoded API key
'MASTER_API_KEY'should be loaded from environment variables - Reliability: No error handling for axios calls - if any POST fails, the test will silently have incomplete data
- Configuration: Hardcoded
localhost:4000should be configurable
Apply these changes:
+ const API_BASE_URL = process.env.API_BASE_URL || 'http://localhost:4000';
+ const API_KEY = process.env.E2E_API_KEY || 'MASTER_API_KEY';
+
for (let i = 0; i < NUMBER_OF_FEEDBACKS; i++) {
- await axios.post(
- `http://localhost:4000/api/projects/${projectId}/channels/${channelId}/feedbacks`,
- {
- message: `test text ${i}`,
- },
- { headers: { 'x-api-key': 'MASTER_API_KEY' } },
- );
+ try {
+ await axios.post(
+ `${API_BASE_URL}/api/projects/${projectId}/channels/${channelId}/feedbacks`,
+ {
+ message: `test text ${i}`,
+ },
+ { headers: { 'x-api-key': API_KEY } },
+ );
+ } catch (error) {
+ throw new Error(`Failed to create feedback ${i}: ${error.message}`);
+ }
}| test.describe('search-feedback suite', () => { | ||
| test('creating and searching a feedback succeed', async ({ page }) => { |
There was a problem hiding this comment.
Add afterEach cleanup to prevent test data pollution.
Unlike other test suites (create-feedback, create-feedback-with-image, create-issue), this test doesn't clean up the created feedbacks, which could cause test pollution or flakiness.
Add cleanup similar to other tests:
test.describe('search-feedback suite', () => {
+ test.afterEach(async ({ page }) => {
+ // Clean up test feedbacks
+ await page.getByRole('checkbox').nth(1).click();
+ await page.getByText('Delete Feedback').click();
+ await page.waitForTimeout(2000);
+ await page.getByRole('button', { name: 'Delete' }).click();
+ await page.waitForTimeout(2000);
+
+ await page.getByRole('checkbox').first().click();
+ await page.getByText('Delete Feedback').click();
+ await page.waitForTimeout(2000);
+ await page.getByRole('button', { name: 'Delete' }).click();
+ await page.waitForTimeout(2000);
+ });
+
test('creating and searching a feedback succeed', async ({ page }) => {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| test.describe('search-feedback suite', () => { | |
| test('creating and searching a feedback succeed', async ({ page }) => { | |
| test.describe('search-feedback suite', () => { | |
| test.afterEach(async ({ page }) => { | |
| // Clean up test feedbacks | |
| await page.getByRole('checkbox').nth(1).click(); | |
| await page.getByText('Delete Feedback').click(); | |
| await page.waitForTimeout(2000); | |
| await page.getByRole('button', { name: 'Delete' }).click(); | |
| await page.waitForTimeout(2000); | |
| await page.getByRole('checkbox').first().click(); | |
| await page.getByText('Delete Feedback').click(); | |
| await page.waitForTimeout(2000); | |
| await page.getByRole('button', { name: 'Delete' }).click(); | |
| await page.waitForTimeout(2000); | |
| }); | |
| test('creating and searching a feedback succeed', async ({ page }) => { |
🤖 Prompt for AI Agents
In apps/e2e/scenarios/search-feedback.spec.ts around lines 5-6, this test suite
lacks an afterEach cleanup which lets created feedbacks remain and pollute other
tests; add an afterEach hook that tracks created feedback IDs during tests
(e.g., push returned IDs into an array) and, after each test, iterates that
array to delete those feedbacks via the same API/DB helper used in other suites,
awaiting each delete and clearing the array; ensure the afterEach handles errors
gracefully so cleanup doesn't mask test failures.
| for (let i = 0; i < maxRetries; i++) { | ||
| try { | ||
| const connection = await createConnection(); | ||
| await connection.ping(); | ||
| await connection.end(); | ||
| console.log('Database is ready'); | ||
| return; | ||
| } catch (error) { | ||
| console.log(`Waiting for database... (${i + 1}/${maxRetries})`); | ||
| if (i === maxRetries - 1) { | ||
| throw new Error( | ||
| `Database is not available after ${maxRetries} retries`, | ||
| ); | ||
| } | ||
| await new Promise((resolve) => setTimeout(resolve, delayMs)); | ||
| } |
There was a problem hiding this comment.
Close failed connection attempts to avoid leaking sockets.
When createConnection() succeeds but ping() fails, the retry path never reaches connection.end(), so every iteration leaves an open socket behind. A few dozen retries is enough to exhaust MySQL’s connection limit and block the API from starting. Please ensure each attempt releases the connection in a finally block.
Apply this diff to guard the loop:
for (let i = 0; i < maxRetries; i++) {
- try {
- const connection = await createConnection();
- await connection.ping();
- await connection.end();
- console.log('Database is ready');
- return;
- } catch (error) {
+ let connection: mysql.Connection | undefined;
+ try {
+ connection = await createConnection();
+ await connection.ping();
+ console.log('Database is ready');
+ return;
+ } catch (error) {
console.log(`Waiting for database... (${i + 1}/${maxRetries})`);
if (i === maxRetries - 1) {
throw new Error(
`Database is not available after ${maxRetries} retries`,
);
}
await new Promise((resolve) => setTimeout(resolve, delayMs));
+ } finally {
+ if (connection) {
+ try {
+ await connection.end();
+ } catch {
+ connection.destroy();
+ }
+ }
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| for (let i = 0; i < maxRetries; i++) { | |
| try { | |
| const connection = await createConnection(); | |
| await connection.ping(); | |
| await connection.end(); | |
| console.log('Database is ready'); | |
| return; | |
| } catch (error) { | |
| console.log(`Waiting for database... (${i + 1}/${maxRetries})`); | |
| if (i === maxRetries - 1) { | |
| throw new Error( | |
| `Database is not available after ${maxRetries} retries`, | |
| ); | |
| } | |
| await new Promise((resolve) => setTimeout(resolve, delayMs)); | |
| } | |
| for (let i = 0; i < maxRetries; i++) { | |
| let connection: mysql.Connection | undefined; | |
| try { | |
| connection = await createConnection(); | |
| await connection.ping(); | |
| console.log('Database is ready'); | |
| return; | |
| } catch (error) { | |
| console.log(`Waiting for database... (${i + 1}/${maxRetries})`); | |
| if (i === maxRetries - 1) { | |
| throw new Error( | |
| `Database is not available after ${maxRetries} retries`, | |
| ); | |
| } | |
| await new Promise((resolve) => setTimeout(resolve, delayMs)); | |
| } finally { | |
| if (connection) { | |
| try { | |
| await connection.end(); | |
| } catch { | |
| connection.destroy(); | |
| } | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In apps/e2e/utils/database-utils.ts around lines 32 to 47, the loop can leak
sockets when createConnection() succeeds but ping() throws because
connection.end() is only called on the success path; modify the try/catch so you
declare the connection variable outside the try, then use a finally block that
checks whether connection is defined and, if so, awaits connection.end() (wrap
the end call in its own try/catch to swallow or log any close errors) to ensure
every attempted connection is closed before the next retry.
| export async function createOpenSearchClient( | ||
| config: OpenSearchConfig, | ||
| ): Promise<Client> { | ||
| const clientConfig: any = { | ||
| node: config.node, | ||
| }; | ||
|
|
||
| if (config.username && config.password) { | ||
| clientConfig.auth = { | ||
| username: config.username, | ||
| password: config.password, | ||
| }; | ||
| } | ||
|
|
||
| return new Client(clientConfig); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Replace any type with proper OpenSearch client configuration type.
Line 27 uses any type for clientConfig, which defeats TypeScript's type safety. The @opensearch-project/opensearch library exports proper configuration types.
Apply this diff:
+import { Client, ClientOptions } from '@opensearch-project/opensearch';
+
export async function createOpenSearchClient(
config: OpenSearchConfig,
): Promise<Client> {
- const clientConfig: any = {
+ const clientConfig: ClientOptions = {
node: config.node,
};
if (config.username && config.password) {
clientConfig.auth = {
username: config.username,
password: config.password,
};
}
return new Client(clientConfig);
}🤖 Prompt for AI Agents
In apps/e2e/utils/opensearch-utils.ts around lines 24 to 39, replace the use of
the `any` type for clientConfig with the proper OpenSearch client configuration
type from the library: import and use the exported ClientOptions (or the correct
config type) from '@opensearch-project/opensearch' and type clientConfig as that
type; ensure the auth block matches the library's expected shape and keep the
return type Promise<Client> unchanged.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.github/workflows/e2e-test.yml (1)
28-31: OpenSearch is missing required environment configuration.This matches the prior review concern. OpenSearch requires explicit configuration to run in local/test mode with security disabled and single-node discovery enabled.
Apply this diff to configure OpenSearch for local testing:
opensearch: image: opensearchproject/opensearch:2.4.1 + env: + OPENSEARCH_JAVA_OPTS: "-Xms512m -Xmx512m" + discovery.type: single-node + plugins.security.disabled: true ports: - 9200:9200
🧹 Nitpick comments (1)
.github/workflows/e2e-test.yml (1)
22-27: Consider adding health checks for SMTP and OpenSearch services.MySQL includes health checks for reliability, but SMTP and OpenSearch do not. While optional, this improves the robustness of the workflow by ensuring services are fully ready before tests run.
smtp: image: rnwood/smtp4dev:v3 + options: --health-cmd="wget --no-verbose --tries=1 --spider http://localhost/api/health || exit 1" --health-interval=10s --health-timeout=5s --health-retries=3 ports: - 5080:80 - 25:25 - 143:143 opensearch: image: opensearchproject/opensearch:2.4.1 ports: - 9200:9200 + options: --health-cmd="curl -s http://localhost:9200 > /dev/null || exit 1" --health-interval=10s --health-timeout=5s --health-retries=3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/e2e-test.yml(1 hunks)docker/docker-compose.e2e.yml(3 hunks)
✅ Files skipped from review due to trivial changes (1)
- docker/docker-compose.e2e.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: e2e-test
- GitHub Check: integration-test
🔇 Additional comments (1)
.github/workflows/e2e-test.yml (1)
36-42: Verify working directory consistency between setup and run steps.The setup step changes to
apps/e2eto install dependencies, but the subsequent run step does not specify a working directory and will execute from the repository root. Verify thatpnpm buildandpnpm test:e2eare intended to run from the root (likely they should) and that the Playwright installation from theapps/e2esubdirectory is discoverable.If the run step needs to execute in
apps/e2e, add aworking-directory: apps/e2eto that step.
| services: | ||
| mysql: | ||
| image: mysql:8.0.39 | ||
| env: | ||
| MYSQL_ROOT_PASSWORD: userfeedback | ||
| MYSQL_DATABASE: e2e | ||
| MYSQL_USER: userfeedback | ||
| MYSQL_PASSWORD: userfeedback | ||
| TZ: UTC | ||
| ports: | ||
| - 13307:3306 | ||
| options: --health-cmd="mysqladmin ping" --health-interval=10s --health-timeout=5s --health-retries=3 | ||
| smtp: | ||
| image: rnwood/smtp4dev:v3 | ||
| ports: | ||
| - 5080:80 | ||
| - 25:25 | ||
| - 143:143 | ||
| opensearch: | ||
| image: opensearchproject/opensearch:2.4.1 | ||
| ports: | ||
| - 9200:9200 |
There was a problem hiding this comment.
Services are not connected to the application via environment variables.
The workflow defines MySQL, SMTP, and OpenSearch services, but doesn't configure environment variables for the application to discover and connect to them. The e2e test environment needs to know how to reach these services (e.g., database URL, SMTP host, search engine endpoint).
This is a critical gap that will likely cause tests to fail if they attempt to use these services.
Add an env: block at the job level (after runs-on:) to configure service connectivity:
runs-on: ubuntu-latest
+ env:
+ DATABASE_URL: "mysql://userfeedback:userfeedback@localhost:13307/e2e"
+ OPENSEARCH_URL: "http://localhost:9200"
+ SMTP_HOST: "localhost"
+ SMTP_PORT: "25"
services:Adjust the values to match your application's expected environment variable names and service configuration.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
.github/workflows/e2e-test.yml around lines 10 to 31: the job defines mysql,
smtp and opensearch services but doesn't expose any environment variables for
the application to discover them; add an env: block immediately after the job's
runs-on: line that sets the app's expected connection vars (for example
DATABASE_URL or MYSQL_HOST=127.0.0.1 and MYSQL_PORT=13307 plus MYSQL_USER and
MYSQL_PASSWORD, SMTP_HOST=127.0.0.1 and SMTP_PORT=25 or HTTP_PORT=5080 depending
on how your app connects, and OPENSEARCH_URL=http://127.0.0.1:9200) — adjust the
variable names/values to match your app’s expected env names and ports so the
e2e tests can connect to the services.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/e2e/playwright.config.ts (1)
47-47: Considervideo: 'retain-on-failure'to reduce disk usage.Currently, videos are recorded for all tests. For e2e test suites that run frequently, consider changing to
'retain-on-failure'to save disk space while still capturing videos for debugging failed tests.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/e2e/playwright.config.ts(5 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: integration-test
- GitHub Check: e2e-test
🔇 Additional comments (5)
apps/e2e/playwright.config.ts (5)
19-26: LGTM: Timeout increases are appropriate for e2e tests.The 60-second timeouts for both overall test execution and expect assertions are reasonable for end-to-end tests, which typically involve real browser interactions, network requests, and database operations.
53-65: LGTM: Project configuration follows Playwright best practices.The setup/teardown pattern with dependencies ensures proper test initialization and cleanup. The authenticated state management using
STORAGE_STATEis correctly configured.
91-91: Hardcoded JWT_SECRET is acceptable for test environment.The hardcoded value is appropriate given
NODE_ENV: 'test'on line 99. Ensure this value is never used in production environments.
111-117: LGTM: Web server configuration is correct.The Next.js web server is properly configured to communicate with the API server on port 4000.
104-107: Verify empty OpenSearch credentials locally with your OpenSearch instance.The connectivity test failed (status code 000), which is expected in a sandbox environment without running services. Verify that your local OpenSearch instance (at
http://localhost:9200) does not require authentication, or update the emptyOPENSEARCH_USERNAMEandOPENSEARCH_PASSWORDif credentials are needed.Test locally using:
curl -s -o /dev/null -w "%{http_code}" http://localhost:9200Expected: 200 (no auth) or 401 (auth required).
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/e2e/scenarios/create-channel.spec.ts (1)
6-10: Hard-coded URL and redundant waits already flagged.This issue was previously identified in past reviews. The hard-coded URL should use environment variables and the redundant wait pattern should be simplified.
🧹 Nitpick comments (3)
apps/e2e/scenarios/create-channel.spec.ts (3)
10-46: Reduce reliance on arbitrary timeouts.The test contains 8
waitForTimeoutcalls with arbitrary durations (30s, 1s, 500ms, 1.5s). Playwright's auto-waiting usually eliminates the need for explicit timeouts, making tests faster and more reliable.Replace fixed timeouts with state-based waits or rely on auto-waiting:
- await page.waitForLoadState('domcontentloaded'); - await page.waitForTimeout(30000); - await page.getByRole('radio', { name: 'Settings' }).click(); - await page.waitForTimeout(1000); await page.getByText('Create Channel').click(); - await page.waitForTimeout(1000);Most Playwright actions (click, fill, etc.) automatically wait for elements to be actionable. For genuine async operations, prefer
waitForSelectorwith specific state checks orwaitForResponsefor network calls.
20-41: Consider extracting field creation into a helper.The pattern of clicking "Add Field", filling the key, and confirming is repeated twice with minor variations. A helper function could improve readability and maintainability.
Example refactor:
async function addField( page: Page, key: string, options?: { selectValue?: string } ) { await page.getByText('Add Field').first().click(); await page.waitForTimeout(500); await page.waitForSelector('input[name="key"]', { state: 'visible' }); await page.locator('input[name="key"]').click(); await page.locator('input[name="key"]').fill(key); if (options?.selectValue) { await page .locator('select[aria-hidden="true"]') .first() .selectOption(options.selectValue); await page.waitForTimeout(500); } await page.getByRole('button', { name: 'Confirm' }).click(); } // Usage: await addField(page, 'message'); await addField(page, 'image', { selectValue: 'images' });
35-38: Refactor selector to follow Radix UI testing best practices.The
aria-hidden="true"selector works but targets Radix UI's hidden native<select>element, which is an implementation detail. Radix UI best practices recommend using role-based locators (combobox → listbox → option) or getByTestId for a stable contract, avoiding long CSS/XPath chains. Consider:
- Using
page.getByRole('combobox')to find the visible trigger, then click to open- Using
page.getByRole('option', { name: 'images' })to select the visible option- Or adding a
data-testidattribute to the Radix trigger for deterministic test selectorsThis makes tests more resilient to UI library changes and mirrors actual user interaction patterns.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/e2e/scenarios/create-channel.spec.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
apps/e2e/scenarios/create-channel.spec.ts (2)
apps/e2e/scenarios/create-project.spec.ts (1)
test(3-39)apps/e2e/scenarios/create-feedback-with-image.spec.ts (1)
test(4-59)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: e2e-test
- GitHub Check: integration-test
| test('creating a channel succeeds', async ({ page }) => { | ||
| await page.goto('http://localhost:3000', { | ||
| waitUntil: 'domcontentloaded', | ||
| }); | ||
| await page.waitForLoadState('domcontentloaded'); | ||
| await page.waitForTimeout(30000); | ||
|
|
||
| await page.getByRole('radio', { name: 'Settings' }).click(); | ||
| await page.waitForTimeout(1000); | ||
| await page.getByText('Create Channel').click(); | ||
| await page.waitForTimeout(1000); | ||
| await page.locator('input[name="name"]').click(); | ||
| await page.locator('input[name="name"]').fill('TestChannel'); | ||
| await page.getByRole('button', { name: 'Next' }).click(); | ||
|
|
||
| await page.getByText('Add Field').first().click(); | ||
| await page.waitForTimeout(500); | ||
|
|
||
| await page.waitForSelector('input[name="key"]', { state: 'visible' }); | ||
| await page.locator('input[name="key"]').click(); | ||
| await page.locator('input[name="key"]').fill('message'); | ||
| await page.getByRole('button', { name: 'Confirm' }).click(); | ||
|
|
||
| await page.getByText('Add Field').first().click(); | ||
| await page.waitForTimeout(500); | ||
|
|
||
| await page.waitForSelector('input[name="key"]', { state: 'visible' }); | ||
| await page.locator('input[name="key"]').click(); | ||
| await page.locator('input[name="key"]').fill('image'); | ||
|
|
||
| await page | ||
| .locator('select[aria-hidden="true"]') | ||
| .first() | ||
| .selectOption('images'); | ||
| await page.waitForTimeout(500); | ||
|
|
||
| await page.getByRole('button', { name: 'Confirm' }).click(); | ||
|
|
||
| await page.getByRole('button', { name: 'Complete' }).click(); | ||
| await page.waitForTimeout(1500); | ||
| await page.getByRole('button', { name: 'Start' }).click(); | ||
| await page.waitForTimeout(1500); | ||
| await expect( | ||
| page.getByRole('tab', { name: 'TestChannel' }).first(), | ||
| ).toContainText('TestChannel'); | ||
| }); |
There was a problem hiding this comment.
Add cleanup to delete test channel after execution.
The test creates "TestChannel" but never deletes it. Without cleanup, re-running the test may fail due to duplicate names, and test data pollution can affect other tests.
Add an afterEach hook to clean up the test channel:
test.describe('create-channel suite', () => {
+ test.afterEach(async ({ page }) => {
+ // Navigate to Settings and delete TestChannel
+ await page.getByRole('radio', { name: 'Settings' }).click();
+ await page.waitForTimeout(1000);
+ // Add deletion logic based on your UI
+ });
+
test('creating a channel succeeds', async ({ page }) => {Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/e2e/scenarios/create-channel.spec.ts around lines 5 to 50, the test
creates a persistent "TestChannel" but never deletes it, causing duplicate-name
failures and test pollution; add an afterEach hook that runs after the test to
remove the channel (preferably via the app's API or an authenticated HTTP
request for speed and reliability, falling back to UI navigation if no API
exists), locate the channel by name ("TestChannel"), perform a delete operation
with proper waits and retries to be idempotent, and ensure the hook handles
not-found responses gracefully so repeated or failed runs do not error.
There was a problem hiding this comment.
Actionable comments posted: 5
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/e2e/scenarios/create-issue.spec.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: integration-test
- GitHub Check: e2e-test
| test.afterEach(async ({ page }) => { | ||
| await page.waitForTimeout(10000); | ||
| await page.getByRole('button', { name: 'Delete' }).click(); | ||
|
|
||
| await expect(page.getByText('Delete').nth(3)).toBeVisible(); | ||
| await page.getByText('Delete').nth(3).click(); | ||
| await page.waitForTimeout(2000); | ||
|
|
||
| await expect(page.locator('tbody')).not.toContainText('test text'); | ||
| }); |
There was a problem hiding this comment.
Replace arbitrary timeouts with deterministic waits and clarify selector logic.
The cleanup logic has several issues:
- Line 7: The 10-second
waitForTimeoutwill significantly slow down the test suite and is non-deterministic. - Lines 10-11: Using
.nth(3)is fragile and will break if the DOM structure changes. It's unclear why the 3rd "Delete" element is targeted. - Line 12: Another arbitrary 2-second wait compounds flakiness.
Replace with deterministic waits based on element visibility or state changes:
test.afterEach(async ({ page }) => {
- await page.waitForTimeout(10000);
await page.getByRole('button', { name: 'Delete' }).click();
- await expect(page.getByText('Delete').nth(3)).toBeVisible();
- await page.getByText('Delete').nth(3).click();
- await page.waitForTimeout(2000);
+ // Wait for the confirmation dialog and click the confirm button
+ const confirmButton = page.getByRole('button', { name: 'Confirm' }); // Adjust selector as needed
+ await expect(confirmButton).toBeVisible();
+ await confirmButton.click();
+ await expect(page.locator('tbody')).not.toContainText('test text', { timeout: 5000 });
- await expect(page.locator('tbody')).not.toContainText('test text');
});Note: Adjust the confirm button selector based on your actual UI implementation.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| test.afterEach(async ({ page }) => { | |
| await page.waitForTimeout(10000); | |
| await page.getByRole('button', { name: 'Delete' }).click(); | |
| await expect(page.getByText('Delete').nth(3)).toBeVisible(); | |
| await page.getByText('Delete').nth(3).click(); | |
| await page.waitForTimeout(2000); | |
| await expect(page.locator('tbody')).not.toContainText('test text'); | |
| }); | |
| test.afterEach(async ({ page }) => { | |
| await page.getByRole('button', { name: 'Delete' }).click(); | |
| // Wait for the confirmation dialog and click the confirm button | |
| const confirmButton = page.getByRole('button', { name: 'Confirm' }); // Adjust selector as needed | |
| await expect(confirmButton).toBeVisible(); | |
| await confirmButton.click(); | |
| await expect(page.locator('tbody')).not.toContainText('test text', { timeout: 5000 }); | |
| }); |
🤖 Prompt for AI Agents
In apps/e2e/scenarios/create-issue.spec.ts around lines 6 to 15, replace the
arbitrary timeouts and fragile .nth(3) selector with deterministic waits and
scoped locators: locate the table row containing the text 'test text', find and
click the Delete button inside that specific row, then wait for the confirmation
dialog/button to become visible and click it (use a clear selector like
role/button name for the confirm action), and finally wait for the original row
to be removed or assert it is no longer visible/detached; this removes fixed
delays and avoids relying on the third "Delete" element.
| await page.goto('http://localhost:3000', { | ||
| waitUntil: 'domcontentloaded', | ||
| }); | ||
|
|
||
| await page.getByRole('radio', { name: 'Issue' }).click(); | ||
| await page.getByRole('button', { name: 'Create Issue' }).click(); | ||
| await page.waitForTimeout(2000); | ||
| await page.getByPlaceholder('Please enter.').first().fill('test_issue'); | ||
| await page.getByRole('button', { name: 'Save' }).click(); | ||
| await page.waitForTimeout(10000); | ||
|
|
||
| await page.getByRole('radio', { name: 'Feedback' }).click(); | ||
| await page.waitForURL(/.*channelId.*/, { timeout: 10000 }); |
There was a problem hiding this comment.
Move hardcoded URLs to configuration and replace arbitrary timeouts.
Lines 20, 29: Hardcoded localhost:3000 should be extracted to a configuration file or environment variable to support different test environments.
Line 29: The 10-second waitForTimeout is excessive and non-deterministic. Use Playwright's built-in waiting mechanisms that wait for specific conditions.
Apply these changes:
+ // Extract to config: const BASE_URL = process.env.E2E_BASE_URL || 'http://localhost:3000';
- await page.goto('http://localhost:3000', {
+ await page.goto(BASE_URL, {
waitUntil: 'domcontentloaded',
});
await page.getByRole('radio', { name: 'Issue' }).click();
await page.getByRole('button', { name: 'Create Issue' }).click();
- await page.waitForTimeout(2000);
await page.getByPlaceholder('Please enter.').first().fill('test_issue');
await page.getByRole('button', { name: 'Save' }).click();
- await page.waitForTimeout(10000);
+ // Wait for navigation or success indicator
+ await expect(page.getByRole('radio', { name: 'Feedback' })).toBeEnabled();Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/e2e/scenarios/create-issue.spec.ts around lines 20 to 32, the test uses
a hardcoded URL and arbitrary timeouts; extract 'http://localhost:3000' into a
test config or environment variable (e.g., BASE_URL) and reference that instead,
and replace the two waitForTimeout calls with deterministic Playwright waits:
use page.goto(BASE_URL, { waitUntil: 'domcontentloaded' }) as-is but read
BASE_URL from config, replace the short waitForTimeout(2000) with waits for the
specific element to be ready (e.g., waitForSelector/getByPlaceholder/element
state) before filling, and replace the long waitForTimeout(10000) with explicit
waits for the post-save navigation or a success/toast/DOM change (e.g.,
waitForURL, waitForSelector, or waitForResponse) with appropriate timeouts.
| await axios.post( | ||
| `http://localhost:4000/api/projects/${projectId}/channels/${channelId}/feedbacks`, | ||
| { | ||
| message: 'test text', | ||
| }, | ||
| { headers: { 'x-api-key': 'MASTER_API_KEY' } }, | ||
| ); |
There was a problem hiding this comment.
CRITICAL: Remove hardcoded API key and move URLs to configuration.
Line 48: Hardcoding 'MASTER_API_KEY' is a critical security issue. If this code is committed to version control, the key is exposed. Even in a test environment, this is a security risk.
Line 44: The backend URL localhost:4000 should be configurable for different environments.
Additionally, the axios call lacks error handling, which could lead to cryptic test failures.
Apply these changes:
+ // Extract to config or env vars:
+ // const API_BASE_URL = process.env.E2E_API_URL || 'http://localhost:4000';
+ // const API_KEY = process.env.E2E_API_KEY;
+
+ try {
- await axios.post(
- `http://localhost:4000/api/projects/${projectId}/channels/${channelId}/feedbacks`,
+ await axios.post(
+ `${API_BASE_URL}/api/projects/${projectId}/channels/${channelId}/feedbacks`,
{
message: 'test text',
},
- { headers: { 'x-api-key': 'MASTER_API_KEY' } },
+ { headers: { 'x-api-key': API_KEY } },
);
+ } catch (error) {
+ throw new Error(`Failed to seed feedback data: ${error.message}`);
+ }Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/e2e/scenarios/create-issue.spec.ts around lines 43 to 49, the call
hardcodes the API key and backend URL and lacks error handling; replace the
literal 'MASTER_API_KEY' with a value read from configuration/environment (e.g.,
process.env.MASTER_API_KEY or a test config helper) and replace the hardcoded
'http://localhost:4000' with a configurable base URL (e.g.,
process.env.API_BASE_URL or a shared test config); update the axios.post call to
build the URL from that base and pass the API key from config, and wrap the
request in try/catch (or use axios .catch) so test failures surface a clear,
descriptive error message and optionally rethrow/assert to fail the test.
| await page.goto( | ||
| `http://localhost:3000/main/project/${projectId}/feedback?channelId=${channelId}`, | ||
| { waitUntil: 'domcontentloaded' }, | ||
| ); | ||
|
|
||
| await page.waitForTimeout(2000); | ||
|
|
||
| await expect(page.locator('tbody')).toContainText('test text'); |
There was a problem hiding this comment.
Use configuration for URLs and replace timeout with assertion-based wait.
Line 52: Same hardcoded localhost:3000 URL should use configuration.
Line 56: The 2-second waitForTimeout is arbitrary. Instead, wait for the expected element to appear.
Apply this diff:
+ // Use BASE_URL from config
await page.goto(
- `http://localhost:3000/main/project/${projectId}/feedback?channelId=${channelId}`,
+ `${BASE_URL}/main/project/${projectId}/feedback?channelId=${channelId}`,
{ waitUntil: 'domcontentloaded' },
);
- await page.waitForTimeout(2000);
-
+ // Wait for the data to load
await expect(page.locator('tbody')).toContainText('test text');Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/e2e/scenarios/create-issue.spec.ts around lines 51 to 58, replace the
hardcoded "http://localhost:3000" in page.goto with the test configuration base
URL (e.g., use a BASE_URL from your test config or process.env) while preserving
the route interpolation for projectId and channelId, and remove the fixed await
page.waitForTimeout(2000); instead add an assertion-based wait such as waiting
for the tbody locator to appear/be visible (e.g., locator.waitFor({ state:
'visible' }) or page.waitForSelector) before the expect so the test waits
deterministically for the content.
| await page | ||
| .locator('button[data-slot="popover-trigger"]:has-text("+")') | ||
| .first() | ||
| .click(); | ||
| await page.waitForTimeout(2000); | ||
|
|
||
| await page.getByText('test_issue').first().click(); | ||
| await page.waitForTimeout(2000); | ||
|
|
||
| await page | ||
| .locator('button[data-slot="popover-trigger"]:has-text("+")') | ||
| .first() | ||
| .click(); | ||
|
|
||
| await page.getByText('test text').first().click(); | ||
|
|
||
| await page | ||
| .locator('button[data-slot="popover-trigger"]:has-text("+")') | ||
| .nth(1) | ||
| .click(); | ||
| await page.waitForTimeout(2000); | ||
|
|
||
| await page.keyboard.insertText('test_issue2'); | ||
|
|
||
| await page.getByText('Create', { exact: true }).click(); | ||
| await page.waitForTimeout(2000); | ||
|
|
||
| await page.keyboard.press('Enter'); | ||
| await page.waitForTimeout(2000); | ||
| }); |
There was a problem hiding this comment.
Replace brittle selectors and arbitrary timeouts with deterministic waits.
This section has multiple issues that will make the test fragile and slow:
- Lines 62, 66, 71, 74: Using
.first()repeatedly assumes consistent DOM ordering and will break if elements are reordered. - Lines 64, 67, 80, 85, 88: Multiple 2-second
waitForTimeoutcalls are arbitrary and non-deterministic. - Line 78: Using
.nth(1)is brittle and lacks context about which element is being targeted. - The interaction flow lacks comments explaining what's being tested.
Consider these improvements:
+ // Open popover to link issue to feedback
await page
.locator('button[data-slot="popover-trigger"]:has-text("+")')
.first()
.click();
- await page.waitForTimeout(2000);
+ await expect(page.getByText('test_issue').first()).toBeVisible();
await page.getByText('test_issue').first().click();
- await page.waitForTimeout(2000);
+ await expect(page.getByText('test_issue')).toBeAttached();
+ // Select feedback message
await page
.locator('button[data-slot="popover-trigger"]:has-text("+")')
.first()
.click();
await page.getByText('test text').first().click();
+ // Add new issue
await page
.locator('button[data-slot="popover-trigger"]:has-text("+")')
.nth(1)
.click();
- await page.waitForTimeout(2000);
+ await expect(page.keyboard).toBeDefined(); // or wait for input field to be ready
await page.keyboard.insertText('test_issue2');
await page.getByText('Create', { exact: true }).click();
- await page.waitForTimeout(2000);
+ await expect(page.getByRole('button', { name: 'Save' })).toBeVisible(); // Adjust based on UI
await page.keyboard.press('Enter');
- await page.waitForTimeout(2000);
+ await expect(page.locator('tbody')).toContainText('test_issue2');
});Additionally, consider using more specific test IDs (e.g., data-testid attributes) instead of relying on text content and DOM position.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/e2e/scenarios/create-issue.spec.ts around lines 60 to 89, the test uses
fragile positional selectors (.first(), .nth(1)) and multiple hard coded
waitForTimeout(2000) calls which make it brittle and slow; replace positional
selectors with explicit, stable locators (prefer data-testid attributes like
data-testid="add-issue-btn", "issue-option-test_issue",
"issue-option-test_text", "add-subitem-btn", etc.), remove waitForTimeout calls
and instead wait deterministically for element states (e.g., locator.waitFor({
state: 'visible' }) or expect(locator).toBeVisible()/toHaveText() before
interacting), avoid .first()/.nth() by targeting the specific data-testid or a
more specific role/text locator, add brief inline comments describing each step
of the interaction flow, and ensure keyboard interactions use explicit focus on
the target input before insertText/press to guarantee deterministic behavior.
…ion for allowedVersions
fix: downgrade next version to ^15.5.5 and update renovate configuration for allowedVersions
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
renovate.json (1)
21-24: Add documentation for the Next.js version constraint.The
<16.0.0constraint aligns with the downgrade inapps/web/package.json, but the reason for this restriction is unclear. Consider:
- Adding a comment explaining why Next.js 16+ is not allowed
- Creating a tracking issue if this is a temporary constraint
- Referencing the issue number in a comment
This helps future maintainers understand the constraint's purpose and timeline.
Apply this diff to add a clarifying comment:
{ "matchPackageNames": ["@swc/core"], "allowedVersions": "<=1.13.5" }, { + // TODO: Remove once Next.js 16 compatibility issues are resolved (issue #XXXX) "matchPackageNames": ["next"], "allowedVersions": "<16.0.0" }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
apps/web/next.config.js(1 hunks)apps/web/package.json(1 hunks)renovate.json(1 hunks)
🔇 Additional comments (2)
apps/web/next.config.js (1)
13-13: Add a TODO comment if this is temporary, or removeignoreBuildErrorsand fix underlying TypeScript errors.The git history shows a downgrade from Next.js 16 to 15.5.5 (commit 9b9bc26), supporting your concern about working around compatibility issues. The current configuration suppresses TypeScript build errors without documentation or a tracking issue. Best practice requires either:
- Adding a clear TODO comment with an issue reference if this is temporary (while the actual errors are being fixed)
- Removing this setting entirely and fixing the root TypeScript errors
The recommendation to use targeted
@ts-expect-errorcomments instead of a blanket suppression is also sound for maintainability.apps/web/package.json (1)
52-52: Clarify the root cause of the Next.js downgrade; address TypeScript error suppression.The downgrade is documented in
renovate.json(allowedVersions constraint), but the commit lacks explanation. More critically, the same commit addstypescript: { ignoreBuildErrors: true }tonext.config.jsand removesreactStrictMode, indicating that Next.js 16 + React 19 breaks the build.Next.js 16 and React 19 have known ecosystem frictions with third-party packages, but the specific TypeScript errors causing this build failure are not documented. Rather than silently suppressing them, either:
- Document the exact TypeScript errors and explain why they can't be resolved, or
- Fix the underlying compatibility issues and remove the error suppression
Consider adding a comment in
next.config.jsexplaining theignoreBuildErrorsrationale and link to tracking issue/documentation.
…user-feedback into feat/enhance-e2e-tests
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/e2e/playwright.config.ts(5 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: e2e-test
- GitHub Check: integration-test
🔇 Additional comments (2)
apps/e2e/playwright.config.ts (2)
92-92: No changes needed — hardcoded credentials are properly isolated to test environments.Verification confirms:
JWT_SECRETandMASTER_API_KEYvalues are test-only; production code usesprocess.envreferences, not hardcoded literals- Empty OpenSearch credentials are intentional for test environments and validated by the schema in
apps/api/src/configs/opensearch.config.ts- No hardcoded test secrets appear in non-test production code
The configuration is correct as-is.
19-19: Based on my research of Playwright best practices and timeout configuration, I can now provide the rewritten review comment.Verify that increased timeouts are necessary and won't mask performance issues.
The test timeouts have been significantly increased:
- Global test timeout: 30s → 60s (Playwright default is 30s per 1-3)
- Expect timeout: → 60s
- Web server startup: → 120s
For complex e2e tests, timeout values should be tailored to context—tests involving heavy data processing or third-party integrations may require longer timeouts. Given that this setup includes OpenSearch integration, JWT authentication, and likely database migrations, 60 seconds may be necessary. However, confirm that:
- These timeouts reflect actual test needs, not arbitrary increases
- Local testing doesn't show performance regressions that would have been caught by shorter timeouts
- CI performance remains acceptable
Also applies to: 25-25, 90-90, 115-115
| timeout: 60 * 1000, | ||
| }, | ||
| /* Run tests in files in parallel */ | ||
| fullyParallel: true, |
There was a problem hiding this comment.
Resolve configuration inconsistency between fullyParallel and workers.
The configuration has fullyParallel: true but workers: 1. With only one worker, tests will run sequentially regardless of the fullyParallel setting, making it ineffective.
If sequential execution is required (e.g., to avoid shared resource conflicts with the database or OpenSearch), consider setting fullyParallel: false for clarity. Otherwise, increase the worker count to enable parallel execution.
Also applies to: 34-34
🤖 Prompt for AI Agents
In apps/e2e/playwright.config.ts around lines 28 and 34, the config sets
fullyParallel: true while workers: 1 which is contradictory; update the file so
the settings are consistent — either set fullyParallel: false if you need
sequential runs (keep workers: 1), or increase workers to a value >1 (e.g.,
workers: undefined or a numeric >1) to enable actual parallel execution; change
both occurrences so they match your intended execution mode.
| reporter: "html", | ||
| testMatch: "test.list.*", | ||
| reporter: 'html', | ||
| testMatch: 'test.list.*', |
There was a problem hiding this comment.
🧩 Analysis chain
Verify that test files match the testMatch pattern.
The testMatch pattern is set to 'test.list.*', which is unconventional. This will only match files like test.list.ts, test.list.js, etc. in the test directory.
Ensure that your e2e test files follow this naming convention. If they use standard naming like *.spec.ts or *.test.ts, tests won't be discovered.
🏁 Script executed:
#!/bin/bash
# Check what test files exist and whether they match the pattern
echo "Files matching 'test.list.*':"
fd -t f "^test\.list\." apps/e2e
echo -e "\nOther test files that might not match:"
fd -e spec.ts -e spec.js -e test.ts -e test.js apps/e2eLength of output: 429
🏁 Script executed:
#!/bin/bash
# Find all test files in apps/e2e with proper syntax
echo "Files matching 'test.list.*':"
fd "^test\.list\." --type f apps/e2e
echo -e "\nAll files in apps/e2e directory (excluding node_modules):"
fd --type f . apps/e2e | grep -E '\.(ts|js|tsx|jsx)$' | head -20
echo -e "\nChecking for common test file patterns:"
fd -e spec.ts -e spec.js -e test.ts -e test.js --type f . apps/e2e 2>/dev/null || echo "(No files found with standard patterns)"Length of output: 958
Fix testMatch pattern to discover all test files in apps/e2e/scenarios/.
The testMatch pattern 'test.list.*' will only match the single file test.list.ts in the testDir root. However, there are 7 actual test files in apps/e2e/scenarios/ that all use the .spec.ts naming convention:
create-channel.spec.tscreate-feedback-with-image.spec.tscreate-feedback.spec.tscreate-issue.spec.tscreate-multiple-feedbacks.spec.tscreate-project.spec.tssearch-feedback.spec.ts
These test files will not be discovered or executed by Playwright. Change testMatch to a pattern that captures both the structure—for example, '**/*.spec.ts' or '**/test.list.*' depending on your intent for test.list.ts.
🤖 Prompt for AI Agents
In apps/e2e/playwright.config.ts around line 37, the current testMatch pattern
'test.list.*' only matches a single file and prevents Playwright from
discovering the seven .spec.ts files under apps/e2e/scenarios/; update testMatch
to a glob that captures those spec files (for example '**/*.spec.ts' to match
all .spec.ts files under the testDir, or a combined pattern like
['**/*.spec.ts','**/test.list.*'] if you still need to include test.list.ts).
Summary by CodeRabbit
Tests
Chores
Bug Fixes