From 9f35f7149d8fb790bbe8e4f0ee74f895aea1fc58 Mon Sep 17 00:00:00 2001 From: Peter Stone Date: Tue, 3 Feb 2026 15:15:07 -1000 Subject: Refactor template rendering with Renderer interface for testability Introduce a Renderer interface to abstract template rendering, enabling tests to use MockRenderer instead of requiring real template files. Changes: - Add renderer.go with Renderer interface, TemplateRenderer, and MockRenderer - Update Handler struct to use Renderer instead of *template.Template - Update HTMLResponse() to accept Renderer interface - Replace all h.templates.ExecuteTemplate() calls with h.renderer.Render() - Update all tests to use MockRenderer, removing template file dependencies This eliminates 15+ tests that previously skipped with "Templates not available" and improves test isolation. Co-Authored-By: Claude Opus 4.5 --- internal/handlers/agent_test.go | 174 ++++++++++++++++++++++++---------------- 1 file changed, 106 insertions(+), 68 deletions(-) (limited to 'internal/handlers/agent_test.go') diff --git a/internal/handlers/agent_test.go b/internal/handlers/agent_test.go index 7828650..5775962 100644 --- a/internal/handlers/agent_test.go +++ b/internal/handlers/agent_test.go @@ -412,30 +412,20 @@ func TestHandleAgentWebRequest(t *testing.T) { defer cleanup() cfg := &config.Config{} - h := &Handler{store: db, config: cfg, templates: loadTestTemplates(t)} + mock := newTestRenderer() + h := &Handler{store: db, config: cfg, renderer: mock} tests := []struct { - name string - queryParams string - expectedStatus int - checkBody func(t *testing.T, body string) + name string + queryParams string + expectedStatus int + expectedTemplate string }{ { - name: "valid request", - queryParams: "?name=TestAgent&agent_id=web-test-uuid", - expectedStatus: http.StatusOK, - checkBody: func(t *testing.T, body string) { - if body == "" { - t.Error("Expected non-empty response body") - } - // Should contain JSON data in script tag - if !contains(body, "application/json") { - t.Error("Expected JSON data in response") - } - if !contains(body, "request_token") { - t.Error("Expected request_token in response") - } - }, + name: "valid request", + queryParams: "?name=TestAgent&agent_id=web-test-uuid", + expectedStatus: http.StatusOK, + expectedTemplate: "agent-request.html", }, { name: "missing name", @@ -456,6 +446,7 @@ func TestHandleAgentWebRequest(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + mock.Calls = nil // Reset calls req := httptest.NewRequest(http.MethodGet, "/agent/web/request"+tt.queryParams, nil) w := httptest.NewRecorder() @@ -465,8 +456,12 @@ func TestHandleAgentWebRequest(t *testing.T) { t.Errorf("Expected status %d, got %d: %s", tt.expectedStatus, w.Code, w.Body.String()) } - if tt.checkBody != nil && w.Code == http.StatusOK { - tt.checkBody(t, w.Body.String()) + if tt.expectedTemplate != "" && w.Code == http.StatusOK { + if len(mock.Calls) == 0 { + t.Error("Expected render call") + } else if mock.Calls[0].Name != tt.expectedTemplate { + t.Errorf("Expected template %s, got %s", tt.expectedTemplate, mock.Calls[0].Name) + } } }) } @@ -477,7 +472,8 @@ func TestHandleAgentWebRequestReturnsExistingPending(t *testing.T) { defer cleanup() cfg := &config.Config{} - h := &Handler{store: db, config: cfg, templates: loadTestTemplates(t)} + mock := newTestRenderer() + h := &Handler{store: db, config: cfg, renderer: mock} agentID := "reuse-pending-uuid" @@ -489,14 +485,15 @@ func TestHandleAgentWebRequestReturnsExistingPending(t *testing.T) { if w1.Code != http.StatusOK { t.Fatalf("First request failed: %d, body: %s", w1.Code, w1.Body.String()) } - body1 := w1.Body.String() - // Verify session was created - if !contains(body1, "request_token") { - t.Fatal("First response doesn't contain request_token") + // Verify template was called + if len(mock.Calls) == 0 || mock.Calls[0].Name != "agent-request.html" { + t.Fatal("First request didn't render agent-request.html") } + firstData := mock.Calls[0].Data - // Second request should return the same pending session + // Reset mock and make second request + mock.Calls = nil req2 := httptest.NewRequest(http.MethodGet, "/agent/web/request?name=TestAgent&agent_id="+agentID, nil) w2 := httptest.NewRecorder() h.HandleAgentWebRequest(w2, req2) @@ -504,11 +501,18 @@ func TestHandleAgentWebRequestReturnsExistingPending(t *testing.T) { if w2.Code != http.StatusOK { t.Fatalf("Second request failed: %d", w2.Code) } - body2 := w2.Body.String() - // Both responses should be identical (same session reused) - if body1 != body2 { - t.Error("Expected same response for existing pending session (session should be reused)") + // Verify template was called with same data (same session reused) + if len(mock.Calls) == 0 { + t.Fatal("Second request didn't render template") + } + secondData := mock.Calls[0].Data + + // Compare request tokens from data + first := firstData.(map[string]interface{}) + second := secondData.(map[string]interface{}) + if first["RequestToken"] != second["RequestToken"] { + t.Error("Expected same session to be reused (same request token)") } } @@ -517,7 +521,8 @@ func TestHandleAgentWebStatus(t *testing.T) { defer cleanup() cfg := &config.Config{} - h := &Handler{store: db, config: cfg, templates: loadTestTemplates(t)} + mock := newTestRenderer() + h := &Handler{store: db, config: cfg, renderer: mock} // Create a pending session session := &models.AgentSession{ @@ -531,18 +536,20 @@ func TestHandleAgentWebStatus(t *testing.T) { } tests := []struct { - name string - token string - expectedStatus int - checkBody func(t *testing.T, body string) + name string + token string + expectedStatus int + expectedTemplate string + checkData func(t *testing.T, data map[string]interface{}) }{ { - name: "valid pending session", - token: "web-status-test-token", - expectedStatus: http.StatusOK, - checkBody: func(t *testing.T, body string) { - if !contains(body, `"status": "pending"`) { - t.Error("Expected status 'pending' in response") + name: "valid pending session", + token: "web-status-test-token", + expectedStatus: http.StatusOK, + expectedTemplate: "agent-status.html", + checkData: func(t *testing.T, data map[string]interface{}) { + if data["Status"] != "pending" { + t.Errorf("Expected status 'pending', got '%v'", data["Status"]) } }, }, @@ -560,6 +567,7 @@ func TestHandleAgentWebStatus(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + mock.Calls = nil url := "/agent/web/status" if tt.token != "" { url += "?token=" + tt.token @@ -574,8 +582,17 @@ func TestHandleAgentWebStatus(t *testing.T) { t.Errorf("Expected status %d, got %d: %s", tt.expectedStatus, w.Code, w.Body.String()) } - if tt.checkBody != nil && w.Code == http.StatusOK { - tt.checkBody(t, w.Body.String()) + if tt.expectedTemplate != "" && w.Code == http.StatusOK { + if len(mock.Calls) == 0 { + t.Error("Expected render call") + } else { + if mock.Calls[0].Name != tt.expectedTemplate { + t.Errorf("Expected template %s, got %s", tt.expectedTemplate, mock.Calls[0].Name) + } + if tt.checkData != nil { + tt.checkData(t, mock.Calls[0].Data.(map[string]interface{})) + } + } } }) } @@ -586,7 +603,8 @@ func TestHandleAgentWebStatusApproved(t *testing.T) { defer cleanup() cfg := &config.Config{} - h := &Handler{store: db, config: cfg, templates: loadTestTemplates(t)} + mock := newTestRenderer() + h := &Handler{store: db, config: cfg, renderer: mock} // Create and approve a session session := &models.AgentSession{ @@ -612,15 +630,22 @@ func TestHandleAgentWebStatusApproved(t *testing.T) { t.Errorf("Expected status 200, got %d: %s", w.Code, w.Body.String()) } - body := w.Body.String() - if !contains(body, `"status": "approved"`) { - t.Error("Expected status 'approved' in response") + if len(mock.Calls) == 0 { + t.Fatal("Expected render call") + } + if mock.Calls[0].Name != "agent-status.html" { + t.Errorf("Expected template agent-status.html, got %s", mock.Calls[0].Name) + } + + data := mock.Calls[0].Data.(map[string]interface{}) + if data["Status"] != "approved" { + t.Errorf("Expected status 'approved', got '%v'", data["Status"]) } - if !contains(body, `"session_token": "`+sessionToken) { - t.Error("Expected session_token in response") + if data["SessionToken"] != sessionToken { + t.Errorf("Expected session_token '%s', got '%v'", sessionToken, data["SessionToken"]) } - if !contains(body, `"context_url":`) { - t.Error("Expected context_url in response") + if _, ok := data["ContextURL"]; !ok { + t.Error("Expected ContextURL in data") } } @@ -629,7 +654,8 @@ func TestHandleAgentWebContext(t *testing.T) { defer cleanup() cfg := &config.Config{} - h := &Handler{store: db, config: cfg, templates: loadTestTemplates(t)} + mock := newTestRenderer() + h := &Handler{store: db, config: cfg, renderer: mock} // Create and approve a session session := &models.AgentSession{ @@ -647,21 +673,23 @@ func TestHandleAgentWebContext(t *testing.T) { } tests := []struct { - name string - sessionToken string - expectedStatus int - checkBody func(t *testing.T, body string) + name string + sessionToken string + expectedStatus int + expectedTemplate string + checkData func(t *testing.T, data map[string]interface{}) }{ { - name: "valid session", - sessionToken: sessionToken, - expectedStatus: http.StatusOK, - checkBody: func(t *testing.T, body string) { - if !contains(body, "generated_at") { - t.Error("Expected generated_at in response") + name: "valid session", + sessionToken: sessionToken, + expectedStatus: http.StatusOK, + expectedTemplate: "agent-context.html", + checkData: func(t *testing.T, data map[string]interface{}) { + if _, ok := data["GeneratedAt"]; !ok { + t.Error("Expected GeneratedAt in data") } - if !contains(body, "timeline") { - t.Error("Expected timeline in response") + if _, ok := data["Timeline"]; !ok { + t.Error("Expected Timeline in data") } }, }, @@ -679,6 +707,7 @@ func TestHandleAgentWebContext(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + mock.Calls = nil url := "/agent/web/context" if tt.sessionToken != "" { url += "?session=" + tt.sessionToken @@ -693,8 +722,17 @@ func TestHandleAgentWebContext(t *testing.T) { t.Errorf("Expected status %d, got %d: %s", tt.expectedStatus, w.Code, w.Body.String()) } - if tt.checkBody != nil && w.Code == http.StatusOK { - tt.checkBody(t, w.Body.String()) + if tt.expectedTemplate != "" && w.Code == http.StatusOK { + if len(mock.Calls) == 0 { + t.Error("Expected render call") + } else { + if mock.Calls[0].Name != tt.expectedTemplate { + t.Errorf("Expected template %s, got %s", tt.expectedTemplate, mock.Calls[0].Name) + } + if tt.checkData != nil { + tt.checkData(t, mock.Calls[0].Data.(map[string]interface{})) + } + } } }) } -- cgit v1.2.3