Added tests to check the websocket in logs section (Object Explorer page) #2220
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Pull Request Overview
This PR adds comprehensive end-to-end tests for WebSocket and API integration in the Object Explorer page, specifically focusing on the logs section functionality. The tests verify real-time log streaming via WebSocket, API request handling, error scenarios, and response validation.
Key Changes:
- Added 8 new E2E tests covering WebSocket log streaming, reconnection handling, and API request validation
- Implemented feature detection logic to gracefully handle incomplete implementations
- Added test scenarios for concurrent API requests and content-type validation
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| page.on('websocket', (ws: WebSocket) => { | ||
| console.info('WebSocket opened:', ws.url()); | ||
| ws.on('framereceived', event => { | ||
| if (event.payload) { | ||
| logMessages.push(event.payload.toString()); | ||
| } | ||
| }); | ||
| }); |
There was a problem hiding this comment.
The WebSocket event listener is registered after opening the resource details. This creates a race condition where WebSocket connections established before line 61 won't be captured. Move the WebSocket listener registration before openResourceDetails(0) at line 50 to ensure all WebSocket connections are captured.
|
|
||
| if (!isAvailable) { | ||
| console.warn('Resource details feature not implemented - WebSocket test skipped'); | ||
| expect(true).toBe(true); |
There was a problem hiding this comment.
The assertion expect(true).toBe(true) is redundant and doesn't validate anything. If the test should be skipped when the feature is not implemented, consider using test.skip() instead, or simply return without an assertion.
| expect(true).toBe(true); |
| const hasValidLogFormat = logMessages.some( | ||
| msg => | ||
| msg.includes('INFO') || | ||
| msg.includes('ERROR') || | ||
| msg.includes('WARN') || | ||
| msg.includes('timestamp') || | ||
| msg.includes('level') | ||
| ); | ||
|
|
||
| if (hasValidLogFormat) { | ||
| expect(hasValidLogFormat).toBe(true); | ||
| } else { | ||
| console.warn('Log messages received but format may be different than expected'); | ||
| expect(true).toBe(true); | ||
| } |
There was a problem hiding this comment.
The nested conditional logic (lines 80-94) adds complexity and makes the test harder to maintain. Consider extracting this validation into a separate helper function like validateLogMessageFormat(messages: string[]): boolean.
| test('should make API requests for resource YAML data', async ({ page }) => { | ||
| await objectExplorerPage.openResourceDetails(0); | ||
| const isAvailable = await checkResourceDetailsAvailable(page); | ||
|
|
||
| if (!isAvailable) { | ||
| console.warn('Resource details feature not implemented - YAML API test skipped'); | ||
| expect(true).toBe(true); | ||
| return; | ||
| } | ||
|
|
||
| const apiRequests: string[] = []; | ||
| page.on('request', request => { | ||
| apiRequests.push(request.url()); | ||
| }); | ||
|
|
||
| await mswHelper.applyScenario('resourceYamlAPI'); | ||
|
|
||
| const editTab = objectExplorerPage.editTab; | ||
| if (await editTab.isVisible().catch(() => false)) { | ||
| await editTab.click(); | ||
| await page.waitForTimeout(2000); | ||
|
|
||
| if (apiRequests.length > 0) { | ||
| const hasYamlRequest = apiRequests.some( | ||
| url => | ||
| url.includes('/yaml') || | ||
| (url.includes('/api/') && url.includes('pod')) || | ||
| url.includes('/v1/namespaces/default/pods/') | ||
| ); | ||
|
|
||
| if (hasYamlRequest) { | ||
| expect(hasYamlRequest).toBe(true); | ||
| } else { | ||
| console.warn('No YAML API requests found - YAML API may not be implemented'); | ||
| expect(true).toBe(true); | ||
| } | ||
| } else { | ||
| console.warn('No API requests made - YAML API may not be implemented'); | ||
| expect(true).toBe(true); | ||
| } | ||
| } else { | ||
| console.warn('EDIT tab not implemented - YAML API test skipped'); | ||
| expect(true).toBe(true); | ||
| } | ||
| }); | ||
|
|
||
| test('should make API requests for resource logs', async ({ page }) => { | ||
| await objectExplorerPage.openResourceDetails(0); | ||
| const isAvailable = await checkResourceDetailsAvailable(page); | ||
|
|
||
| if (!isAvailable) { | ||
| console.warn('Resource details feature not implemented - logs API test skipped'); | ||
| expect(true).toBe(true); | ||
| return; | ||
| } | ||
|
|
||
| const apiRequests: string[] = []; | ||
| page.on('request', request => { | ||
| apiRequests.push(request.url()); | ||
| }); | ||
|
|
||
| await mswHelper.applyScenario('resourceLogsAPI'); | ||
|
|
||
| const logsTab = objectExplorerPage.logsTab; | ||
| if (await logsTab.isVisible().catch(() => false)) { | ||
| await logsTab.click(); | ||
| await page.waitForTimeout(2000); | ||
|
|
||
| if (apiRequests.length > 0) { | ||
| const hasLogsRequest = apiRequests.some( | ||
| url => | ||
| url.includes('/logs') || | ||
| (url.includes('/api/') && url.includes('log')) || | ||
| (url.includes('/v1/namespaces/default/pods/') && url.includes('/log')) | ||
| ); | ||
|
|
||
| if (hasLogsRequest) { | ||
| expect(hasLogsRequest).toBe(true); | ||
| } else { | ||
| console.warn('No logs API requests found - logs API may not be implemented'); | ||
| expect(true).toBe(true); | ||
| } | ||
| } else { | ||
| console.warn('No API requests made - logs API may not be implemented'); | ||
| expect(true).toBe(true); | ||
| } | ||
| } else { | ||
| console.warn('LOGS tab not implemented - logs API test skipped'); | ||
| expect(true).toBe(true); | ||
| } | ||
| }); |
There was a problem hiding this comment.
This test and the following tests (lines 151-241) follow a repetitive pattern with nearly identical structure: open details, check availability, register listener, apply scenario, click tab, wait, check results with fallbacks. Consider creating a reusable test helper function to reduce duplication and improve maintainability.
| const hasYamlRequest = apiRequests.some( | ||
| url => | ||
| url.includes('/yaml') || | ||
| (url.includes('/api/') && url.includes('pod')) || | ||
| url.includes('/v1/namespaces/default/pods/') | ||
| ); |
There was a problem hiding this comment.
This complex multi-condition check for YAML requests is repeated in similar form for logs requests (lines 220-225). Consider extracting into a helper function like hasMatchingRequest(requests: string[], patterns: string[]): boolean to improve maintainability.
| const logsTab = objectExplorerPage.logsTab; | ||
| if (await logsTab.isVisible().catch(() => false)) { | ||
| await logsTab.click(); | ||
| await page.waitForTimeout(5000); |
There was a problem hiding this comment.
Hard-coded timeout of 5000ms may cause flakiness in CI environments or slower machines. Consider using Playwright's built-in waiting mechanisms like waitForEvent() or making this timeout configurable through an environment variable.
| await page.waitForTimeout(5000); | |
| await expect.poll(() => logMessages.length, { | |
| message: 'Waiting for at least one log message via WebSocket', | |
| timeout: parseInt(process.env.LOG_MESSAGE_TIMEOUT ?? '10000', 10), | |
| }).toBeGreaterThan(0); |
| if (logMessages.length > 0) { | ||
| expect(logMessages.length).toBeGreaterThan(0); | ||
|
|
||
| const hasValidLogFormat = logMessages.some( | ||
| msg => | ||
| msg.includes('INFO') || | ||
| msg.includes('ERROR') || | ||
| msg.includes('WARN') || | ||
| msg.includes('timestamp') || | ||
| msg.includes('level') | ||
| ); | ||
|
|
||
| if (hasValidLogFormat) { | ||
| expect(hasValidLogFormat).toBe(true); | ||
| } else { | ||
| console.warn('Log messages received but format may be different than expected'); | ||
| expect(true).toBe(true); | ||
| } | ||
| } else { | ||
| console.warn('No WebSocket messages received - WebSocket feature may not be implemented'); | ||
| expect(true).toBe(true); | ||
| } |
There was a problem hiding this comment.
This test will always pass due to the fallback logic. When logMessages.length > 0 is false, the test falls through to line 97 where it sets expect(true).toBe(true). This means the test passes even when WebSocket functionality doesn't work. Consider failing the test or using test.skip() if the feature is genuinely not implemented.
| page.on('websocket', (ws: WebSocket) => { | ||
| console.info('WebSocket connection attempt:', ws.url()); | ||
| connectionCount++; | ||
| }); |
There was a problem hiding this comment.
Similar race condition as in the previous test. The WebSocket listener is registered after opening resource details. Move this listener registration before openResourceDetails(0) at line 106 to capture all WebSocket connections.
| test('should handle WebSocket reconnection on connection loss', async ({ page }) => { | ||
| await objectExplorerPage.openResourceDetails(0); | ||
| const isAvailable = await checkResourceDetailsAvailable(page); | ||
|
|
||
| if (!isAvailable) { | ||
| console.warn( | ||
| 'Resource details feature not implemented - WebSocket reconnection test skipped' | ||
| ); | ||
| expect(true).toBe(true); | ||
| return; | ||
| } | ||
|
|
||
| let connectionCount = 0; | ||
|
|
||
| page.on('websocket', (ws: WebSocket) => { | ||
| console.info('WebSocket connection attempt:', ws.url()); | ||
| connectionCount++; | ||
| }); | ||
|
|
||
| await mswHelper.applyScenario('logStreamingUnstable'); | ||
|
|
||
| const logsTab = objectExplorerPage.logsTab; | ||
| if (await logsTab.isVisible().catch(() => false)) { | ||
| await logsTab.click(); | ||
| await page.waitForTimeout(2000); | ||
|
|
||
| await mswHelper.applyScenario('networkInterruption'); | ||
| await page.waitForTimeout(1000); | ||
|
|
||
| await mswHelper.applyScenario('logStreamingWebSocket'); | ||
| await page.waitForTimeout(3000); | ||
|
|
||
| if (connectionCount > 0) { | ||
| expect(connectionCount).toBeGreaterThan(0); | ||
| } else { | ||
| console.warn( | ||
| 'No WebSocket connections detected - WebSocket feature may not be implemented' | ||
| ); | ||
| expect(true).toBe(true); | ||
| } | ||
| } else { | ||
| console.warn('LOGS tab not implemented - WebSocket reconnection test skipped'); | ||
| expect(true).toBe(true); | ||
| } | ||
| }); |
There was a problem hiding this comment.
This test follows the same pattern as the previous WebSocket test with similar conditional logic and fallbacks. Consider extracting common test setup (resource details opening, feature availability check, WebSocket listener registration) into a reusable helper function to reduce code duplication.
| page.on('request', request => { | ||
| apiRequests.push(request.url()); | ||
| }); |
There was a problem hiding this comment.
The request listener should be registered before opening resource details to avoid missing early API requests. Move this listener registration before line 152 (await objectExplorerPage.openResourceDetails(0);).
1 participant
Description
Added tests to check the websocket in logs section (Object Explorer page)
Related Issue
Fixes #2200
Changes Made
Checklist
Please ensure the following before submitting your PR:
Screenshots or Logs (if applicable)
Additional Notes