diff --git a/cmd/cmdutil/errors_test.go b/cmd/cmdutil/errors_test.go new file mode 100644 index 0000000..c7a27ba --- /dev/null +++ b/cmd/cmdutil/errors_test.go @@ -0,0 +1,86 @@ +package cmdutil + +import ( + "errors" + "fmt" + "testing" + + "github.com/patramsey/namecom-cli/internal/api" +) + +// IsNotFound is what turns a bare 404 into "domain not found — run 'namecom +// domain list'" across the whole CLI. It matches through wrapping, so the +// wrapped cases matter as much as the direct one: commands routinely add +// context with %w before the error reaches a caller that checks this. +func TestIsNotFound(t *testing.T) { + notFound := &api.APIError{StatusCode: 404, Message: "Domain not found"} + + tests := []struct { + name string + err error + want bool + }{ + {"nil is not a 404", nil, false}, + {"a plain error is not a 404", errors.New("boom"), false}, + {"a direct 404", notFound, true}, + {"a 404 wrapped once", fmt.Errorf("fetching domain: %w", notFound), true}, + {"a 404 wrapped twice", fmt.Errorf("outer: %w", fmt.Errorf("inner: %w", notFound)), true}, + {"a 404 behind a UsageError", &UsageError{Err: notFound}, true}, + {"403 is not a 404", &api.APIError{StatusCode: 403, Message: "Forbidden"}, false}, + {"500 is not a 404", &api.APIError{StatusCode: 500}, false}, + {"400 is not a 404", &api.APIError{StatusCode: 400}, false}, + {"an unwrapped error mentioning 404 is not a 404", errors.New("got status 404"), false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := IsNotFound(tt.err); got != tt.want { + t.Errorf("IsNotFound(%v) = %v, want %v", tt.err, got, tt.want) + } + }) + } +} + +// UsageError marks a problem with how the command was invoked, which the root +// command turns into a different exit code than an API failure. That only +// works if it stays both matchable and unwrappable. +func TestUsageError(t *testing.T) { + inner := errors.New("bad flag combination") + ue := &UsageError{Err: inner} + + if got := ue.Error(); got != inner.Error() { + t.Errorf("Error() = %q, want the inner message %q", got, inner.Error()) + } + if got := ue.Unwrap(); !errors.Is(got, inner) { + t.Errorf("Unwrap() = %v, want %v", got, inner) + } + if !errors.Is(ue, inner) { + t.Error("errors.Is(UsageError, inner) = false; the chain is broken") + } + + var target *UsageError + if !errors.As(fmt.Errorf("context: %w", ue), &target) { + t.Error("a wrapped UsageError should still be findable with errors.As") + } +} + +// NewUsageError returns nil for nil so callers can apply it straight to a +// function result. Returning a non-nil *UsageError wrapping nil would make +// `if err != nil` true on success — the classic typed-nil trap. +func TestNewUsageError(t *testing.T) { + if err := NewUsageError(nil); err != nil { + t.Errorf("NewUsageError(nil) = %v, want nil", err) + } + + inner := errors.New("boom") + err := NewUsageError(inner) + if err == nil { + t.Fatal("NewUsageError(non-nil) = nil, want an error") + } + var ue *UsageError + if !errors.As(err, &ue) { + t.Fatalf("NewUsageError returned %T, want a *UsageError", err) + } + if !errors.Is(err, inner) { + t.Error("NewUsageError should preserve the original error in the chain") + } +} diff --git a/cmd/root.go b/cmd/root.go index ad7b271..8f3ec36 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -367,12 +367,6 @@ func initContext(cmd *cobra.Command) error { return nil } -// IsYes reports whether --yes / -y was set globally (skip confirmation). -func IsYes() bool { return gf.yes } - -// IsDryRun reports whether --dry-run was set globally. -func IsDryRun() bool { return gf.dryRun } - // skipClientInit returns true for commands that don't need API credentials. func skipClientInit(cmd *cobra.Command) bool { for c := cmd; c != nil; c = c.Parent() { diff --git a/internal/output/output_test.go b/internal/output/output_test.go index 71b67c0..5771acd 100644 --- a/internal/output/output_test.go +++ b/internal/output/output_test.go @@ -366,3 +366,127 @@ func TestSuccess_StructuredFormats(t *testing.T) { } }) } + +// ---- flag value parsing ----------------------------------------------------- + +// ParseFormat and ParseColorMode validate --output and --color. Both lowercase +// their input, so mixed case must be accepted — nothing previously proved that, +// and `--output JSON` silently erroring would be a poor way to find out. +func TestParseFormat(t *testing.T) { + tests := []struct { + in string + want Format + wantErr bool + }{ + {"table", FormatTable, false}, + {"json", FormatJSON, false}, + {"yaml", FormatYAML, false}, + {"JSON", FormatJSON, false}, + {"YaMl", FormatYAML, false}, + {"TABLE", FormatTable, false}, + {"", "", true}, + {"xml", "", true}, + {"jsonl", "", true}, + {" json", "", true}, // not trimmed: a stray space is a real mistake, not a synonym + } + for _, tt := range tests { + t.Run(tt.in, func(t *testing.T) { + got, err := ParseFormat(tt.in) + if tt.wantErr { + if err == nil { + t.Fatalf("ParseFormat(%q) = %q, want an error", tt.in, got) + } + // The message has to name the valid choices; "unknown format" + // alone leaves the user guessing. + for _, want := range []string{"table", "json", "yaml"} { + if !strings.Contains(err.Error(), want) { + t.Errorf("error %q should list %q as a valid choice", err, want) + } + } + return + } + if err != nil { + t.Fatalf("ParseFormat(%q) unexpected error: %v", tt.in, err) + } + if got != tt.want { + t.Errorf("ParseFormat(%q) = %q, want %q", tt.in, got, tt.want) + } + }) + } +} + +func TestParseColorMode(t *testing.T) { + tests := []struct { + in string + want ColorMode + wantErr bool + }{ + {"auto", ColorAuto, false}, + {"always", ColorAlways, false}, + {"never", ColorNever, false}, + {"ALWAYS", ColorAlways, false}, + {"Never", ColorNever, false}, + {"", "", true}, + {"yes", "", true}, + {"true", "", true}, + {"none", "", true}, + } + for _, tt := range tests { + t.Run(tt.in, func(t *testing.T) { + got, err := ParseColorMode(tt.in) + if tt.wantErr { + if err == nil { + t.Fatalf("ParseColorMode(%q) = %q, want an error", tt.in, got) + } + for _, want := range []string{"auto", "always", "never"} { + if !strings.Contains(err.Error(), want) { + t.Errorf("error %q should list %q as a valid choice", err, want) + } + } + return + } + if err != nil { + t.Fatalf("ParseColorMode(%q) unexpected error: %v", tt.in, err) + } + if got != tt.want { + t.Errorf("ParseColorMode(%q) = %q, want %q", tt.in, got, tt.want) + } + }) + } +} + +// ---- color helpers ---------------------------------------------------------- + +// Red and Amber must return the text unchanged when color is off. A helper that +// emits escape sequences regardless would corrupt piped output and any file +// written with --debug-file. +func TestRedAmber_NoColorReturnsPlainText(t *testing.T) { + c := noColor() + for _, tt := range []struct { + name string + got string + }{ + {"Red", c.Red("failed")}, + {"Amber", c.Amber("careful")}, + } { + if strings.ContainsRune(tt.got, '\x1b') { + t.Errorf("%s with color disabled returned an escape sequence: %q", tt.name, tt.got) + } + } + if got := c.Red("failed"); got != "failed" { + t.Errorf("Red = %q, want %q unchanged", got, "failed") + } + if got := c.Amber("careful"); got != "careful" { + t.Errorf("Amber = %q, want %q unchanged", got, "careful") + } +} + +func TestRedAmber_ColorWrapsButPreservesText(t *testing.T) { + c := &Config{Color: ColorAlways} + if got := c.Red("failed"); !strings.Contains(got, "failed") { + t.Errorf("Red = %q, want it to still contain %q", got, "failed") + } + if got := c.Amber("careful"); !strings.Contains(got, "careful") { + t.Errorf("Amber = %q, want it to still contain %q", got, "careful") + } +} diff --git a/internal/update/cache_test.go b/internal/update/cache_test.go new file mode 100644 index 0000000..4553b3e --- /dev/null +++ b/internal/update/cache_test.go @@ -0,0 +1,176 @@ +package update + +import ( + "encoding/json" + "os" + "path/filepath" + "runtime" + "testing" + "time" +) + +// isolateConfigDir points os.UserConfigDir() at a temp directory. The variable +// it reads differs by platform: HOME on darwin ($HOME/Library/Application +// Support), XDG_CONFIG_HOME on unix. Setting both keeps this test honest on +// whichever one is running it rather than passing vacuously on the other. +func isolateConfigDir(t *testing.T) string { + t.Helper() + dir := t.TempDir() + t.Setenv("HOME", dir) + t.Setenv("XDG_CONFIG_HOME", filepath.Join(dir, ".config")) + if runtime.GOOS == "windows" { + t.Setenv("AppData", dir) + } + got := cacheFile() + if got == "" { + t.Fatal("cacheFile() is empty after isolating the config dir") + } + return got +} + +func TestCacheFile_UnderConfigDir(t *testing.T) { + path := isolateConfigDir(t) + if filepath.Base(path) != "version_check.json" { + t.Errorf("cache file = %q, want it to be named version_check.json", path) + } + if filepath.Base(filepath.Dir(path)) != "namecom" { + t.Errorf("cache file = %q, want it under a namecom/ directory", path) + } +} + +// The round trip is the contract: what writeCache stores, readCache returns. +func TestCache_RoundTrip(t *testing.T) { + isolateConfigDir(t) + + if got, ok := readCache(); ok { + t.Fatalf("readCache on an empty dir = (%q, true), want ok=false", got) + } + + writeCache("1.2.3") + + got, ok := readCache() + if !ok { + t.Fatal("readCache after writeCache = ok:false, want the value back") + } + if got != "1.2.3" { + t.Errorf("readCache = %q, want %q", got, "1.2.3") + } +} + +// The cache exists to keep the CLI off the network for a day. An entry older +// than the TTL must be ignored, or the check never refreshes; an entry inside +// it must be honored, or the cache does nothing and every invocation hits +// GitHub. +func TestCache_TTL(t *testing.T) { + tests := []struct { + name string + age time.Duration + wantOK bool + wantVal string + }{ + {"fresh entry is used", time.Hour, true, "1.2.3"}, + {"just inside the TTL is used", cacheTTL - time.Minute, true, "1.2.3"}, + {"just past the TTL is ignored", cacheTTL + time.Minute, false, ""}, + {"ancient entry is ignored", 30 * 24 * time.Hour, false, ""}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + path := isolateConfigDir(t) + if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil { + t.Fatalf("MkdirAll: %v", err) + } + body, err := json.Marshal(versionCache{ + CheckedAt: time.Now().Add(-tt.age), + Latest: "1.2.3", + }) + if err != nil { + t.Fatalf("Marshal: %v", err) + } + if err := os.WriteFile(path, body, 0o600); err != nil { + t.Fatalf("WriteFile: %v", err) + } + + got, ok := readCache() + if ok != tt.wantOK { + t.Errorf("readCache ok = %v, want %v (age %s, TTL %s)", ok, tt.wantOK, tt.age, cacheTTL) + } + if got != tt.wantVal { + t.Errorf("readCache = %q, want %q", got, tt.wantVal) + } + }) + } +} + +// Every failure path in readCache is deliberately silent — a broken cache must +// degrade to "no cached value", never to an error surfacing in the CLI or a +// bogus version being reported. +func TestReadCache_BadInputIsSilentlyIgnored(t *testing.T) { + tests := []struct { + name string + content string + }{ + {"not json", "this is not json"}, + {"truncated json", `{"checked_at":`}, + {"wrong shape", `[1,2,3]`}, + {"empty file", ""}, + {"valid json, empty version", `{"checked_at":"` + time.Now().Format(time.RFC3339) + `","latest":""}`}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + path := isolateConfigDir(t) + if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil { + t.Fatalf("MkdirAll: %v", err) + } + if err := os.WriteFile(path, []byte(tt.content), 0o600); err != nil { + t.Fatalf("WriteFile: %v", err) + } + if got, ok := readCache(); ok || got != "" { + t.Errorf("readCache = (%q, %v), want (\"\", false) for %s", got, ok, tt.name) + } + }) + } +} + +// The cache can hold the user's update history; it does not belong to the +// group or to other users. +func TestWriteCache_FilePermissions(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("unix permission bits do not apply on windows") + } + path := isolateConfigDir(t) + writeCache("1.2.3") + + fi, err := os.Stat(path) + if err != nil { + t.Fatalf("stat: %v", err) + } + if perm := fi.Mode().Perm(); perm&0o077 != 0 { + t.Errorf("cache file mode = %#o, want no group/other bits", perm) + } + di, err := os.Stat(filepath.Dir(path)) + if err != nil { + t.Fatalf("stat dir: %v", err) + } + if perm := di.Mode().Perm(); perm&0o077 != 0 { + t.Errorf("cache dir mode = %#o, want no group/other bits", perm) + } +} + +// writeCache swallows its errors by design; the CLI must not fail because a +// version check could not be cached. +func TestWriteCache_UnwritablePathDoesNotPanic(t *testing.T) { + path := isolateConfigDir(t) + // Occupy the directory slot with a regular file so MkdirAll must fail. + if err := os.MkdirAll(filepath.Dir(filepath.Dir(path)), 0o700); err != nil { + t.Fatalf("MkdirAll: %v", err) + } + if err := os.WriteFile(filepath.Dir(path), []byte("not a directory"), 0o600); err != nil { + t.Fatalf("WriteFile: %v", err) + } + + writeCache("1.2.3") // must not panic + + if got, ok := readCache(); ok || got != "" { + t.Errorf("readCache = (%q, %v), want (\"\", false) when the cache could not be written", got, ok) + } +}