From c7ddbe2e3677a75869181f38aad74be368cd87d0 Mon Sep 17 00:00:00 2001 From: Andrei Aaron Date: Fri, 8 May 2026 20:19:26 +0300 Subject: [PATCH] feat(zli): add config list/show/get/set/reset and isolate deprecated syntax (#4037) * feat(zli): add config list/show/get/set/reset and isolate deprecated syntax Introduce first-class subcommands for listing profiles, showing a profile, getting and setting keys, and resetting optional keys (alongside existing add/remove). The parent command now resolves ~/.zot via zliUserConfigPath(), documents that profile names must not clash with subcommand names, and states that positional/--list/--reset usage is deprecated and will be removed soon. Legacy behavior is delegated to config_cmd_deprecated.go with stderr warnings for old flags and positional get/set. Examples and inline help point users at the new commands. FormatNames/FormatListedVars comments reference config list/show. Tests are split so config_cmd_test.go exercises the supported subcommands while config_cmd_deprecated_test.go retains coverage for the deprecated paths under renamed TestConfigCmdDeprecated* entries. Signed-off-by: Andrei Aaron * test: stabilize retention check tests See https://github.com/project-zot/zot/actions/runs/25361779632/job/74362802944?pr=4037 Signed-off-by: Andrei Aaron --------- Signed-off-by: Andrei Aaron --- errors/errors.go | 1 + pkg/cli/client/config.go | 4 +- pkg/cli/client/config_cmd.go | 280 +++++++--- pkg/cli/client/config_cmd_deprecated.go | 89 +++ pkg/cli/client/config_cmd_deprecated_test.go | 542 +++++++++++++++++++ pkg/cli/client/config_cmd_test.go | 405 +++++++------- pkg/cli/server/verify_retention_test.go | 54 ++ 7 files changed, 1108 insertions(+), 267 deletions(-) create mode 100644 pkg/cli/client/config_cmd_deprecated.go create mode 100644 pkg/cli/client/config_cmd_deprecated_test.go diff --git a/errors/errors.go b/errors/errors.go index 6bf99f68..1e2a6ebb 100644 --- a/errors/errors.go +++ b/errors/errors.go @@ -87,6 +87,7 @@ var ( ErrScanNotSupported = errors.New("scanning is not supported for given media type") ErrCLITimeout = errors.New("query timed out while waiting for results") ErrDuplicateConfigName = errors.New("cli config name already added") + ErrReservedConfigName = errors.New("cli config name is reserved") ErrInvalidRoute = errors.New("invalid route prefix") ErrImgStoreNotFound = errors.New("image store not found corresponding to given route") ErrLocalImgStoreNotFound = errors.New("local image store not found corresponding to given route") diff --git a/pkg/cli/client/config.go b/pkg/cli/client/config.go index 7f39c24a..1a770092 100644 --- a/pkg/cli/client/config.go +++ b/pkg/cli/client/config.go @@ -217,7 +217,7 @@ func (f *ZliConfigFile) RemoveEntry(configName string) error { return zerr.ErrConfigNotFound } -// FormatNames renders name and URL columns for `zli config --list`. +// FormatNames renders name and URL columns for `zli config list`. func (f *ZliConfigFile) FormatNames() (string, error) { var builder strings.Builder @@ -317,7 +317,7 @@ func (c *ZliConfig) ResetVar(key string) error { return nil } -// FormatListedVars renders lines for `zli config --list`. +// FormatListedVars renders lines for `zli config show `. func (c *ZliConfig) FormatListedVars() string { var builder strings.Builder diff --git a/pkg/cli/client/config_cmd.go b/pkg/cli/client/config_cmd.go index f24bda7e..9707bf91 100644 --- a/pkg/cli/client/config_cmd.go +++ b/pkg/cli/client/config_cmd.go @@ -6,6 +6,9 @@ import ( "fmt" "os" "path/filepath" + "slices" + "sort" + "strings" "github.com/spf13/cobra" @@ -18,80 +21,89 @@ func NewConfigCommand() *cobra.Command { var isReset bool configCmd := &cobra.Command{ - Use: "config [variable] [value]", + Use: "config", Example: examples, Short: "Configure zot registry parameters for CLI", - Long: `Configure zot registry parameters for CLI`, Args: cobra.ArbitraryArgs, RunE: func(cmd *cobra.Command, args []string) error { - home, err := os.UserHomeDir() + configPath, err := zliUserConfigPath() if err != nil { return err } - configPath := filepath.Join(home, ".zot") - - switch len(args) { - case noArgs: - if isListing { // zli config -l - res, err := getConfigNames(configPath) - if err != nil { - return err - } - - fmt.Fprint(cmd.OutOrStdout(), res) - - return nil - } - - return zerr.ErrInvalidArgs - case oneArg: - // zli config -l - if isListing { - res, err := getAllConfig(configPath, args[0]) - if err != nil { - return err - } - - fmt.Fprint(cmd.OutOrStdout(), res) - - return nil - } - - return zerr.ErrInvalidArgs - case twoArgs: - if isReset { // zli config --reset - return resetConfigValue(configPath, args[0], args[1]) - } - // zli config - res, err := getConfigValue(configPath, args[0], args[1]) - if err != nil { - return err - } - fmt.Fprintln(cmd.OutOrStdout(), res) - case threeArgs: - // zli config - if err := setConfigValue(configPath, args[0], args[1], args[2]); err != nil { - return err - } - - default: - return zerr.ErrInvalidArgs - } - - return nil + return runLegacyConfig(cmd, args, configPath, isListing, isReset) }, } - configCmd.Flags().BoolVarP(&isListing, "list", "l", false, "List configurations") - configCmd.Flags().BoolVar(&isReset, "reset", false, "Reset a variable value") + configCmd.Flags().BoolVarP(&isListing, "list", "l", false, + "[deprecated: use \"config list\" or \"config show \"] List configurations") + + configCmd.Flags().BoolVar(&isReset, "reset", false, + "[deprecated: use \"config reset\"] Reset a variable value") + configCmd.SetUsageTemplate(configCmd.UsageTemplate() + supportedOptions) configCmd.AddCommand(NewConfigAddCommand()) configCmd.AddCommand(NewConfigRemoveCommand()) + configCmd.AddCommand(NewConfigListCommand()) + configCmd.AddCommand(NewConfigShowCommand()) + configCmd.AddCommand(NewConfigGetCommand()) + configCmd.AddCommand(NewConfigSetCommand()) + configCmd.AddCommand(NewConfigResetCommand()) + + // Build this from actual subcommands to avoid drift. + reserved := strings.Join(reservedProfileNames(configCmd), ", ") + configCmd.Long = fmt.Sprintf(`Configure zot registry parameters for CLI. + +Use the list, show, get, set, and reset subcommands for inspecting and editing profiles. +Profile names must not collide with subcommand names (%s). + +Older positional syntax on this command is deprecated and will soon be removed.`, reserved) return configCmd } +func zliUserConfigPath() (string, error) { + home, err := os.UserHomeDir() + if err != nil { + return "", err + } + + return filepath.Join(home, ".zot"), nil +} + +// validateProfileNameForCreation prevents creating profiles that shadow subcommand names. +// We intentionally allow interacting with pre-existing profiles that collide with subcommand names +// so users can migrate/rename/remove them without editing ~/.zot by hand. +func validateProfileNameForCreation(configCmd *cobra.Command, name string) error { + if slices.Contains(reservedProfileNames(configCmd), name) { + return fmt.Errorf("%w: %q", zerr.ErrReservedConfigName, name) + } + + return nil +} + +func reservedProfileNames(configCmd *cobra.Command) []string { + seen := make(map[string]struct{}) + + for _, sub := range configCmd.Commands() { + name := sub.Name() + if name == "" { + continue + } + + seen[name] = struct{}{} + } + + reserved := make([]string, 0, len(seen)) + for name := range seen { + reserved = append(reserved, name) + } + + sort.Strings(reserved) + + return reserved +} + func NewConfigAddCommand() *cobra.Command { configAddCmd := &cobra.Command{ Use: "add ", @@ -100,13 +112,20 @@ func NewConfigAddCommand() *cobra.Command { Long: "Add configuration for a zot registry", Args: cobra.ExactArgs(twoArgs), RunE: func(cmd *cobra.Command, args []string) error { - home, err := os.UserHomeDir() + configPath, err := zliUserConfigPath() if err != nil { return err } - configPath := filepath.Join(home, ".zot") - // zli config add + configRoot := cmd.Parent() + if configRoot == nil { + configRoot = cmd + } + + if err := validateProfileNameForCreation(configRoot, args[0]); err != nil { + return err + } + err = addConfig(configPath, args[0], args[1]) if err != nil { return err @@ -130,13 +149,11 @@ func NewConfigRemoveCommand() *cobra.Command { Long: "Remove configuration for a zot registry", Args: cobra.ExactArgs(oneArg), RunE: func(cmd *cobra.Command, args []string) error { - home, err := os.UserHomeDir() + configPath, err := zliUserConfigPath() if err != nil { return err } - configPath := filepath.Join(home, ".zot") - // zli config remove err = removeConfig(configPath, args[0]) if err != nil { return err @@ -152,6 +169,137 @@ func NewConfigRemoveCommand() *cobra.Command { return configRemoveCmd } +func NewConfigListCommand() *cobra.Command { + listCmd := &cobra.Command{ + Use: "list", + Example: " zli config list", + Short: "List all configuration profile names", + Long: "Print every configured CLI profile name (and URLs where applicable).", + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, args []string) error { + configPath, err := zliUserConfigPath() + if err != nil { + return err + } + + res, err := getConfigNames(configPath) + if err != nil { + return err + } + + fmt.Fprint(cmd.OutOrStdout(), res) + + return nil + }, + } + + listCmd.SetUsageTemplate(listCmd.UsageTemplate()) + + return listCmd +} + +func NewConfigShowCommand() *cobra.Command { + showCmd := &cobra.Command{ + Use: "show ", + Example: " zli config show main", + Short: "Show all variables for one profile", + Long: "Print every variable set for the named CLI profile.", + Args: cobra.ExactArgs(oneArg), + RunE: func(cmd *cobra.Command, args []string) error { + configPath, err := zliUserConfigPath() + if err != nil { + return err + } + + res, err := getAllConfig(configPath, args[0]) + if err != nil { + return err + } + + fmt.Fprint(cmd.OutOrStdout(), res) + + return nil + }, + } + + showCmd.SetUsageTemplate(showCmd.UsageTemplate()) + + return showCmd +} + +func NewConfigGetCommand() *cobra.Command { + getCmd := &cobra.Command{ + Use: "get ", + Example: " zli config get main url", + Short: "Print one configuration variable", + Long: "Print the value of a single key for the named profile.", + Args: cobra.ExactArgs(twoArgs), + RunE: func(cmd *cobra.Command, args []string) error { + configPath, err := zliUserConfigPath() + if err != nil { + return err + } + + res, err := getConfigValue(configPath, args[0], args[1]) + if err != nil { + return err + } + + fmt.Fprintln(cmd.OutOrStdout(), res) + + return nil + }, + } + + getCmd.SetUsageTemplate(getCmd.UsageTemplate()) + + return getCmd +} + +func NewConfigSetCommand() *cobra.Command { + setCmd := &cobra.Command{ + Use: "set ", + Example: " zli config set main showspinner false", + Short: "Set a configuration variable", + Long: "Set a single key for the named profile and persist ~/.zot.", + Args: cobra.ExactArgs(threeArgs), + RunE: func(cmd *cobra.Command, args []string) error { + configPath, err := zliUserConfigPath() + if err != nil { + return err + } + + return setConfigValue(configPath, args[0], args[1], args[2]) + }, + } + + setCmd.SetUsageTemplate(setCmd.UsageTemplate()) + + return setCmd +} + +func NewConfigResetCommand() *cobra.Command { + resetCmd := &cobra.Command{ + Use: "reset ", + Example: " zli config reset main showspinner", + Short: "Reset a configuration variable to its default", + Long: "Remove a non-default key from the named profile (URL and profile name cannot be reset).", + Args: cobra.ExactArgs(twoArgs), + RunE: func(cmd *cobra.Command, args []string) error { + configPath, err := zliUserConfigPath() + if err != nil { + return err + } + + return resetConfigValue(configPath, args[0], args[1]) + }, + } + + resetCmd.SetUsageTemplate(resetCmd.UsageTemplate()) + + return resetCmd +} + func getConfigNames(configPath string) (string, error) { cfg, err := ReadZliConfigFile(configPath) if err != nil { @@ -289,9 +437,11 @@ func getAllConfig(configPath, configName string) (string, error) { const ( examples = ` zli config add main https://zot-foo.com:8080 - zli config --list - zli config main url - zli config main --list + zli config list + zli config show main + zli config get main url + zli config set main showspinner false + zli config reset main showspinner zli config remove main` supportedOptions = ` diff --git a/pkg/cli/client/config_cmd_deprecated.go b/pkg/cli/client/config_cmd_deprecated.go new file mode 100644 index 00000000..118b4471 --- /dev/null +++ b/pkg/cli/client/config_cmd_deprecated.go @@ -0,0 +1,89 @@ +//go:build search + +package client + +import ( + "fmt" + "io" + + "github.com/spf13/cobra" + + zerr "zotregistry.dev/zot/v2/errors" +) + +// runLegacyConfig handles deprecated positional syntax and --list/--reset on the parent command. +// Prefer subcommands (list, show, get, set, reset); this file emits deprecation warnings to stderr. +func runLegacyConfig(cmd *cobra.Command, args []string, configPath string, isListing, isReset bool) error { + switch len(args) { + case noArgs: + if isListing { // zli config -l + warnLegacyDeprecatedInvocation(cmd.ErrOrStderr(), "`zli config --list`", "`zli config list`") + + res, err := getConfigNames(configPath) + if err != nil { + return err + } + + fmt.Fprint(cmd.OutOrStdout(), res) + + return nil + } + + return zerr.ErrInvalidArgs + case oneArg: + // zli config -l + if isListing { + warnLegacyDeprecatedInvocation(cmd.ErrOrStderr(), "`zli config --list`", "`zli config show `") + + res, err := getAllConfig(configPath, args[0]) + if err != nil { + return err + } + + fmt.Fprint(cmd.OutOrStdout(), res) + + return nil + } + + return zerr.ErrInvalidArgs + case twoArgs: + if isReset { // zli config --reset + warnLegacyDeprecatedInvocation( + cmd.ErrOrStderr(), + "`zli config --reset`", + "`zli config reset `", + ) + + return resetConfigValue(configPath, args[0], args[1]) + } + + warnLegacyDeprecatedInvocation(cmd.ErrOrStderr(), "`zli config `", "`zli config get `") + + res, err := getConfigValue(configPath, args[0], args[1]) + if err != nil { + return err + } + + fmt.Fprintln(cmd.OutOrStdout(), res) + + case threeArgs: + warnLegacyDeprecatedInvocation( + cmd.ErrOrStderr(), + "`zli config `", + "`zli config set `", + ) + + if err := setConfigValue(configPath, args[0], args[1], args[2]); err != nil { + return err + } + + default: + return zerr.ErrInvalidArgs + } + + return nil +} + +func warnLegacyDeprecatedInvocation(w io.Writer, invoked, replacement string) { + fmt.Fprintf(w, "Warning: deprecated invocation %s; use %s instead.\n", invoked, replacement) +} diff --git a/pkg/cli/client/config_cmd_deprecated_test.go b/pkg/cli/client/config_cmd_deprecated_test.go new file mode 100644 index 00000000..06d43c03 --- /dev/null +++ b/pkg/cli/client/config_cmd_deprecated_test.go @@ -0,0 +1,542 @@ +//go:build search + +// Deprecated parent `config` invocation (--list, positional args, --reset). Prefer tests in config_cmd_test.go. +package client_test + +import ( + "bytes" + "errors" + "os" + "regexp" + "strings" + "testing" + + . "github.com/smartystreets/goconvey/convey" + + zerr "zotregistry.dev/zot/v2/errors" + "zotregistry.dev/zot/v2/pkg/cli/client" +) + +func TestConfigCmdDeprecatedBasics(t *testing.T) { + Convey("Test config help", t, func() { + args := []string{"--help"} + + _ = makeConfigFile(t, "showspinner = false") + + cmd := client.NewConfigCommand() + buff := bytes.NewBufferString("") + cmd.SetOut(buff) + cmd.SetErr(buff) + cmd.SetArgs(args) + err := cmd.Execute() + + So(err, ShouldBeNil) + So(buff.String(), ShouldContainSubstring, "Usage") + + Convey("with the shorthand", func() { + args[0] = "-h" + + _ = makeConfigFile(t, "showspinner = false") + + cmd := client.NewConfigCommand() + buff := bytes.NewBufferString("") + cmd.SetOut(buff) + cmd.SetErr(buff) + cmd.SetArgs(args) + err := cmd.Execute() + + So(buff.String(), ShouldContainSubstring, "Usage") + So(err, ShouldBeNil) + }) + }) + + Convey("Test config no args", t, func() { + args := []string{} + + _ = makeConfigFile(t, "showspinner = false") + + cmd := client.NewConfigCommand() + buff := bytes.NewBufferString("") + cmd.SetOut(buff) + cmd.SetErr(buff) + cmd.SetArgs(args) + err := cmd.Execute() + + So(buff.String(), ShouldContainSubstring, "Usage") + So(err, ShouldNotBeNil) + }) +} + +func TestConfigCmdDeprecatedMain(t *testing.T) { + Convey("Test add config", t, func() { + args := []string{"add", "configtest1", "https://test-url.com"} + + configPath := makeConfigFile(t, "") + + cmd := client.NewConfigCommand() + buff := bytes.NewBufferString("") + cmd.SetOut(buff) + cmd.SetErr(buff) + cmd.SetArgs(args) + _ = cmd.Execute() + + actual, err := os.ReadFile(configPath) + if err != nil { + panic(err) + } + actualStr := string(actual) + So(actualStr, ShouldContainSubstring, "configtest1") + So(actualStr, ShouldContainSubstring, "https://test-url.com") + }) + + Convey("Test error on home directory", t, func() { + args := []string{"add", "configtest1", "https://test-url.com"} + + _ = makeConfigFile(t, "") + + t.Setenv("HOME", "nonExistentDirectory") + + cmd := client.NewConfigCommand() + buff := bytes.NewBufferString("") + cmd.SetOut(buff) + cmd.SetErr(buff) + cmd.SetArgs(args) + err := cmd.Execute() + So(err, ShouldNotBeNil) + }) + + Convey("Test error on home directory at new add config", t, func() { + args := []string{"configtest1", "https://test-url.com"} + + _ = makeConfigFile(t, "") + + t.Setenv("HOME", "nonExistentDirectory") + + cmd := client.NewConfigAddCommand() + buff := bytes.NewBufferString("") + cmd.SetOut(buff) + cmd.SetErr(buff) + cmd.SetArgs(args) + err := cmd.Execute() + So(err, ShouldNotBeNil) + }) + + Convey("Test list config with invalid format", t, func() { + args := []string{"--list"} + + _ = makeConfigFile(t, `{"configs":{"_name":"configtest","url":"https://test-url.com","showspinner":false}}`) + + cmd := client.NewConfigCommand() + buff := bytes.NewBufferString("") + cmd.SetOut(buff) + cmd.SetErr(buff) + cmd.SetArgs(args) + err := cmd.Execute() + So(errors.Is(err, zerr.ErrCliBadConfig), ShouldBeTrue) + }) + + Convey("Test add config with invalid URL", t, func() { + args := []string{"add", "configtest1", "test..com"} + + _ = makeConfigFile(t, "") + + cmd := client.NewConfigCommand() + buff := bytes.NewBufferString("") + cmd.SetOut(buff) + cmd.SetErr(buff) + cmd.SetArgs(args) + err := cmd.Execute() + So(err, ShouldNotBeNil) + So(strings.Contains(err.Error(), zerr.ErrInvalidURL.Error()), ShouldBeTrue) + }) + + Convey("Test remove config entry successfully", t, func() { + args := []string{"remove", "configtest"} + + configPath := makeConfigFile(t, + `{"configs":[{"_name":"configtest","url":"https://test-url.com","showspinner":false}]}`) + + cmd := client.NewConfigCommand() + buff := bytes.NewBufferString("") + cmd.SetOut(buff) + cmd.SetErr(buff) + cmd.SetArgs(args) + err := cmd.Execute() + So(err, ShouldBeNil) + actual, err := os.ReadFile(configPath) + So(err, ShouldBeNil) + space := regexp.MustCompile(`\s+`) + actualString := space.ReplaceAllString(string(actual), " ") + So(actualString, ShouldEqual, `{ "configs": [] }`) + }) + + Convey("Test remove missing config entry", t, func() { + args := []string{"remove", "configtest"} + + _ = makeConfigFile(t, `{"configs":[]}`) + + cmd := client.NewConfigCommand() + buff := bytes.NewBufferString("") + cmd.SetOut(buff) + cmd.SetErr(buff) + cmd.SetArgs(args) + err := cmd.Execute() + So(err, ShouldNotBeNil) + So(buff.String(), ShouldContainSubstring, "does not exist") + }) + + Convey("Test remove bad config file content", t, func() { + args := []string{"remove", "configtest"} + + _ = makeConfigFile(t, `{"asdf":[]`) + + cmd := client.NewConfigCommand() + buff := bytes.NewBufferString("") + cmd.SetOut(buff) + cmd.SetErr(buff) + cmd.SetArgs(args) + err := cmd.Execute() + So(err, ShouldNotBeNil) + So(errors.Is(err, zerr.ErrCliBadConfig), ShouldBeTrue) + }) + + Convey("Test remove bad config file entry", t, func() { + args := []string{"remove", "configtest"} + + _ = makeConfigFile(t, `{"configs":[asdad]`) + + cmd := client.NewConfigCommand() + buff := bytes.NewBufferString("") + cmd.SetOut(buff) + cmd.SetErr(buff) + cmd.SetArgs(args) + err := cmd.Execute() + So(err, ShouldNotBeNil) + So(buff.String(), ShouldContainSubstring, zerr.ErrCliBadConfig.Error()) + }) + + Convey("Test remove config bad permissions", t, func() { + args := []string{"remove", "configtest"} + configPath := makeConfigFile(t, + `{"configs":[{"_name":"configtest","url":"https://test-url.com","showspinner":false}]}`) + + defer func() { + _ = os.Chmod(configPath, 0o600) + }() + + err := os.Chmod(configPath, 0o400) // Read-only, so we fail only on updating the file, not reading + So(err, ShouldBeNil) + + cmd := client.NewConfigCommand() + buff := bytes.NewBufferString("") + cmd.SetOut(buff) + cmd.SetErr(buff) + cmd.SetArgs(args) + err = cmd.Execute() + So(err, ShouldNotBeNil) + So(buff.String(), ShouldContainSubstring, "permission denied") + }) + + Convey("Test fetch all config", t, func() { + args := []string{"--list"} + + _ = makeConfigFile(t, `{"configs":[{"_name":"configtest","url":"https://test-url.com","showspinner":false}]}`) + + cmd := client.NewConfigCommand() + outBuff := bytes.NewBufferString("") + errBuff := bytes.NewBufferString("") + cmd.SetOut(outBuff) + cmd.SetErr(errBuff) + cmd.SetArgs(args) + err := cmd.Execute() + + So(outBuff.String(), ShouldContainSubstring, "https://test-url.com") + So(errBuff.String(), ShouldContainSubstring, "`zli config list`") + So(err, ShouldBeNil) + + Convey("with the shorthand", func() { + args := []string{"-l"} + + _ = makeConfigFile(t, `{"configs":[{"_name":"configtest","url":"https://test-url.com","showspinner":false}]}`) + + cmd := client.NewConfigCommand() + outBuff := bytes.NewBufferString("") + errBuff := bytes.NewBufferString("") + cmd.SetOut(outBuff) + cmd.SetErr(errBuff) + cmd.SetArgs(args) + + err := cmd.Execute() + So(err, ShouldBeNil) + + So(outBuff.String(), ShouldContainSubstring, "https://test-url.com") + So(errBuff.String(), ShouldContainSubstring, "`zli config list`") + }) + + Convey("From empty file", func() { + args := []string{"-l"} + + _ = makeConfigFile(t, ``) + + cmd := client.NewConfigCommand() + outBuff := bytes.NewBufferString("") + errBuff := bytes.NewBufferString("") + cmd.SetOut(outBuff) + cmd.SetErr(errBuff) + cmd.SetArgs(args) + + err := cmd.Execute() + So(err, ShouldBeNil) + + So(strings.TrimSpace(outBuff.String()), ShouldEqual, "") + So(errBuff.String(), ShouldContainSubstring, "`zli config list`") + }) + }) + + Convey("Test fetch a config", t, func() { + args := []string{"configtest", "--list"} + + _ = makeConfigFile(t, `{"configs":[{"_name":"configtest","url":"https://test-url.com","showspinner":false}]}`) + + cmd := client.NewConfigCommand() + outBuff := bytes.NewBufferString("") + errBuff := bytes.NewBufferString("") + cmd.SetOut(outBuff) + cmd.SetErr(errBuff) + cmd.SetArgs(args) + + err := cmd.Execute() + So(err, ShouldBeNil) + + So(outBuff.String(), ShouldContainSubstring, "url = https://test-url.com") + So(outBuff.String(), ShouldContainSubstring, "showspinner = false") + So(errBuff.String(), ShouldContainSubstring, "`zli config show `") + + Convey("with the shorthand", func() { + args := []string{"configtest", "-l"} + + _ = makeConfigFile(t, `{"configs":[{"_name":"configtest","url":"https://test-url.com","showspinner":false}]}`) + + cmd := client.NewConfigCommand() + outBuff := bytes.NewBufferString("") + errBuff := bytes.NewBufferString("") + cmd.SetOut(outBuff) + cmd.SetErr(errBuff) + cmd.SetArgs(args) + + err := cmd.Execute() + So(err, ShouldBeNil) + + So(outBuff.String(), ShouldContainSubstring, "url = https://test-url.com") + So(outBuff.String(), ShouldContainSubstring, "showspinner = false") + So(errBuff.String(), ShouldContainSubstring, "`zli config show `") + }) + + Convey("From empty file", func() { + args := []string{"configtest", "-l"} + + _ = makeConfigFile(t, ``) + + cmd := client.NewConfigCommand() + outBuff := bytes.NewBufferString("") + errBuff := bytes.NewBufferString("") + cmd.SetOut(outBuff) + cmd.SetErr(errBuff) + cmd.SetArgs(args) + + err := cmd.Execute() + So(err, ShouldBeNil) + + So(strings.TrimSpace(outBuff.String()), ShouldEqual, "") + So(errBuff.String(), ShouldContainSubstring, "`zli config show `") + }) + }) + + Convey("Test fetch a config val", t, func() { + args := []string{"configtest", "url"} + + _ = makeConfigFile(t, `{"configs":[{"_name":"configtest","url":"https://test-url.com","showspinner":false}]}`) + + cmd := client.NewConfigCommand() + outBuff := bytes.NewBufferString("") + errBuff := bytes.NewBufferString("") + cmd.SetOut(outBuff) + cmd.SetErr(errBuff) + cmd.SetArgs(args) + + err := cmd.Execute() + So(err, ShouldBeNil) + So(outBuff.String(), ShouldEqual, "https://test-url.com\n") + So(errBuff.String(), ShouldContainSubstring, "deprecated invocation") + + Convey("From empty file", func() { + args := []string{"configtest", "url"} + + _ = makeConfigFile(t, ``) + + cmd := client.NewConfigCommand() + outBuff := bytes.NewBufferString("") + errBuff := bytes.NewBufferString("") + cmd.SetOut(outBuff) + cmd.SetErr(errBuff) + cmd.SetArgs(args) + + err := cmd.Execute() + So(err, ShouldNotBeNil) + + combined := errBuff.String() + outBuff.String() + So(combined, ShouldContainSubstring, "does not exist") + }) + }) + + Convey("Test add a config val", t, func() { + args := []string{"configtest", "showspinner", "false"} + + configPath := makeConfigFile(t, `{"configs":[{"_name":"configtest","url":"https://test-url.com"}]}`) + + cmd := client.NewConfigCommand() + outBuff := bytes.NewBufferString("") + errBuff := bytes.NewBufferString("") + cmd.SetOut(outBuff) + cmd.SetErr(errBuff) + cmd.SetArgs(args) + err := cmd.Execute() + So(err, ShouldBeNil) + + actual, err := os.ReadFile(configPath) + if err != nil { + panic(err) + } + actualStr := string(actual) + So(actualStr, ShouldContainSubstring, "https://test-url.com") + So(actualStr, ShouldContainSubstring, `"showspinner": false`) + So(outBuff.String(), ShouldEqual, "") + So(errBuff.String(), ShouldContainSubstring, "deprecated invocation") + + Convey("To an empty file", func() { + args := []string{"configtest", "showspinner", "false"} + + _ = makeConfigFile(t, ``) + + cmd := client.NewConfigCommand() + outBuff := bytes.NewBufferString("") + errBuff := bytes.NewBufferString("") + cmd.SetOut(outBuff) + cmd.SetErr(errBuff) + cmd.SetArgs(args) + + err := cmd.Execute() + So(err, ShouldNotBeNil) + + combined := errBuff.String() + outBuff.String() + So(combined, ShouldContainSubstring, "does not exist") + }) + }) + + Convey("Test overwrite a config", t, func() { + args := []string{"configtest", "url", "https://new-url.com"} + + configPath := makeConfigFile(t, + `{"configs":[{"_name":"configtest","url":"https://test-url.com","showspinner":false}]}`) + + cmd := client.NewConfigCommand() + outBuff := bytes.NewBufferString("") + errBuff := bytes.NewBufferString("") + cmd.SetOut(outBuff) + cmd.SetErr(errBuff) + cmd.SetArgs(args) + err := cmd.Execute() + So(err, ShouldBeNil) + + actual, err := os.ReadFile(configPath) + if err != nil { + panic(err) + } + actualStr := string(actual) + So(actualStr, ShouldContainSubstring, `https://new-url.com`) + So(actualStr, ShouldContainSubstring, `"showspinner": false`) + So(actualStr, ShouldNotContainSubstring, `https://test-url.com`) + So(outBuff.String(), ShouldEqual, "") + So(errBuff.String(), ShouldContainSubstring, "deprecated invocation") + }) + + Convey("Test reset a config val", t, func() { + args := []string{"configtest", "showspinner", "--reset"} + + configPath := makeConfigFile(t, + `{"configs":[{"_name":"configtest","url":"https://test-url.com","showspinner":false}]}`) + + cmd := client.NewConfigCommand() + outBuff := bytes.NewBufferString("") + errBuff := bytes.NewBufferString("") + cmd.SetOut(outBuff) + cmd.SetErr(errBuff) + cmd.SetArgs(args) + err := cmd.Execute() + So(err, ShouldBeNil) + + actual, err := os.ReadFile(configPath) + if err != nil { + panic(err) + } + actualStr := string(actual) + So(actualStr, ShouldNotContainSubstring, "showspinner") + So(actualStr, ShouldContainSubstring, `"url": "https://test-url.com"`) + So(outBuff.String(), ShouldEqual, "") + So(errBuff.String(), ShouldContainSubstring, "`zli config reset `") + }) + + Convey("Test reset a url", t, func() { + args := []string{"configtest", "url", "--reset"} + + _ = makeConfigFile(t, `{"configs":[{"_name":"configtest","url":"https://test-url.com","showspinner":false}]}`) + + cmd := client.NewConfigCommand() + outBuff := bytes.NewBufferString("") + errBuff := bytes.NewBufferString("") + cmd.SetOut(outBuff) + cmd.SetErr(errBuff) + cmd.SetArgs(args) + + err := cmd.Execute() + So(err, ShouldNotBeNil) + + combined := errBuff.String() + outBuff.String() + So(combined, ShouldContainSubstring, "cannot reset") + So(combined, ShouldContainSubstring, "`zli config reset `") + }) + + Convey("Test add a config with an existing saved name", t, func() { + args := []string{"add", "configtest", "https://test-url.com/new"} + + _ = makeConfigFile(t, `{"configs":[{"_name":"configtest","url":"https://test-url.com","showspinner":false}]}`) + + cmd := client.NewConfigCommand() + buff := bytes.NewBufferString("") + cmd.SetOut(buff) + cmd.SetErr(buff) + cmd.SetArgs(args) + + err := cmd.Execute() + So(err, ShouldNotBeNil) + + So(buff.String(), ShouldContainSubstring, "cli config name already added") + }) + + Convey("Test deprecated config invalid args (too many args)", t, func() { + args := []string{"configtest", "url", "x", "y"} + + _ = makeConfigFile(t, `{"configs":[{"_name":"configtest","url":"https://test-url.com","showspinner":false}]}`) + + cmd := client.NewConfigCommand() + outBuff := bytes.NewBufferString("") + errBuff := bytes.NewBufferString("") + cmd.SetOut(outBuff) + cmd.SetErr(errBuff) + cmd.SetArgs(args) + + err := cmd.Execute() + So(err, ShouldNotBeNil) + So(errors.Is(err, zerr.ErrInvalidArgs), ShouldBeTrue) + }) +} diff --git a/pkg/cli/client/config_cmd_test.go b/pkg/cli/client/config_cmd_test.go index ae84dd8b..27cca1df 100644 --- a/pkg/cli/client/config_cmd_test.go +++ b/pkg/cli/client/config_cmd_test.go @@ -88,6 +88,63 @@ func TestConfigCmdMain(t *testing.T) { So(actualStr, ShouldContainSubstring, "https://test-url.com") }) + Convey("Test add config rejects reserved names", t, func() { + args := []string{"add", "list", "https://test-url.com"} + + _ = makeConfigFile(t, "") + + cmd := client.NewConfigCommand() + buff := bytes.NewBufferString("") + cmd.SetOut(buff) + cmd.SetErr(buff) + cmd.SetArgs(args) + err := cmd.Execute() + + So(err, ShouldNotBeNil) + So(errors.Is(err, zerr.ErrReservedConfigName), ShouldBeTrue) + So(err.Error(), ShouldContainSubstring, `"list"`) + }) + + Convey("Test reserved-named profiles are still accessible for migration", t, func() { + _ = makeConfigFile(t, + `{"configs":[{"_name":"list","url":"https://test-url.com","showspinner":false}]}`) + + Convey("show", func() { + cmd := client.NewConfigCommand() + out := bytes.NewBufferString("") + errOut := bytes.NewBufferString("") + cmd.SetOut(out) + cmd.SetErr(errOut) + cmd.SetArgs([]string{"show", "list"}) + err := cmd.Execute() + So(err, ShouldBeNil) + So(out.String(), ShouldContainSubstring, "https://test-url.com") + }) + + Convey("get", func() { + cmd := client.NewConfigCommand() + out := bytes.NewBufferString("") + errOut := bytes.NewBufferString("") + cmd.SetOut(out) + cmd.SetErr(errOut) + cmd.SetArgs([]string{"get", "list", "url"}) + err := cmd.Execute() + So(err, ShouldBeNil) + So(out.String(), ShouldContainSubstring, "https://test-url.com") + }) + + Convey("remove", func() { + cmd := client.NewConfigCommand() + out := bytes.NewBufferString("") + errOut := bytes.NewBufferString("") + cmd.SetOut(out) + cmd.SetErr(errOut) + cmd.SetArgs([]string{"remove", "list"}) + err := cmd.Execute() + So(err, ShouldBeNil) + }) + }) + Convey("Test error on home directory", t, func() { args := []string{"add", "configtest1", "https://test-url.com"} @@ -105,7 +162,7 @@ func TestConfigCmdMain(t *testing.T) { }) Convey("Test error on home directory at new add config", t, func() { - args := []string{"add", "configtest1", "https://test-url.com"} + args := []string{"configtest1", "https://test-url.com"} _ = makeConfigFile(t, "") @@ -120,8 +177,8 @@ func TestConfigCmdMain(t *testing.T) { So(err, ShouldNotBeNil) }) - Convey("Test add config with invalid format", t, func() { - args := []string{"--list"} + Convey("Test list config with invalid format", t, func() { + args := []string{"list"} _ = makeConfigFile(t, `{"configs":{"_name":"configtest","url":"https://test-url.com","showspinner":false}}`) @@ -236,246 +293,194 @@ func TestConfigCmdMain(t *testing.T) { So(buff.String(), ShouldContainSubstring, "permission denied") }) - Convey("Test fetch all config", t, func() { - args := []string{"--list"} - - _ = makeConfigFile(t, `{"configs":[{"_name":"configtest","url":"https://test-url.com","showspinner":false}]}`) - - cmd := client.NewConfigCommand() - buff := bytes.NewBufferString("") - cmd.SetOut(buff) - cmd.SetErr(buff) - cmd.SetArgs(args) - err := cmd.Execute() - - So(buff.String(), ShouldContainSubstring, "https://test-url.com") - So(err, ShouldBeNil) - - Convey("with the shorthand", func() { - args := []string{"-l"} - + Convey("Test config list", t, func() { + Convey("prints profile names and URLs", func() { _ = makeConfigFile(t, `{"configs":[{"_name":"configtest","url":"https://test-url.com","showspinner":false}]}`) cmd := client.NewConfigCommand() - buff := bytes.NewBufferString("") - cmd.SetOut(buff) - cmd.SetErr(buff) - cmd.SetArgs(args) - - err := cmd.Execute() - So(err, ShouldBeNil) - - So(buff.String(), ShouldContainSubstring, "https://test-url.com") + outBuff := bytes.NewBufferString("") + errBuff := bytes.NewBufferString("") + cmd.SetOut(outBuff) + cmd.SetErr(errBuff) + cmd.SetArgs([]string{"list"}) + So(cmd.Execute(), ShouldBeNil) + So(outBuff.String(), ShouldContainSubstring, "https://test-url.com") + So(errBuff.String(), ShouldEqual, "") }) - Convey("From empty file", func() { - args := []string{"-l"} - + Convey("from empty config file", func() { _ = makeConfigFile(t, ``) cmd := client.NewConfigCommand() - buff := bytes.NewBufferString("") - cmd.SetOut(buff) - cmd.SetErr(buff) - cmd.SetArgs(args) - - err := cmd.Execute() - So(err, ShouldBeNil) - - So(strings.TrimSpace(buff.String()), ShouldEqual, "") + outBuff := bytes.NewBufferString("") + errBuff := bytes.NewBufferString("") + cmd.SetOut(outBuff) + cmd.SetErr(errBuff) + cmd.SetArgs([]string{"list"}) + So(cmd.Execute(), ShouldBeNil) + So(strings.TrimSpace(outBuff.String()), ShouldEqual, "") + So(errBuff.String(), ShouldEqual, "") }) }) - Convey("Test fetch a config", t, func() { - args := []string{"configtest", "--list"} - - _ = makeConfigFile(t, `{"configs":[{"_name":"configtest","url":"https://test-url.com","showspinner":false}]}`) - - cmd := client.NewConfigCommand() - buff := bytes.NewBufferString("") - cmd.SetOut(buff) - cmd.SetErr(buff) - cmd.SetArgs(args) - - err := cmd.Execute() - So(err, ShouldBeNil) - - So(buff.String(), ShouldContainSubstring, "url = https://test-url.com") - So(buff.String(), ShouldContainSubstring, "showspinner = false") - - Convey("with the shorthand", func() { - args := []string{"configtest", "-l"} - + Convey("Test config show", t, func() { + Convey("prints variables for the profile", func() { _ = makeConfigFile(t, `{"configs":[{"_name":"configtest","url":"https://test-url.com","showspinner":false}]}`) cmd := client.NewConfigCommand() - buff := bytes.NewBufferString("") - cmd.SetOut(buff) - cmd.SetErr(buff) - cmd.SetArgs(args) + outBuff := bytes.NewBufferString("") + errBuff := bytes.NewBufferString("") + cmd.SetOut(outBuff) + cmd.SetErr(errBuff) + cmd.SetArgs([]string{"show", "configtest"}) + So(cmd.Execute(), ShouldBeNil) + So(outBuff.String(), ShouldContainSubstring, "url = https://test-url.com") + So(outBuff.String(), ShouldContainSubstring, "showspinner = false") + So(errBuff.String(), ShouldEqual, "") + }) - err := cmd.Execute() + Convey("from empty config file", func() { + _ = makeConfigFile(t, ``) + + cmd := client.NewConfigCommand() + outBuff := bytes.NewBufferString("") + errBuff := bytes.NewBufferString("") + cmd.SetOut(outBuff) + cmd.SetErr(errBuff) + cmd.SetArgs([]string{"show", "configtest"}) + So(cmd.Execute(), ShouldBeNil) + So(strings.TrimSpace(outBuff.String()), ShouldEqual, "") + So(errBuff.String(), ShouldEqual, "") + }) + }) + + Convey("Test config get", t, func() { + Convey("prints one key", func() { + _ = makeConfigFile(t, `{"configs":[{"_name":"configtest","url":"https://test-url.com","showspinner":false}]}`) + + cmd := client.NewConfigCommand() + outBuff := bytes.NewBufferString("") + errBuff := bytes.NewBufferString("") + cmd.SetOut(outBuff) + cmd.SetErr(errBuff) + cmd.SetArgs([]string{"get", "configtest", "url"}) + So(cmd.Execute(), ShouldBeNil) + So(outBuff.String(), ShouldEqual, "https://test-url.com\n") + So(errBuff.String(), ShouldEqual, "") + }) + + Convey("from empty config file", func() { + _ = makeConfigFile(t, ``) + + cmd := client.NewConfigCommand() + outBuff := bytes.NewBufferString("") + errBuff := bytes.NewBufferString("") + cmd.SetOut(outBuff) + cmd.SetErr(errBuff) + cmd.SetArgs([]string{"get", "configtest", "url"}) + So(cmd.Execute(), ShouldNotBeNil) + + combined := errBuff.String() + outBuff.String() + So(combined, ShouldContainSubstring, "does not exist") + }) + }) + + Convey("Test config set", t, func() { + Convey("adds a variable", func() { + configPath := makeConfigFile(t, `{"configs":[{"_name":"configtest","url":"https://test-url.com"}]}`) + + cmd := client.NewConfigCommand() + outBuff := bytes.NewBufferString("") + errBuff := bytes.NewBufferString("") + cmd.SetOut(outBuff) + cmd.SetErr(errBuff) + cmd.SetArgs([]string{"set", "configtest", "showspinner", "false"}) + So(cmd.Execute(), ShouldBeNil) + So(outBuff.String(), ShouldEqual, "") + So(errBuff.String(), ShouldEqual, "") + + actual, err := os.ReadFile(configPath) So(err, ShouldBeNil) - - So(buff.String(), ShouldContainSubstring, "url = https://test-url.com") - So(buff.String(), ShouldContainSubstring, "showspinner = false") + actualStr := string(actual) + So(actualStr, ShouldContainSubstring, "https://test-url.com") + So(actualStr, ShouldContainSubstring, `"showspinner": false`) }) - Convey("From empty file", func() { - args := []string{"configtest", "-l"} - + Convey("to an empty config file", func() { _ = makeConfigFile(t, ``) cmd := client.NewConfigCommand() - buff := bytes.NewBufferString("") - cmd.SetOut(buff) - cmd.SetErr(buff) - cmd.SetArgs(args) + outBuff := bytes.NewBufferString("") + errBuff := bytes.NewBufferString("") + cmd.SetOut(outBuff) + cmd.SetErr(errBuff) + cmd.SetArgs([]string{"set", "configtest", "showspinner", "false"}) + So(cmd.Execute(), ShouldNotBeNil) - err := cmd.Execute() - So(err, ShouldBeNil) - - So(strings.TrimSpace(buff.String()), ShouldEqual, "") + combined := errBuff.String() + outBuff.String() + So(combined, ShouldContainSubstring, "does not exist") }) }) - Convey("Test fetch a config val", t, func() { - args := []string{"configtest", "url"} - - _ = makeConfigFile(t, `{"configs":[{"_name":"configtest","url":"https://test-url.com","showspinner":false}]}`) - - cmd := client.NewConfigCommand() - buff := bytes.NewBufferString("") - cmd.SetOut(buff) - cmd.SetErr(buff) - cmd.SetArgs(args) - - err := cmd.Execute() - So(err, ShouldBeNil) - So(buff.String(), ShouldEqual, "https://test-url.com\n") - - Convey("From empty file", func() { - args := []string{"configtest", "url"} - - _ = makeConfigFile(t, ``) - - cmd := client.NewConfigCommand() - buff := bytes.NewBufferString("") - cmd.SetOut(buff) - cmd.SetErr(buff) - cmd.SetArgs(args) - - err := cmd.Execute() - So(err, ShouldNotBeNil) - - So(buff.String(), ShouldContainSubstring, "does not exist") - }) - }) - - Convey("Test add a config val", t, func() { - args := []string{"configtest", "showspinner", "false"} - - configPath := makeConfigFile(t, `{"configs":[{"_name":"configtest","url":"https://test-url.com"}]}`) - - cmd := client.NewConfigCommand() - buff := bytes.NewBufferString("") - cmd.SetOut(buff) - cmd.SetErr(buff) - cmd.SetArgs(args) - err := cmd.Execute() - So(err, ShouldBeNil) - - actual, err := os.ReadFile(configPath) - if err != nil { - panic(err) - } - actualStr := string(actual) - So(actualStr, ShouldContainSubstring, "https://test-url.com") - So(actualStr, ShouldContainSubstring, `"showspinner": false`) - So(buff.String(), ShouldEqual, "") - - Convey("To an empty file", func() { - args := []string{"configtest", "showspinner", "false"} - - _ = makeConfigFile(t, ``) - - cmd := client.NewConfigCommand() - buff := bytes.NewBufferString("") - cmd.SetOut(buff) - cmd.SetErr(buff) - cmd.SetArgs(args) - - err := cmd.Execute() - So(err, ShouldNotBeNil) - - So(buff.String(), ShouldContainSubstring, "does not exist") - }) - }) - - Convey("Test overwrite a config", t, func() { - args := []string{"configtest", "url", "https://new-url.com"} - + Convey("Test config set overwrites URL", t, func() { configPath := makeConfigFile(t, `{"configs":[{"_name":"configtest","url":"https://test-url.com","showspinner":false}]}`) cmd := client.NewConfigCommand() - buff := bytes.NewBufferString("") - cmd.SetOut(buff) - cmd.SetErr(buff) - cmd.SetArgs(args) - err := cmd.Execute() - So(err, ShouldBeNil) + outBuff := bytes.NewBufferString("") + errBuff := bytes.NewBufferString("") + cmd.SetOut(outBuff) + cmd.SetErr(errBuff) + cmd.SetArgs([]string{"set", "configtest", "url", "https://new-url.com"}) + So(cmd.Execute(), ShouldBeNil) + So(outBuff.String(), ShouldEqual, "") + So(errBuff.String(), ShouldEqual, "") actual, err := os.ReadFile(configPath) - if err != nil { - panic(err) - } + So(err, ShouldBeNil) actualStr := string(actual) So(actualStr, ShouldContainSubstring, `https://new-url.com`) So(actualStr, ShouldContainSubstring, `"showspinner": false`) So(actualStr, ShouldNotContainSubstring, `https://test-url.com`) - So(buff.String(), ShouldEqual, "") }) - Convey("Test reset a config val", t, func() { - args := []string{"configtest", "showspinner", "--reset"} + Convey("Test config reset", t, func() { + Convey("clears an optional variable", func() { + configPath := makeConfigFile(t, + `{"configs":[{"_name":"configtest","url":"https://test-url.com","showspinner":false}]}`) - configPath := makeConfigFile(t, - `{"configs":[{"_name":"configtest","url":"https://test-url.com","showspinner":false}]}`) + cmd := client.NewConfigCommand() + outBuff := bytes.NewBufferString("") + errBuff := bytes.NewBufferString("") + cmd.SetOut(outBuff) + cmd.SetErr(errBuff) + cmd.SetArgs([]string{"reset", "configtest", "showspinner"}) + So(cmd.Execute(), ShouldBeNil) + So(outBuff.String(), ShouldEqual, "") + So(errBuff.String(), ShouldEqual, "") - cmd := client.NewConfigCommand() - buff := bytes.NewBufferString("") - cmd.SetOut(buff) - cmd.SetErr(buff) - cmd.SetArgs(args) - err := cmd.Execute() - So(err, ShouldBeNil) + actual, err := os.ReadFile(configPath) + So(err, ShouldBeNil) + actualStr := string(actual) + So(actualStr, ShouldNotContainSubstring, "showspinner") + So(actualStr, ShouldContainSubstring, `"url": "https://test-url.com"`) + }) - actual, err := os.ReadFile(configPath) - if err != nil { - panic(err) - } - actualStr := string(actual) - So(actualStr, ShouldNotContainSubstring, "showspinner") - So(actualStr, ShouldContainSubstring, `"url": "https://test-url.com"`) - So(buff.String(), ShouldEqual, "") - }) + Convey("rejects resetting url", func() { + _ = makeConfigFile(t, `{"configs":[{"_name":"configtest","url":"https://test-url.com","showspinner":false}]}`) - Convey("Test reset a url", t, func() { - args := []string{"configtest", "url", "--reset"} + cmd := client.NewConfigCommand() + outBuff := bytes.NewBufferString("") + errBuff := bytes.NewBufferString("") + cmd.SetOut(outBuff) + cmd.SetErr(errBuff) + cmd.SetArgs([]string{"reset", "configtest", "url"}) - _ = makeConfigFile(t, `{"configs":[{"_name":"configtest","url":"https://test-url.com","showspinner":false}]}`) + So(cmd.Execute(), ShouldNotBeNil) - cmd := client.NewConfigCommand() - buff := bytes.NewBufferString("") - cmd.SetOut(buff) - cmd.SetErr(buff) - cmd.SetArgs(args) - - err := cmd.Execute() - So(err, ShouldNotBeNil) - - So(buff.String(), ShouldContainSubstring, "cannot reset") + combined := errBuff.String() + outBuff.String() + So(combined, ShouldContainSubstring, "cannot reset") + }) }) Convey("Test add a config with an existing saved name", t, func() { diff --git a/pkg/cli/server/verify_retention_test.go b/pkg/cli/server/verify_retention_test.go index 54e1c5d3..16d76729 100644 --- a/pkg/cli/server/verify_retention_test.go +++ b/pkg/cli/server/verify_retention_test.go @@ -1445,6 +1445,10 @@ func TestRetentionCheckWithSubpaths(t *testing.T) { }, } + // The command returns after its timeout; ensure we have all expected decisions in the log + // before validating, to avoid flakes from buffered writes. + logContent = readLogUntilRetentionDecisions(t, logFile, len(expectedResults), 2*time.Second) + logStr = string(logContent) validateRetentionDecisions(t, logContent, expectedResults) }) } @@ -1714,6 +1718,56 @@ func validateRetentionDecisions(t *testing.T, logContent []byte, expectedResults } } +func readLogUntilRetentionDecisions( + t *testing.T, + logFile string, + expectedCount int, + timeout time.Duration, +) []byte { + t.Helper() + + deadline := time.Now().Add(timeout) + + var ( + lastContent []byte + lastReadErr error + ) + + for time.Now().Before(deadline) { + content, err := os.ReadFile(logFile) + if err != nil { + lastReadErr = err + time.Sleep(50 * time.Millisecond) + + continue + } + + lastReadErr = nil + lastContent = content + if len(parseRetentionDecisions(content)) >= expectedCount { + return content + } + + time.Sleep(50 * time.Millisecond) + } + + if lastContent == nil && lastReadErr != nil { + t.Fatalf("failed to read log file %q: %v", logFile, lastReadErr) + } + + if lastReadErr != nil { + t.Logf("last log read error for %q: %v", logFile, lastReadErr) + } + + observed := len(parseRetentionDecisions(lastContent)) + if observed < expectedCount { + t.Fatalf("timed out waiting for retention decisions in %q: observed=%d expected>=%d", + logFile, observed, expectedCount) + } + + return lastContent +} + func logRetentionDecisions(t *testing.T, actualDecisions []RetentionDecision) { t.Helper()