diff --git a/.claude/settings.local.json b/.claude/settings.local.json index cfc635a..adc11d8 100644 --- a/.claude/settings.local.json +++ b/.claude/settings.local.json @@ -1,7 +1,10 @@ { "permissions": { "allow": [ - "Bash(find:*)" + "Bash(find:*)", + "Bash(touch:*)", + "Bash(gh pr checks:*)", + "Bash(grep:*)" ], "deny": [] } diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5a1247b..162bf39 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -11,7 +11,7 @@ jobs: name: Test runs-on: ubuntu-latest container: - image: mcr.microsoft.com/playwright:v1.48.0-noble + image: mcr.microsoft.com/playwright:v1.52.0-noble strategy: matrix: diff --git a/internal/client/browser_pool.go b/internal/client/browser_pool.go index 3520b3c..c88eb98 100644 --- a/internal/client/browser_pool.go +++ b/internal/client/browser_pool.go @@ -5,6 +5,7 @@ import ( "fmt" "log/slog" "sync" + "testing" "time" "github.com/playwright-community/playwright-go" @@ -213,6 +214,12 @@ func (p *BrowserPool) RenderPage(ctx context.Context, targetURL string) (string, } defer page.Close() + // Setup debug handlers if running in test mode + var consoleLogs, networkLogs []string + if testing.Testing() { + consoleLogs, networkLogs = SetupPageDebugHandlers(page) + } + // Set timeout page.SetDefaultTimeout(float64(p.config.Timeout.Milliseconds())) @@ -234,12 +241,28 @@ func (p *BrowserPool) RenderPage(ctx context.Context, targetURL string) (string, Timeout: playwright.Float(float64(p.config.Timeout.Milliseconds())), }) if err != nil { + // Log debug info when running in test mode + if testing.Testing() && (len(consoleLogs) > 0 || len(networkLogs) > 0) { + p.logger.Error("Navigation failed with debug info", + "url", targetURL, + "error", err, + "console_logs", consoleLogs, + "network_logs", networkLogs) + } return "", fmt.Errorf("failed to navigate to URL %s: %w", targetURL, err) } // Get the final HTML content content, err := page.Content() if err != nil { + // Log debug info when running in test mode + if testing.Testing() && (len(consoleLogs) > 0 || len(networkLogs) > 0) { + p.logger.Error("Failed to get content with debug info", + "url", targetURL, + "error", err, + "console_logs", consoleLogs, + "network_logs", networkLogs) + } return "", fmt.Errorf("failed to get page content: %w", err) } diff --git a/internal/client/browser_pool_test.go b/internal/client/browser_pool_test.go index 011332e..34dd267 100644 --- a/internal/client/browser_pool_test.go +++ b/internal/client/browser_pool_test.go @@ -3,7 +3,6 @@ package client import ( "context" "log/slog" - "os" "testing" "time" ) @@ -23,7 +22,7 @@ func TestNewBrowserPool(t *testing.T) { Enabled: true, BrowserType: "chromium", Headless: true, - Timeout: 30 * time.Second, + Timeout: 60 * time.Second, } pool2, err := NewBrowserPool(config, logger) @@ -39,7 +38,7 @@ func TestBrowserPool_AcquireContext(t *testing.T) { Enabled: true, BrowserType: "chromium", Headless: true, - Timeout: 30 * time.Second, + Timeout: 60 * time.Second, } pool, err := NewBrowserPool(config, logger) @@ -79,17 +78,12 @@ func TestBrowserPool_AcquireContext(t *testing.T) { } func TestBrowserPool_RenderPage(t *testing.T) { - // Skip this test in CI environment due to missing dependencies - if os.Getenv("CI") == "true" { - t.Skip("Skipping browser test in CI environment") - } - logger := slog.Default() config := &JSConfig{ Enabled: true, BrowserType: "chromium", Headless: true, - Timeout: 30 * time.Second, + Timeout: 60 * time.Second, } pool, err := NewBrowserPool(config, logger) @@ -102,6 +96,7 @@ func TestBrowserPool_RenderPage(t *testing.T) { ctx := context.Background() content, err := pool.RenderPage(ctx, "https://example.com") if err != nil { + // The browser pool now logs debug info automatically t.Fatalf("Failed to render page: %v", err) } @@ -121,7 +116,7 @@ func TestBrowserPool_GetPoolStats(t *testing.T) { Enabled: true, BrowserType: "chromium", Headless: true, - Timeout: 30 * time.Second, + Timeout: 60 * time.Second, WaitFor: "networkidle", } @@ -161,7 +156,7 @@ func TestBrowserPool_ConcurrentAccess(t *testing.T) { Enabled: true, BrowserType: "chromium", Headless: true, - Timeout: 30 * time.Second, + Timeout: 60 * time.Second, WaitFor: "networkidle", } @@ -208,7 +203,7 @@ func TestBrowserPool_Close(t *testing.T) { Enabled: true, BrowserType: "chromium", Headless: true, - Timeout: 30 * time.Second, + Timeout: 60 * time.Second, WaitFor: "networkidle", } @@ -242,7 +237,7 @@ func TestBrowserContext_ReleaseContext(t *testing.T) { Enabled: true, BrowserType: "chromium", Headless: true, - Timeout: 30 * time.Second, + Timeout: 60 * time.Second, WaitFor: "networkidle", } diff --git a/internal/client/js_client_test.go b/internal/client/js_client_test.go index 6a462ae..534f982 100644 --- a/internal/client/js_client_test.go +++ b/internal/client/js_client_test.go @@ -3,7 +3,6 @@ package client import ( "context" "log/slog" - "os" "testing" "time" ) @@ -23,7 +22,7 @@ func TestNewJSClient(t *testing.T) { Enabled: true, BrowserType: "chromium", Headless: true, - Timeout: 30 * time.Second, + Timeout: 60 * time.Second, WaitFor: "networkidle", } @@ -35,17 +34,12 @@ func TestNewJSClient(t *testing.T) { } func TestJSClient_RenderPage(t *testing.T) { - // Skip this test in CI environment due to missing dependencies - if os.Getenv("CI") == "true" { - t.Skip("Skipping browser test in CI environment") - } - logger := slog.Default() config := &JSConfig{ Enabled: true, BrowserType: "chromium", Headless: true, - Timeout: 30 * time.Second, + Timeout: 60 * time.Second, WaitFor: "networkidle", } @@ -73,17 +67,12 @@ func TestJSClient_RenderPage(t *testing.T) { } func TestJSClient_Get(t *testing.T) { - // Skip this test in CI environment due to missing dependencies - if os.Getenv("CI") == "true" { - t.Skip("Skipping browser test in CI environment") - } - logger := slog.Default() config := &JSConfig{ Enabled: true, BrowserType: "chromium", Headless: true, - Timeout: 30 * time.Second, + Timeout: 60 * time.Second, WaitFor: "networkidle", } @@ -127,7 +116,7 @@ func TestJSClient_GetPoolStats(t *testing.T) { Enabled: true, BrowserType: "chromium", Headless: true, - Timeout: 30 * time.Second, + Timeout: 60 * time.Second, WaitFor: "networkidle", } @@ -157,7 +146,7 @@ func TestJSClient_Disabled(t *testing.T) { Enabled: false, BrowserType: "chromium", Headless: true, - Timeout: 30 * time.Second, + Timeout: 60 * time.Second, } client, err := NewJSClient(config, logger) diff --git a/internal/client/test_helpers.go b/internal/client/test_helpers.go index 99d1553..b418bb9 100644 --- a/internal/client/test_helpers.go +++ b/internal/client/test_helpers.go @@ -1,8 +1,165 @@ package client -import "os" +import ( + "fmt" + "os" + "path/filepath" + "testing" + "time" -// isGitHubActions returns true if running in GitHub Actions -func isGitHubActions() bool { - return os.Getenv("GITHUB_ACTIONS") != "" + "github.com/playwright-community/playwright-go" +) + +// TestError wraps an error with additional context for better debugging +type TestError struct { + Err error + BrowserLogs []string + ConsoleLogs []string + NetworkLogs []string + ScreenshotPath string +} + +func (e *TestError) Error() string { + msg := fmt.Sprintf("Test failed: %v", e.Err) + if len(e.BrowserLogs) > 0 { + msg += fmt.Sprintf("\nBrowser logs: %v", e.BrowserLogs) + } + if len(e.ConsoleLogs) > 0 { + msg += fmt.Sprintf("\nConsole logs: %v", e.ConsoleLogs) + } + if len(e.NetworkLogs) > 0 { + msg += fmt.Sprintf("\nNetwork logs: %v", e.NetworkLogs) + } + if e.ScreenshotPath != "" { + msg += fmt.Sprintf("\nScreenshot saved to: %s", e.ScreenshotPath) + } + return msg +} + +// CapturePageDebugInfo captures debug information from a Playwright page +func CapturePageDebugInfo(t *testing.T, page playwright.Page, testName string) *TestError { + debugInfo := &TestError{} + + // Capture console logs + page.OnConsole(func(msg playwright.ConsoleMessage) { + debugInfo.ConsoleLogs = append(debugInfo.ConsoleLogs, + fmt.Sprintf("[%s] %s", msg.Type(), msg.Text())) + }) + + // Capture network requests + page.OnRequest(func(req playwright.Request) { + debugInfo.NetworkLogs = append(debugInfo.NetworkLogs, + fmt.Sprintf("Request: %s %s", req.Method(), req.URL())) + }) + + page.OnResponse(func(resp playwright.Response) { + debugInfo.NetworkLogs = append(debugInfo.NetworkLogs, + fmt.Sprintf("Response: %d %s", resp.Status(), resp.URL())) + }) + + // Capture screenshot on failure + if t.Failed() { + screenshotDir := filepath.Join(os.TempDir(), "urlmap-test-screenshots") + if err := os.MkdirAll(screenshotDir, 0755); err == nil { + screenshotPath := filepath.Join(screenshotDir, fmt.Sprintf("%s.png", testName)) + if _, err := page.Screenshot(playwright.PageScreenshotOptions{ + Path: playwright.String(screenshotPath), + FullPage: playwright.Bool(true), + }); err == nil { + debugInfo.ScreenshotPath = screenshotPath + } + } + } + + return debugInfo +} + +// SetupPageDebugHandlers sets up debug handlers for a page +func SetupPageDebugHandlers(page playwright.Page) (consoleLogs []string, networkLogs []string) { + // Capture console logs + page.OnConsole(func(msg playwright.ConsoleMessage) { + consoleLogs = append(consoleLogs, + fmt.Sprintf("[%s] %s", msg.Type(), msg.Text())) + }) + + // Capture network activity + page.OnRequest(func(req playwright.Request) { + networkLogs = append(networkLogs, + fmt.Sprintf("Request: %s %s", req.Method(), req.URL())) + }) + + page.OnResponse(func(resp playwright.Response) { + networkLogs = append(networkLogs, + fmt.Sprintf("Response: %d %s", resp.Status(), resp.URL())) + }) + + return consoleLogs, networkLogs +} + +// LogTestDebugInfo logs debug information when a test fails +func LogTestDebugInfo(t *testing.T, testName string, consoleLogs, networkLogs []string, err error) { + if err == nil { + return + } + + t.Errorf("Test %s failed: %v", testName, err) + + if len(consoleLogs) > 0 { + t.Logf("Console logs:") + for _, log := range consoleLogs { + t.Logf(" %s", log) + } + } + + if len(networkLogs) > 0 { + t.Logf("Network activity:") + for _, log := range networkLogs { + t.Logf(" %s", log) + } + } +} + +// CaptureScreenshotOnFailure captures a screenshot if the test has failed +func CaptureScreenshotOnFailure(t *testing.T, page playwright.Page, testName string) string { + if !t.Failed() { + return "" + } + + screenshotDir := filepath.Join(os.TempDir(), "urlmap-test-screenshots") + if err := os.MkdirAll(screenshotDir, 0755); err != nil { + t.Logf("Failed to create screenshot directory: %v", err) + return "" + } + + screenshotPath := filepath.Join(screenshotDir, fmt.Sprintf("%s.png", testName)) + if _, err := page.Screenshot(playwright.PageScreenshotOptions{ + Path: playwright.String(screenshotPath), + FullPage: playwright.Bool(true), + }); err != nil { + t.Logf("Failed to capture screenshot: %v", err) + return "" + } + + t.Logf("Screenshot saved to: %s", screenshotPath) + return screenshotPath +} + +// RetryTest runs a test function with retry logic for potentially flaky tests +func RetryTest(t *testing.T, maxAttempts int, testFunc func() error) { + var lastErr error + for attempt := 1; attempt <= maxAttempts; attempt++ { + lastErr = testFunc() + if lastErr == nil { + return // Test passed + } + + if attempt < maxAttempts { + t.Logf("Test attempt %d/%d failed: %v. Retrying...", attempt, maxAttempts, lastErr) + // Small delay between retries + time.Sleep(time.Second) + } + } + + // All attempts failed + t.Errorf("Test failed after %d attempts. Last error: %v", maxAttempts, lastErr) } diff --git a/internal/client/unified_client_test.go b/internal/client/unified_client_test.go index 6c9ca9e..b49da6f 100644 --- a/internal/client/unified_client_test.go +++ b/internal/client/unified_client_test.go @@ -36,10 +36,6 @@ func TestNewUnifiedClient_HTTPOnly(t *testing.T) { } func TestNewUnifiedClient_JSEnabled(t *testing.T) { - // Skip this test in GitHub Actions where Playwright may not be available - if isGitHubActions() { - t.Skip("Skipping Playwright test in GitHub Actions") - } config := &UnifiedConfig{ UserAgent: "test-agent",