Fix test issues: properly check mock expectations and improve log capture (#3)

askdba · web-flow · commit 56ca116d8152 · 2025-12-10T18:23:11.000+02:00
- connection_test.go: Fix TestConnectionManagerMultipleConnections to properly
  check ExpectationsWereMet() errors instead of silently discarding them with
  blank identifiers

- logging_test.go: Improve log output capture by also redirecting the standard
  log package output, and add proper error checking for empty output
diff --git a/cmd/mysql-mcp-server/connection_test.go b/cmd/mysql-mcp-server/connection_test.go
@@ -69,11 +69,8 @@ func TestConnectionManagerWithMockDB(t *testing.T) {
 	}
 	defer mockDB.Close()
 
-	// Expect ping
-	mock.ExpectPing()
-
 	cm := NewConnectionManager()
-	
+
 	// Manually add the mock connection (bypassing AddConnectionWithPoolConfig)
 	cm.connections["test"] = mockDB
 	cm.configs["test"] = config.ConnectionConfig{
@@ -138,9 +135,6 @@ func TestConnectionManagerMultipleConnections(t *testing.T) {
 	}
 	defer mockDB2.Close()
 
-	mock1.ExpectPing()
-	mock2.ExpectPing()
-
 	cm := NewConnectionManager()
 
 	// Add connections manually
@@ -186,8 +180,12 @@ func TestConnectionManagerMultipleConnections(t *testing.T) {
 	}
 
 	cm.Close()
-	_ = mock1.ExpectationsWereMet()
-	_ = mock2.ExpectationsWereMet()
+	if err := mock1.ExpectationsWereMet(); err != nil {
+		t.Errorf("mock1 unfulfilled expectations: %v", err)
+	}
+	if err := mock2.ExpectationsWereMet(); err != nil {
+		t.Errorf("mock2 unfulfilled expectations: %v", err)
+	}
 }
 
 func TestGetDBWithConnManager(t *testing.T) {
@@ -266,14 +264,12 @@ func TestConnectionConfigStruct(t *testing.T) {
 
 func TestConnectionManagerConcurrency(t *testing.T) {
 	// Create mock database
-	mockDB, mock, err := sqlmock.New()
+	mockDB, _, err := sqlmock.New()
 	if err != nil {
 		t.Fatalf("failed to create mock: %v", err)
 	}
 	defer mockDB.Close()
 
-	mock.ExpectPing()
-
 	cm := NewConnectionManager()
 	cm.connections["test"] = mockDB
 	cm.configs["test"] = config.ConnectionConfig{Name: "test", DSN: "test:test@tcp(localhost)/test"}
@@ -306,4 +302,3 @@ func TestConnectionManagerConcurrency(t *testing.T) {
 		<-done
 	}
 }
-
diff --git a/cmd/mysql-mcp-server/logging_test.go b/cmd/mysql-mcp-server/logging_test.go
@@ -4,6 +4,7 @@ package main
 import (
 	"bytes"
 	"encoding/json"
+	"log"
 	"os"
 	"path/filepath"
 	"strings"
@@ -34,47 +35,56 @@ func TestNewQueryTimer(t *testing.T) {
 }
 
 func TestQueryTimerLogSuccess(t *testing.T) {
-	// Capture stderr
+	// Capture stderr and log output
 	oldStderr := os.Stderr
+	oldLogOutput := log.Writer()
 	r, w, _ := os.Pipe()
 	os.Stderr = w
+	log.SetOutput(w)
 
 	timer := NewQueryTimer("test_query")
 	timer.LogSuccess(5, "SELECT * FROM test")
 
 	w.Close()
 	os.Stderr = oldStderr
+	log.SetOutput(oldLogOutput)
 
 	var buf bytes.Buffer
 	buf.ReadFrom(r)
 	output := buf.String()
 
-	// Should contain log output
-	if !strings.Contains(output, "test_query") && !strings.Contains(output, "query executed") {
-		// Either JSON or plain log format should work
-		t.Logf("Log output: %s", output)
+	// Should contain log output - either JSON or plain log format should work
+	if output == "" {
+		t.Error("expected log output, got empty string")
+	} else if !strings.Contains(output, "test_query") && !strings.Contains(output, "query executed") {
+		t.Errorf("expected output to contain 'test_query' or 'query executed', got: %s", output)
 	}
 }
 
 func TestQueryTimerLogError(t *testing.T) {
-	// Capture stderr
+	// Capture stderr and log output
 	oldStderr := os.Stderr
+	oldLogOutput := log.Writer()
 	r, w, _ := os.Pipe()
 	os.Stderr = w
+	log.SetOutput(w)
 
 	timer := NewQueryTimer("test_query")
 	timer.LogError(os.ErrNotExist, "SELECT * FROM test")
 
 	w.Close()
 	os.Stderr = oldStderr
+	log.SetOutput(oldLogOutput)
 
 	var buf bytes.Buffer
 	buf.ReadFrom(r)
 	output := buf.String()
 
-	// Should contain error info
-	if !strings.Contains(output, "test_query") && !strings.Contains(output, "failed") {
-		t.Logf("Log output: %s", output)
+	// Should contain error info - either tool name or failure indicator
+	if output == "" {
+		t.Error("expected log output, got empty string")
+	} else if !strings.Contains(output, "test_query") && !strings.Contains(output, "failed") {
+		t.Errorf("expected output to contain 'test_query' or 'failed', got: %s", output)
 	}
 }
 
@@ -295,4 +305,3 @@ func TestAuditEntryWithError(t *testing.T) {
 		t.Error("error message should be in JSON output")
 	}
 }
-
