From ee4450fcfad90596a7c5f2f829e39ba409c9a453 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 5 Jan 2026 06:58:33 +0000 Subject: [PATCH 1/5] Initial plan From 03a732af057a9581e4eb0d695e543f50dd2f8cb4 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 5 Jan 2026 07:04:40 +0000 Subject: [PATCH 2/5] fix(testutil): improve error handling in EnsureNamespace function - Add explicit handling for GetNamespaces errors with fallback logic - Handle "already exists" errors gracefully when creating namespaces - Add strings package import for error message checking - Improve comments to explain error handling strategy Co-authored-by: freeznet <1381618+freeznet@users.noreply.github.com> --- pkg/mcp/internal/testutil/pulsar.go | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/pkg/mcp/internal/testutil/pulsar.go b/pkg/mcp/internal/testutil/pulsar.go index c527fce..b0e55aa 100644 --- a/pkg/mcp/internal/testutil/pulsar.go +++ b/pkg/mcp/internal/testutil/pulsar.go @@ -20,6 +20,7 @@ import ( "context" "fmt" "net/http" + "strings" "time" "github.com/apache/pulsar-client-go/pulsaradmin" @@ -106,7 +107,12 @@ func (h *PulsarTestHelper) EnsureNamespace(ctx context.Context, tenant, namespac // Check if namespace exists namespaces, err := h.adminClient.Namespaces().GetNamespaces(tenant) - if err == nil { + if err != nil { + // If we cannot list namespaces, we'll try to create it anyway. + // This could be due to permission issues or other transient errors. + // The CreateNamespace call will fail with a proper error if there's a real issue. + } else { + // Successfully retrieved namespaces, check if ours exists for _, ns := range namespaces { if ns == fullNamespace { return nil // Already exists @@ -114,9 +120,17 @@ func (h *PulsarTestHelper) EnsureNamespace(ctx context.Context, tenant, namespac } } - // Create namespace + // Create namespace - this will fail if it already exists or if there are permission/connection issues err = h.adminClient.Namespaces().CreateNamespace(fullNamespace) if err != nil { + // Check if the error is because namespace already exists + // The Pulsar admin API returns a specific error message for this case + errMsg := err.Error() + if strings.Contains(errMsg, "already exists") || + strings.Contains(errMsg, "AlreadyExists") || + strings.Contains(errMsg, "409") { + return nil // Namespace already exists, which is what we want + } return fmt.Errorf("failed to create namespace %s: %w", fullNamespace, err) } From 3709c455355aad401594e0f9fe738aaaba1d8e2c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 5 Jan 2026 07:06:26 +0000 Subject: [PATCH 3/5] docs(testutil): improve error handling documentation in EnsureNamespace - Add comprehensive comment explaining why string matching is used - Add "Conflict" to the list of error patterns to check - Clarify this is a best-effort approach for test utilities Co-authored-by: freeznet <1381618+freeznet@users.noreply.github.com> --- pkg/mcp/internal/testutil/pulsar.go | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/pkg/mcp/internal/testutil/pulsar.go b/pkg/mcp/internal/testutil/pulsar.go index b0e55aa..7752290 100644 --- a/pkg/mcp/internal/testutil/pulsar.go +++ b/pkg/mcp/internal/testutil/pulsar.go @@ -123,12 +123,17 @@ func (h *PulsarTestHelper) EnsureNamespace(ctx context.Context, tenant, namespac // Create namespace - this will fail if it already exists or if there are permission/connection issues err = h.adminClient.Namespaces().CreateNamespace(fullNamespace) if err != nil { - // Check if the error is because namespace already exists - // The Pulsar admin API returns a specific error message for this case + // Check if the error is because namespace already exists. + // The Pulsar admin client doesn't provide typed errors, so we check the error message. + // This is a best-effort approach for test utilities - we check for common patterns: + // - "already exists" (common HTTP API message) + // - "AlreadyExists" (possible gRPC/protobuf message) + // - "409" (HTTP Conflict status code) errMsg := err.Error() if strings.Contains(errMsg, "already exists") || strings.Contains(errMsg, "AlreadyExists") || - strings.Contains(errMsg, "409") { + strings.Contains(errMsg, "409") || + strings.Contains(errMsg, "Conflict") { return nil // Namespace already exists, which is what we want } return fmt.Errorf("failed to create namespace %s: %w", fullNamespace, err) From 0f183fd1520cf5f8a23683caaf4031fcf977ba81 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 5 Jan 2026 07:07:36 +0000 Subject: [PATCH 4/5] refactor(testutil): use case-insensitive error pattern matching - Convert error messages to lowercase for robust pattern matching - Add comment about using case-insensitive matching - Ensures error detection works regardless of message casing Co-authored-by: freeznet <1381618+freeznet@users.noreply.github.com> --- pkg/mcp/internal/testutil/pulsar.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/pkg/mcp/internal/testutil/pulsar.go b/pkg/mcp/internal/testutil/pulsar.go index 7752290..3e48250 100644 --- a/pkg/mcp/internal/testutil/pulsar.go +++ b/pkg/mcp/internal/testutil/pulsar.go @@ -129,11 +129,13 @@ func (h *PulsarTestHelper) EnsureNamespace(ctx context.Context, tenant, namespac // - "already exists" (common HTTP API message) // - "AlreadyExists" (possible gRPC/protobuf message) // - "409" (HTTP Conflict status code) - errMsg := err.Error() + // - "Conflict" (HTTP status text) + // Using case-insensitive matching for robustness + errMsg := strings.ToLower(err.Error()) if strings.Contains(errMsg, "already exists") || - strings.Contains(errMsg, "AlreadyExists") || + strings.Contains(errMsg, "alreadyexists") || strings.Contains(errMsg, "409") || - strings.Contains(errMsg, "Conflict") { + strings.Contains(errMsg, "conflict") { return nil // Namespace already exists, which is what we want } return fmt.Errorf("failed to create namespace %s: %w", fullNamespace, err) From d0635e0c2bbc7bf42bb0ae886bbab8ac0dd0e067 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 5 Jan 2026 07:08:55 +0000 Subject: [PATCH 5/5] style(testutil): apply go fmt formatting and update comments - Run go fmt to ensure consistent formatting - Update comment patterns to reflect lowercase matching - Fix struct field alignment Co-authored-by: freeznet <1381618+freeznet@users.noreply.github.com> --- pkg/mcp/internal/testutil/pulsar.go | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/pkg/mcp/internal/testutil/pulsar.go b/pkg/mcp/internal/testutil/pulsar.go index 3e48250..2d2e5b3 100644 --- a/pkg/mcp/internal/testutil/pulsar.go +++ b/pkg/mcp/internal/testutil/pulsar.go @@ -30,10 +30,10 @@ import ( // PulsarTestHelper provides helper functions for Pulsar E2E testing. type PulsarTestHelper struct { - adminClient pulsaradmin.Client - adminURL string - serviceURL string - httpClient *http.Client + adminClient pulsaradmin.Client + adminURL string + serviceURL string + httpClient *http.Client } // NewPulsarTestHelper creates a new PulsarTestHelper. @@ -127,15 +127,15 @@ func (h *PulsarTestHelper) EnsureNamespace(ctx context.Context, tenant, namespac // The Pulsar admin client doesn't provide typed errors, so we check the error message. // This is a best-effort approach for test utilities - we check for common patterns: // - "already exists" (common HTTP API message) - // - "AlreadyExists" (possible gRPC/protobuf message) + // - "alreadyexists" (possible gRPC/protobuf message) // - "409" (HTTP Conflict status code) - // - "Conflict" (HTTP status text) + // - "conflict" (HTTP status text) // Using case-insensitive matching for robustness errMsg := strings.ToLower(err.Error()) - if strings.Contains(errMsg, "already exists") || - strings.Contains(errMsg, "alreadyexists") || - strings.Contains(errMsg, "409") || - strings.Contains(errMsg, "conflict") { + if strings.Contains(errMsg, "already exists") || + strings.Contains(errMsg, "alreadyexists") || + strings.Contains(errMsg, "409") || + strings.Contains(errMsg, "conflict") { return nil // Namespace already exists, which is what we want } return fmt.Errorf("failed to create namespace %s: %w", fullNamespace, err)