fix: E2E test stability and UI performance improvements (#823)

This commit is contained in:
gsxdsm
2026-03-02 18:46:28 -08:00
committed by GitHub
parent 1c3d6434a8
commit 54d69e907b
9 changed files with 88 additions and 21 deletions

View File

@@ -119,7 +119,6 @@ export function useBoardEffects({
if (featuresFingerprint && !isLoading) { if (featuresFingerprint && !isLoading) {
checkAllContexts(); checkAllContexts();
} }
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [featuresFingerprint, isLoading, checkContextExists, setFeaturesWithContext]); }, [featuresFingerprint, isLoading, checkContextExists, setFeaturesWithContext]);
// Re-check context when a feature stops, completes, or errors // Re-check context when a feature stops, completes, or errors

View File

@@ -1,4 +1,4 @@
import { useEffect, useRef, useCallback, useState } from 'react'; import { useEffect, useRef, useCallback, useState, useMemo } from 'react';
import { Dialog, DialogContent, DialogHeader, DialogTitle } from '@/components/ui/dialog'; import { Dialog, DialogContent, DialogHeader, DialogTitle } from '@/components/ui/dialog';
import { Button } from '@/components/ui/button'; import { Button } from '@/components/ui/button';
import { import {
@@ -54,6 +54,8 @@ export function DevServerLogsPanel({
const { const {
logs, logs,
logsVersion,
didTrim,
isRunning, isRunning,
isLoading, isLoading,
error, error,
@@ -81,8 +83,9 @@ export function DevServerLogsPanel({
return; return;
} }
// If logs got shorter (e.g., cleared), rewrite all // If logs got shorter (e.g., cleared) or buffer was trimmed (content shifted),
if (logs.length < lastLogsLengthRef.current) { // do a full rewrite so the terminal stays in sync
if (logs.length < lastLogsLengthRef.current || didTrim) {
xtermRef.current.write(logs); xtermRef.current.write(logs);
lastLogsLengthRef.current = logs.length; lastLogsLengthRef.current = logs.length;
return; return;
@@ -94,7 +97,7 @@ export function DevServerLogsPanel({
xtermRef.current.append(newContent); xtermRef.current.append(newContent);
lastLogsLengthRef.current = logs.length; lastLogsLengthRef.current = logs.length;
} }
}, [logs, worktree?.path]); }, [logs, logsVersion, didTrim, worktree?.path]);
// Reset when panel opens with a new worktree // Reset when panel opens with a new worktree
useEffect(() => { useEffect(() => {
@@ -123,10 +126,19 @@ export function DevServerLogsPanel({
} }
}, []); }, []);
const lineCount = useMemo(() => {
if (!logs) return 0;
// Count newlines directly instead of allocating a split array
let count = 1;
for (let i = 0; i < logs.length; i++) {
if (logs.charCodeAt(i) === 10) count++;
}
return count;
}, [logs]);
if (!worktree) return null; if (!worktree) return null;
const formattedStartTime = formatStartedAt(startedAt); const formattedStartTime = formatStartedAt(startedAt);
const lineCount = logs ? logs.split('\n').length : 0;
return ( return (
<Dialog open={open} onOpenChange={(isOpen) => !isOpen && onClose()}> <Dialog open={open} onOpenChange={(isOpen) => !isOpen && onClose()}>

View File

@@ -5,9 +5,16 @@ import { pathsEqual } from '@/lib/utils';
const logger = createLogger('DevServerLogs'); const logger = createLogger('DevServerLogs');
// Maximum log buffer size (characters) - matches server-side MAX_SCROLLBACK_SIZE
const MAX_LOG_BUFFER_SIZE = 50_000; // ~50KB
export interface DevServerLogState { export interface DevServerLogState {
/** The log content (buffered + live) */ /** The log content (buffered + live) */
logs: string; logs: string;
/** Incremented whenever logs content changes (including trim+shift) */
logsVersion: number;
/** True when the latest append caused head truncation */
didTrim: boolean;
/** Whether the server is currently running */ /** Whether the server is currently running */
isRunning: boolean; isRunning: boolean;
/** Whether initial logs are being fetched */ /** Whether initial logs are being fetched */
@@ -52,6 +59,8 @@ interface UseDevServerLogsOptions {
export function useDevServerLogs({ worktreePath, autoSubscribe = true }: UseDevServerLogsOptions) { export function useDevServerLogs({ worktreePath, autoSubscribe = true }: UseDevServerLogsOptions) {
const [state, setState] = useState<DevServerLogState>({ const [state, setState] = useState<DevServerLogState>({
logs: '', logs: '',
logsVersion: 0,
didTrim: false,
isRunning: false, isRunning: false,
isLoading: false, isLoading: false,
error: null, error: null,
@@ -123,6 +132,8 @@ export function useDevServerLogs({ worktreePath, autoSubscribe = true }: UseDevS
const clearLogs = useCallback(() => { const clearLogs = useCallback(() => {
setState({ setState({
logs: '', logs: '',
logsVersion: 0,
didTrim: false,
isRunning: false, isRunning: false,
isLoading: false, isLoading: false,
error: null, error: null,
@@ -136,13 +147,27 @@ export function useDevServerLogs({ worktreePath, autoSubscribe = true }: UseDevS
}, []); }, []);
/** /**
* Append content to logs * Append content to logs, enforcing a maximum buffer size to prevent
* unbounded memory growth and progressive UI lag.
*/ */
const appendLogs = useCallback((content: string) => { const appendLogs = useCallback((content: string) => {
setState((prev) => ({ setState((prev) => {
const combined = prev.logs + content;
const didTrim = combined.length > MAX_LOG_BUFFER_SIZE;
let newLogs = combined;
if (didTrim) {
const slicePoint = combined.length - MAX_LOG_BUFFER_SIZE;
// Find the next newline after the slice point to avoid cutting a line in half
const firstNewlineIndex = combined.indexOf('\n', slicePoint);
newLogs = combined.slice(firstNewlineIndex > -1 ? firstNewlineIndex + 1 : slicePoint);
}
return {
...prev, ...prev,
logs: prev.logs + content, logs: newLogs,
})); didTrim,
logsVersion: prev.logsVersion + 1,
};
});
}, []); }, []);
// Fetch initial logs when worktreePath changes // Fetch initial logs when worktreePath changes

View File

@@ -28,6 +28,7 @@ test.describe('Feature Deep Link', () => {
let projectPath: string; let projectPath: string;
let projectName: string; let projectName: string;
// eslint-disable-next-line no-empty-pattern
test.beforeEach(async ({}, testInfo) => { test.beforeEach(async ({}, testInfo) => {
projectName = `test-project-${testInfo.workerIndex}-${Date.now()}`; projectName = `test-project-${testInfo.workerIndex}-${Date.now()}`;
projectPath = path.join(TEST_TEMP_DIR, projectName); projectPath = path.join(TEST_TEMP_DIR, projectName);

View File

@@ -13,6 +13,7 @@ import {
waitForNetworkIdle, waitForNetworkIdle,
authenticateForTests, authenticateForTests,
handleLoginScreenIfPresent, handleLoginScreenIfPresent,
dismissSandboxWarningIfVisible,
} from '../../utils'; } from '../../utils';
const TEST_TEMP_DIR = createTempDirPath('responsive-modal-test'); const TEST_TEMP_DIR = createTempDirPath('responsive-modal-test');
@@ -100,6 +101,9 @@ test.describe('AgentOutputModal Responsive Behavior', () => {
await expect(page.locator('[data-testid="board-view"]')).toBeVisible({ timeout: 10000 }); await expect(page.locator('[data-testid="board-view"]')).toBeVisible({ timeout: 10000 });
// Dismiss sandbox warning dialog if it appears (blocks pointer events)
await dismissSandboxWarningIfVisible(page);
// Wait for the verified feature card to appear // Wait for the verified feature card to appear
const featureCard = page.locator(`[data-testid="kanban-card-${featureId}"]`); const featureCard = page.locator(`[data-testid="kanban-card-${featureId}"]`);
await expect(featureCard).toBeVisible({ timeout: 10000 }); await expect(featureCard).toBeVisible({ timeout: 10000 });

View File

@@ -13,6 +13,7 @@ import {
waitForNetworkIdle, waitForNetworkIdle,
authenticateForTests, authenticateForTests,
handleLoginScreenIfPresent, handleLoginScreenIfPresent,
dismissSandboxWarningIfVisible,
} from '../utils'; } from '../utils';
/** /**
@@ -109,6 +110,9 @@ test.describe('Success log output contrast', () => {
await expect(page.locator('[data-testid="board-view"]')).toBeVisible({ timeout: 10000 }); await expect(page.locator('[data-testid="board-view"]')).toBeVisible({ timeout: 10000 });
// Dismiss sandbox warning dialog if it appears (blocks pointer events)
await dismissSandboxWarningIfVisible(page);
// Wait for the verified feature card to appear // Wait for the verified feature card to appear
const featureCard = page.locator(`[data-testid="kanban-card-${featureId}"]`); const featureCard = page.locator(`[data-testid="kanban-card-${featureId}"]`);
await expect(featureCard).toBeVisible({ timeout: 10000 }); await expect(featureCard).toBeVisible({ timeout: 10000 });

View File

@@ -12,7 +12,7 @@
*/ */
import { test, expect, type Page } from '@playwright/test'; import { test, expect, type Page } from '@playwright/test';
import { authenticateForTests, navigateToSettings } from '../utils'; import { authenticateForTests, navigateToSettings, waitForSuccessToast } from '../utils';
// Timeout constants for maintainability // Timeout constants for maintainability
const TIMEOUTS = { const TIMEOUTS = {
@@ -224,6 +224,9 @@ test.describe('Event Hooks Settings', () => {
const addButton = dialog.locator('button:has-text("Add Endpoint")').last(); const addButton = dialog.locator('button:has-text("Add Endpoint")').last();
await addButton.click(); await addButton.click();
// Wait for the success toast to confirm the save completed (including API call)
await waitForSuccessToast(page, 'Endpoint added', { timeout: 10000 });
// Dialog should close // Dialog should close
await expect(dialog).toBeHidden({ timeout: TIMEOUTS.dialogHidden }); await expect(dialog).toBeHidden({ timeout: TIMEOUTS.dialogHidden });
@@ -256,16 +259,13 @@ test.describe('Event Hooks Settings', () => {
// The endpoints tab should show either existing endpoints or the empty state // The endpoints tab should show either existing endpoints or the empty state
// The key is that it should NOT show "empty" if there are endpoints on the server // The key is that it should NOT show "empty" if there are endpoints on the server
// Either we see "No endpoints configured" OR we see endpoint cards // Either we see "No ntfy endpoints configured" OR we see endpoint cards
const emptyState = page.locator('text=No endpoints configured'); const emptyState = page.locator('text=No ntfy endpoints configured');
const endpointCard = page.locator('[data-testid="endpoint-card"]').first(); const endpointCard = page.locator('[data-testid="endpoint-card"]').first();
// One of these should be visible // One of these should be visible (use Playwright's .or() to match either locator)
await expect( await expect(emptyState.or(endpointCard)).toBeVisible({
Promise.race([ timeout: TIMEOUTS.endpointVisible,
emptyState.waitFor({ state: 'visible', timeout: 5000 }).then(() => 'empty'), });
endpointCard.waitFor({ state: 'visible', timeout: 5000 }).then(() => 'card'),
])
).resolves.toBeDefined();
}); });
}); });

View File

@@ -2,6 +2,27 @@ import { Page, Locator } from '@playwright/test';
import { clickElement } from '../core/interactions'; import { clickElement } from '../core/interactions';
import { waitForElement, waitForElementHidden } from '../core/waiting'; import { waitForElement, waitForElementHidden } from '../core/waiting';
/**
* Dismiss the sandbox warning dialog if it appears.
* This dialog blocks pointer events and must be accepted before interacting
* with elements behind it.
*/
export async function dismissSandboxWarningIfVisible(page: Page): Promise<void> {
const sandboxAcceptBtn = page.locator('button:has-text("I Accept the Risks")');
const sandboxVisible = await sandboxAcceptBtn
.waitFor({ state: 'visible', timeout: 2000 })
.then(() => true)
.catch(() => false);
if (sandboxVisible) {
await sandboxAcceptBtn.click();
await page
.locator('[role="dialog"][data-state="open"]')
.first()
.waitFor({ state: 'hidden', timeout: 3000 })
.catch(() => {});
}
}
/** /**
* Check if the add feature dialog is visible * Check if the add feature dialog is visible
*/ */

View File

@@ -242,6 +242,7 @@ export async function setupRealProject(
theme: 'dark', theme: 'dark',
sidebarOpen: true, sidebarOpen: true,
maxConcurrency: 3, maxConcurrency: 3,
skipSandboxWarning: true,
}; };
localStorage.setItem('automaker-settings-cache', JSON.stringify(settingsCache)); localStorage.setItem('automaker-settings-cache', JSON.stringify(settingsCache));