From 8ee54817acfd8f717cb0e244da808a7501e1edef Mon Sep 17 00:00:00 2001 From: Shay Rojansky Date: Tue, 14 Jul 2026 17:03:08 +0200 Subject: [PATCH] Fix Go e2e nil guards for staticcheck Restructure nil checks in Go E2E tests so dereferences stay in the proven non-nil branch. This addresses SA5011 findings from newer golangci-lint/staticcheck without suppressing lint rules or pinning older tools. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1e4710a8-2700-4e51-ae9c-657aa8215080 --- go/internal/e2e/event_fidelity_e2e_test.go | 3 +- go/internal/e2e/rpc_server_e2e_test.go | 6 ++-- go/internal/e2e/rpc_server_misc_e2e_test.go | 17 ++++----- .../e2e/rpc_server_plugins_e2e_test.go | 6 ++-- .../e2e/rpc_ui_ephemeral_query_e2e_test.go | 6 ++-- .../e2e/system_message_sections_e2e_test.go | 36 +++++++++---------- go/internal/e2e/tools_e2e_test.go | 31 ++++++++-------- 7 files changed, 50 insertions(+), 55 deletions(-) diff --git a/go/internal/e2e/event_fidelity_e2e_test.go b/go/internal/e2e/event_fidelity_e2e_test.go index c48a4908a1..ec1f30c512 100644 --- a/go/internal/e2e/event_fidelity_e2e_test.go +++ b/go/internal/e2e/event_fidelity_e2e_test.go @@ -168,8 +168,7 @@ func TestEventFidelityE2E(t *testing.T) { if answer == nil { t.Fatal("Expected SendAndWait to return an assistant message") - } - if ad, ok := answer.Data.(*copilot.AssistantMessageData); !ok || !strings.Contains(ad.Content, "18") { + } else if ad, ok := answer.Data.(*copilot.AssistantMessageData); !ok || !strings.Contains(ad.Content, "18") { t.Errorf("Expected answer to contain '18', got %v", answer.Data) } }) diff --git a/go/internal/e2e/rpc_server_e2e_test.go b/go/internal/e2e/rpc_server_e2e_test.go index 2510eb1d9c..e3916cf464 100644 --- a/go/internal/e2e/rpc_server_e2e_test.go +++ b/go/internal/e2e/rpc_server_e2e_test.go @@ -604,8 +604,7 @@ func TestRPCServerE2E(t *testing.T) { projectSkillPath := findSkillDiscoveryPath(skillPaths.Paths, ctx.WorkDir) if projectSkillPath == nil { t.Fatalf("Expected skill discovery paths to include %q", ctx.WorkDir) - } - if strings.TrimSpace(projectSkillPath.Path) == "" { + } else if strings.TrimSpace(projectSkillPath.Path) == "" { t.Fatal("Expected non-empty skill discovery path") } @@ -632,8 +631,7 @@ func TestRPCServerE2E(t *testing.T) { projectAgentPath := findAgentDiscoveryPath(agentPaths.Paths, ctx.WorkDir) if projectAgentPath == nil { t.Fatalf("Expected agent discovery paths to include %q", ctx.WorkDir) - } - if strings.TrimSpace(projectAgentPath.Path) == "" { + } else if strings.TrimSpace(projectAgentPath.Path) == "" { t.Fatal("Expected non-empty agent discovery path") } diff --git a/go/internal/e2e/rpc_server_misc_e2e_test.go b/go/internal/e2e/rpc_server_misc_e2e_test.go index 38b569798c..9eaf190c42 100644 --- a/go/internal/e2e/rpc_server_misc_e2e_test.go +++ b/go/internal/e2e/rpc_server_misc_e2e_test.go @@ -159,14 +159,15 @@ func TestRpcServerMisc(t *testing.T) { } if users == nil { t.Fatal("Expected non-nil users result") - } - for _, user := range *users { - userInfo, ok := user.AuthInfo.(*rpc.UserAuthInfo) - if !ok { - t.Fatalf("Expected user auth info in all users, got %#v", user.AuthInfo) - } - if userInfo.Login == "go-account-user" && (user.Token == nil || *user.Token != "go-account-token") { - t.Fatalf("Expected logged-in user's token to round trip, got %+v", user) + } else { + for _, user := range *users { + userInfo, ok := user.AuthInfo.(*rpc.UserAuthInfo) + if !ok { + t.Fatalf("Expected user auth info in all users, got %#v", user.AuthInfo) + } + if userInfo.Login == "go-account-user" && (user.Token == nil || *user.Token != "go-account-token") { + t.Fatalf("Expected logged-in user's token to round trip, got %+v", user) + } } } diff --git a/go/internal/e2e/rpc_server_plugins_e2e_test.go b/go/internal/e2e/rpc_server_plugins_e2e_test.go index 1e24a769f0..3dc759f95d 100644 --- a/go/internal/e2e/rpc_server_plugins_e2e_test.go +++ b/go/internal/e2e/rpc_server_plugins_e2e_test.go @@ -58,8 +58,7 @@ func TestRpcServerPlugins(t *testing.T) { listed := findPortedInstalledPlugin(afterInstall.Plugins, portedPluginName, portedMarketplaceName) if listed == nil { t.Fatalf("Expected installed plugin %q in marketplace %q", portedPluginName, portedMarketplaceName) - } - if !listed.Enabled { + } else if !listed.Enabled { t.Fatal("Expected listed marketplace plugin to be enabled") } @@ -229,8 +228,7 @@ func TestRpcServerPlugins(t *testing.T) { mine := findPortedMarketplace(list.Marketplaces, portedMarketplaceName) if mine == nil { t.Fatalf("Expected marketplace %q in list %+v", portedMarketplaceName, list.Marketplaces) - } - if mine.IsDefault != nil && *mine.IsDefault { + } else if mine.IsDefault != nil && *mine.IsDefault { t.Fatal("Expected local marketplace not to be marked default") } if !containsPortedDefaultMarketplace(list.Marketplaces) { diff --git a/go/internal/e2e/rpc_ui_ephemeral_query_e2e_test.go b/go/internal/e2e/rpc_ui_ephemeral_query_e2e_test.go index c6e4033609..e11b7b8ad2 100644 --- a/go/internal/e2e/rpc_ui_ephemeral_query_e2e_test.go +++ b/go/internal/e2e/rpc_ui_ephemeral_query_e2e_test.go @@ -26,11 +26,9 @@ func TestRpcUiEphemeralQuery(t *testing.T) { } if result == nil { t.Fatal("Expected non-nil ephemeral query result") - } - if strings.TrimSpace(result.Answer) == "" { + } else if strings.TrimSpace(result.Answer) == "" { t.Fatal("Expected non-empty ephemeral query answer") - } - if !strings.Contains(strings.ToLower(result.Answer), "blue") { + } else if !strings.Contains(strings.ToLower(result.Answer), "blue") { t.Fatalf("Expected answer to contain blue, got %q", result.Answer) } }) diff --git a/go/internal/e2e/system_message_sections_e2e_test.go b/go/internal/e2e/system_message_sections_e2e_test.go index 61493c8122..bb24c9b2b7 100644 --- a/go/internal/e2e/system_message_sections_e2e_test.go +++ b/go/internal/e2e/system_message_sections_e2e_test.go @@ -43,15 +43,15 @@ func TestSystemMessageSectionsE2E(t *testing.T) { } if response == nil { t.Fatal("Expected a response from the assistant") - } - - ad, ok := response.Data.(*copilot.AssistantMessageData) - if !ok { - t.Fatalf("Expected AssistantMessageData, got %T", response.Data) - } - content := strings.ToLower(ad.Content) - if !strings.Contains(content, "botanica") && !strings.Contains(content, "garden") && !strings.Contains(content, "plant") { - t.Errorf("Expected response to reflect the replaced identity section, but got: %s", ad.Content) + } else { + ad, ok := response.Data.(*copilot.AssistantMessageData) + if !ok { + t.Fatalf("Expected AssistantMessageData, got %T", response.Data) + } + content := strings.ToLower(ad.Content) + if !strings.Contains(content, "botanica") && !strings.Contains(content, "garden") && !strings.Contains(content, "plant") { + t.Errorf("Expected response to reflect the replaced identity section, but got: %s", ad.Content) + } } }) @@ -82,15 +82,15 @@ func TestSystemMessageSectionsE2E(t *testing.T) { } if response == nil { t.Fatal("Expected a response from the assistant") - } - - ad, ok := response.Data.(*copilot.AssistantMessageData) - if !ok { - t.Fatalf("Expected AssistantMessageData, got %T", response.Data) - } - content := strings.ToLower(ad.Content) - if !strings.Contains(content, "botanica") && !strings.Contains(content, "garden") && !strings.Contains(content, "plant") { - t.Errorf("Expected response to reflect the replaced preamble section, but got: %s", ad.Content) + } else { + ad, ok := response.Data.(*copilot.AssistantMessageData) + if !ok { + t.Fatalf("Expected AssistantMessageData, got %T", response.Data) + } + content := strings.ToLower(ad.Content) + if !strings.Contains(content, "botanica") && !strings.Contains(content, "garden") && !strings.Contains(content, "plant") { + t.Errorf("Expected response to reflect the replaced preamble section, but got: %s", ad.Content) + } } }) } diff --git a/go/internal/e2e/tools_e2e_test.go b/go/internal/e2e/tools_e2e_test.go index 1600eb1630..c5b0642631 100644 --- a/go/internal/e2e/tools_e2e_test.go +++ b/go/internal/e2e/tools_e2e_test.go @@ -140,22 +140,23 @@ func TestToolsE2E(t *testing.T) { if answer == nil { t.Fatalf("Expected non-nil assistant message") - } - ad, ok := answer.Data.(*copilot.AssistantMessageData) - if !ok { - t.Fatalf("Expected AssistantMessageData") - } + } else { + ad, ok := answer.Data.(*copilot.AssistantMessageData) + if !ok { + t.Fatalf("Expected AssistantMessageData") + } - content := ad.Content - if content == "" { - t.Fatalf("Expected non-empty response") - } - lower := strings.ToLower(content) - if !strings.Contains(lower, "analyzing") { - t.Errorf("Expected response to contain 'analyzing', got %q", content) - } - if !strings.Contains(lower, "item_alpha") && !strings.Contains(lower, "item_beta") { - t.Errorf("Expected response to contain 'item_alpha' or 'item_beta', got %q", content) + content := ad.Content + if content == "" { + t.Fatalf("Expected non-empty response") + } + lower := strings.ToLower(content) + if !strings.Contains(lower, "analyzing") { + t.Errorf("Expected response to contain 'analyzing', got %q", content) + } + if !strings.Contains(lower, "item_alpha") && !strings.Contains(lower, "item_beta") { + t.Errorf("Expected response to contain 'item_alpha' or 'item_beta', got %q", content) + } } mu.Lock() gotPhase := currentPhase