From 84863a3827064ae89cb573848417fe4dc314f0e5 Mon Sep 17 00:00:00 2001 From: Devon Bautista <17506592+synackd@users.noreply.github.com> Date: Thu, 22 Oct 2026 14:49:21 -0600 Subject: [PATCH 01/14] test(cmd): group smd tests by command path Tests of the smd resource commands were spread across smd_test.go, smd_errors_test.go, smd_write_test.go, and smd_write_errors_test.go as well as each resource's own files, so the tests for one command had no single home. Move each smd group, group member, iface, rfe, and compep test into its resource's smd__test.go or _errors_test.go file, leaving smd_test.go and smd_errors_test.go with only the commands directly under smd and the TestSMDDelete_* tables over every smd resource. Retire smd_write_test.go and smd_write_errors_test.go. No test logic changes. Signed-off-by: Devon Bautista <17506592+synackd@users.noreply.github.com> --- cmd/smd_compep_test.go | 51 +++++ cmd/smd_errors_test.go | 182 ++++++++++++--- cmd/smd_group_errors_test.go | 27 +++ cmd/smd_group_member_errors_test.go | 20 ++ cmd/smd_group_member_test.go | 105 ++++++++- cmd/smd_group_test.go | 113 ++++++++++ cmd/smd_iface_test.go | 73 +++++++ cmd/smd_rfe_test.go | 68 ++++++ cmd/smd_test.go | 328 +--------------------------- cmd/smd_write_errors_test.go | 184 ---------------- cmd/smd_write_test.go | 114 ---------- 11 files changed, 614 insertions(+), 651 deletions(-) delete mode 100644 cmd/smd_write_errors_test.go delete mode 100644 cmd/smd_write_test.go diff --git a/cmd/smd_compep_test.go b/cmd/smd_compep_test.go index 0d5ce4d8..93c1730d 100644 --- a/cmd/smd_compep_test.go +++ b/cmd/smd_compep_test.go @@ -152,3 +152,54 @@ func TestSMDCompepDelete_AllFalse(t *testing.T) { t.Errorf("DELETE paths = %v, want [%s]", gotPaths, want) } } + +// TestSMDCompepGet_Success verifies "smd compep get" issues GET +// /Inventory/ComponentEndpoints. +func TestSMDCompepGet_Success(t *testing.T) { + t.Parallel() + + var gotMethod, gotPath string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotPath = r.URL.Path + gotMethod = r.Method + w.Write([]byte(`{}`)) + })) + defer srv.Close() + + res := runOchamiWithRuntime(t, "smd", "compep", "get", "--ignore-config", "--uri", srv.URL, "--token", "t") + if res.err != nil { + t.Fatalf("unexpected error: %v (exit %d)", res.err, res.exitCode) + } + if gotPath != "/Inventory/ComponentEndpoints" { + t.Errorf("path = %q, want /Inventory/ComponentEndpoints", gotPath) + } + if gotMethod != http.MethodGet { + t.Errorf("method = %q, want GET", gotMethod) + } +} + +// TestSMDCompepDelete_NoConfirm verifies "smd compep delete --no-confirm " +// issues DELETE under /Inventory/ComponentEndpoints. +func TestSMDCompepDelete_NoConfirm(t *testing.T) { + t.Parallel() + + var gotMethod, gotPath string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotMethod = r.Method + gotPath = r.URL.Path + w.WriteHeader(http.StatusOK) + })) + defer srv.Close() + + res := runOchamiWithRuntime(t, "smd", "compep", "delete", "--ignore-config", "--uri", srv.URL, "--token", "t", + "--no-confirm", "x0c0s0b0n0") + if res.err != nil { + t.Fatalf("unexpected error: %v (exit %d)", res.err, res.exitCode) + } + if gotMethod != http.MethodDelete { + t.Errorf("method = %q, want DELETE", gotMethod) + } + if gotPath != "/Inventory/ComponentEndpoints/x0c0s0b0n0" { + t.Errorf("path = %q, want /Inventory/ComponentEndpoints/x0c0s0b0n0", gotPath) + } +} diff --git a/cmd/smd_errors_test.go b/cmd/smd_errors_test.go index c8abbd74..d554c3ea 100644 --- a/cmd/smd_errors_test.go +++ b/cmd/smd_errors_test.go @@ -4,8 +4,10 @@ package cmd -// smd_errors_test.go covers HTTP-, transport-, and config-failure paths for -// the "smd" command group; see smd_test.go for the success paths these mirror. +// smd_errors_test.go covers HTTP- and config-failure paths for the commands +// directly under "smd", and the empty or invalid IDs every "smd" delete +// command rejects before sending a request; see smd_test.go and the +// smd__test.go files for the success paths these mirror. import ( "errors" @@ -38,32 +40,6 @@ func TestSMDDeprecatedStatus_HTTPError(t *testing.T) { } } -// TestSMDGroupGet_TruncatedResponseBody verifies that a truncated HTTP response -// body (a declared Content-Length the server doesn't deliver) is reported as -// a network error, not silently swallowed. -func TestSMDGroupGet_TruncatedResponseBody(t *testing.T) { - t.Parallel() - - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - // Declare a body longer than what's written, then close the - // connection without fulfilling Content-Length. - w.Header().Set("Content-Length", "100") - w.Write([]byte("partial")) - })) - defer srv.Close() - - res := runOchamiWithRuntime(t, "--ignore-config", "smd", "group", "get", "--uri", srv.URL, "--token", "t") - if res.err == nil { - t.Fatal("expected an error for a truncated response body, got nil") - } - if res.exitCode != cli.CodeNetwork { - t.Errorf("exit code = %d, want %d (%s)", res.exitCode, cli.CodeNetwork, cli.CodeName(cli.CodeNetwork)) - } - if !strings.Contains(res.err.Error(), "unexpected EOF") { - t.Errorf("error = %q, want it to report the truncated read", res.err.Error()) - } -} - // TestSMDServiceStatus_DuplicateCluster verifies that "smd service status" // warns about and then fails with a config error, without contacting SMD, // when the cluster selected through default-cluster or --cluster is defined @@ -97,3 +73,153 @@ func TestSMDServiceStatus_DuplicateCluster(t *testing.T) { t.Errorf("SMD received %d requests, want 0", n) } } + +// TestSMDDelete_RejectsEmptyData verifies an explicit empty payload cannot +// turn a requested deletion into a silent no-op. +func TestSMDDelete_RejectsEmptyData(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + command string + payload string + wantMsg string + }{ + {name: "interface", command: "iface", payload: `[]`, wantMsg: "payload contained no ethernet interfaces to delete"}, + {name: "group", command: "group", payload: `[]`, wantMsg: "payload contained no groups to delete"}, + {name: "redfish endpoint", command: "rfe", payload: `{"RedfishEndpoints":[]}`, wantMsg: "payload contained no redfish endpoints to delete"}, + {name: "component endpoint", command: "compep", payload: `[]`, wantMsg: "payload contained no component endpoints to delete"}, + {name: "component", command: "component", payload: `{"Components":[]}`, wantMsg: "payload contained no components to delete"}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + var deletes int + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodDelete { + deletes++ + } + })) + defer srv.Close() + + res := runOchamiWithRuntime(t, "smd", "--ignore-config", tc.command, "delete", + "--uri", srv.URL, "--token", "t", "--no-confirm", "-d", tc.payload) + if res.err == nil { + t.Fatal("expected an error, got nil") + } + if res.exitCode != cli.CodeUsage { + t.Errorf("exit code = %d, want %d (%s)", res.exitCode, cli.CodeUsage, cli.CodeName(cli.CodeUsage)) + } + if !strings.Contains(res.err.Error(), tc.wantMsg) { + t.Errorf("error = %q, want it to contain %q", res.err.Error(), tc.wantMsg) + } + if deletes != 0 { + t.Errorf("DELETE count = %d, want 0 (no request should be made for an empty payload)", deletes) + } + }) + } +} + +// TestSMDDelete_RejectsInvalidPayloadIDs verifies that a payload item whose ID +// is blank or is not a single path segment, which would otherwise address the +// whole collection, fails the delete with CodeUsage before any request is +// made, including for the payload's valid items. +func TestSMDDelete_RejectsInvalidPayloadIDs(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + command string + payload string + wantMsg string + }{ + {name: "component without ID", command: "component", payload: `{"Components":[{}]}`, wantMsg: "component ID cannot be blank (item 1 of 1)"}, + {name: "component with dot ID", command: "component", payload: `{"Components":[{"ID":"."}]}`, wantMsg: `component ID "." is not a single path segment`}, + {name: "component after a valid one", command: "component", payload: `{"Components":[{"ID":"x0c0s0b0n0"},{"ID":"x0/.."}]}`, wantMsg: "(item 2 of 2)"}, + {name: "interface without ID", command: "iface", payload: `[{}]`, wantMsg: "ethernet interface ID cannot be blank"}, + {name: "interface with only a MAC address", command: "iface", payload: `[{"MACAddress":"de:ad:be:ef:00:01"}]`, wantMsg: "delete does not derive an ID from MACAddress"}, + {name: "group without label", command: "group", payload: `[{}]`, wantMsg: "group label cannot be blank"}, + {name: "redfish endpoint without ID", command: "rfe", payload: `{"RedfishEndpoints":[{}]}`, wantMsg: "redfish endpoint ID cannot be blank"}, + {name: "component endpoint without ID", command: "compep", payload: `[{}]`, wantMsg: "component endpoint ID cannot be blank"}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + var deletes int + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodDelete { + deletes++ + } + })) + defer srv.Close() + + res := runOchamiWithRuntime(t, "smd", "--ignore-config", tc.command, "delete", + "--uri", srv.URL, "--token", "t", "--no-confirm", "-d", tc.payload) + if res.err == nil { + t.Fatal("expected an error, got nil") + } + if res.exitCode != cli.CodeUsage { + t.Errorf("exit code = %d, want %d (%s)", res.exitCode, cli.CodeUsage, cli.CodeName(cli.CodeUsage)) + } + if !strings.Contains(res.err.Error(), tc.wantMsg) { + t.Errorf("error = %q, want it to contain %q", res.err.Error(), tc.wantMsg) + } + if deletes != 0 { + t.Errorf("DELETE count = %d, want 0", deletes) + } + }) + } +} + +// TestSMDDelete_RejectsInvalidArgumentIDs verifies that a positional ID that +// is blank or is not a single path segment fails the delete with CodeUsage +// before any request is made, including for the valid arguments. +func TestSMDDelete_RejectsInvalidArgumentIDs(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + args []string + wantMsg string + }{ + {name: "blank component", args: []string{"component", "delete", ""}, wantMsg: "component ID cannot be blank"}, + {name: "component after a valid one", args: []string{"component", "delete", "x0c0s0b0n0", ".."}, wantMsg: `component ID ".." is not a single path segment (item 2 of 2)`}, + {name: "interface with slash", args: []string{"iface", "delete", "x0/.."}, wantMsg: "ethernet interface ID"}, + {name: "blank group", args: []string{"group", "delete", ""}, wantMsg: "group label cannot be blank"}, + {name: "blank redfish endpoint", args: []string{"rfe", "delete", " "}, wantMsg: "redfish endpoint ID cannot be blank"}, + {name: "dot component endpoint", args: []string{"compep", "delete", "."}, wantMsg: "component endpoint ID"}, + {name: "blank member group", args: []string{"group", "member", "delete", "", "x0c0s0b0n0"}, wantMsg: "group label cannot be blank"}, + {name: "blank group member", args: []string{"group", "member", "delete", "compute", ""}, wantMsg: "group member cannot be blank"}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + var deletes int + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodDelete { + deletes++ + } + })) + defer srv.Close() + + args := append([]string{"smd", "--ignore-config"}, tc.args...) + args = append(args, "--uri", srv.URL, "--token", "t", "--no-confirm") + res := runOchamiWithRuntime(t, args...) + if res.err == nil { + t.Fatal("expected an error, got nil") + } + if res.exitCode != cli.CodeUsage { + t.Errorf("exit code = %d, want %d (%s)", res.exitCode, cli.CodeUsage, cli.CodeName(cli.CodeUsage)) + } + if !strings.Contains(res.err.Error(), tc.wantMsg) { + t.Errorf("error = %q, want it to contain %q", res.err.Error(), tc.wantMsg) + } + if deletes != 0 { + t.Errorf("DELETE count = %d, want 0", deletes) + } + }) + } +} diff --git a/cmd/smd_group_errors_test.go b/cmd/smd_group_errors_test.go index b0198d73..7ee3c973 100644 --- a/cmd/smd_group_errors_test.go +++ b/cmd/smd_group_errors_test.go @@ -7,6 +7,7 @@ package cmd import ( "net/http" "net/http/httptest" + "strings" "testing" "github.com/openchami/ochami/internal/cli" @@ -172,3 +173,29 @@ func TestSMDGroupAdd_NetworkError(t *testing.T) { t.Errorf("exit code = %d, want %d (%s)", res.exitCode, cli.CodeNetwork, cli.CodeName(cli.CodeNetwork)) } } + +// TestSMDGroupGet_TruncatedResponseBody verifies that a truncated HTTP response +// body (a declared Content-Length the server doesn't deliver) is reported as +// a network error, not silently swallowed. +func TestSMDGroupGet_TruncatedResponseBody(t *testing.T) { + t.Parallel() + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // Declare a body longer than what's written, then close the + // connection without fulfilling Content-Length. + w.Header().Set("Content-Length", "100") + w.Write([]byte("partial")) + })) + defer srv.Close() + + res := runOchamiWithRuntime(t, "--ignore-config", "smd", "group", "get", "--uri", srv.URL, "--token", "t") + if res.err == nil { + t.Fatal("expected an error for a truncated response body, got nil") + } + if res.exitCode != cli.CodeNetwork { + t.Errorf("exit code = %d, want %d (%s)", res.exitCode, cli.CodeNetwork, cli.CodeName(cli.CodeNetwork)) + } + if !strings.Contains(res.err.Error(), "unexpected EOF") { + t.Errorf("error = %q, want it to report the truncated read", res.err.Error()) + } +} diff --git a/cmd/smd_group_member_errors_test.go b/cmd/smd_group_member_errors_test.go index d3e0d017..04dbf044 100644 --- a/cmd/smd_group_member_errors_test.go +++ b/cmd/smd_group_member_errors_test.go @@ -101,3 +101,23 @@ func TestSMDGroupMember_NetworkErrors(t *testing.T) { t.Errorf("member get network: err=%v exit=%d, want %d (%s)", res.err, res.exitCode, cli.CodeNetwork, cli.CodeName(cli.CodeNetwork)) } } + +// TestSMDGroupMemberAdd_HTTPError verifies an unsuccessful HTTP response +// resolves to CodeHTTP. +func TestSMDGroupMemberAdd_HTTPError(t *testing.T) { + t.Parallel() + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Error(w, "conflict", http.StatusConflict) + })) + defer srv.Close() + + res := runOchamiWithRuntime(t, "smd", "--ignore-config", "group", "member", "add", "compute", "x0c0s0b0n0", + "--uri", srv.URL, "--token", "t") + if res.err == nil { + t.Fatal("expected an error, got nil") + } + if res.exitCode != cli.CodeHTTP { + t.Errorf("exit code = %d, want %d (%s)", res.exitCode, cli.CodeHTTP, cli.CodeName(cli.CodeHTTP)) + } +} diff --git a/cmd/smd_group_member_test.go b/cmd/smd_group_member_test.go index d42657f4..9d611cb9 100644 --- a/cmd/smd_group_member_test.go +++ b/cmd/smd_group_member_test.go @@ -5,9 +5,10 @@ package cmd // smd_group_member_test.go exercises the success paths of the "smd group -// member" add and delete commands end-to-end against an httptest.Server: one -// request per member, and the delete confirmation prompt. Error arms for every -// member verb are covered in smd_group_member_errors_test.go. +// member" commands (get, add, set, delete) end-to-end against an +// httptest.Server: the request each verb issues, one request per member, and +// the delete confirmation prompt. Error arms for every member verb are covered +// in smd_group_member_errors_test.go. import ( "net/http" @@ -91,3 +92,101 @@ func TestSMDGroupMemberDelete_Multiple(t *testing.T) { t.Errorf("DELETE count = %d, want 2", deletes) } } + +// TestSMDGroupMemberGet_Success verifies "smd group member get