From 41a60ca915132ebaaac6bdaa861c60346740a4f3 Mon Sep 17 00:00:00 2001 From: Alexander Nesterenko Date: Tue, 22 Jan 2019 12:03:33 +0200 Subject: [PATCH] Improve implementation Make test be better designed to use interface implementation instead of leaking variables with closures. The removed warning was concidered as irrelevant, so it won't be needed in fact, because legacy version is not goint to become deprecated Signed-off-by: Alexander Nesterenko --- cmd/helm/root.go | 10 ++-- cmd/helm/root_test.go | 4 +- pkg/helm/environment/environment.go | 6 +-- pkg/helm/environment/environment_test.go | 5 +- .../helmpath/default_helm_home_dir_test.go | 47 ++++++++++++------- pkg/helm/helmpath/default_home_dir.go | 40 ++++++++-------- 6 files changed, 59 insertions(+), 53 deletions(-) diff --git a/cmd/helm/root.go b/cmd/helm/root.go index fb7644fb0..48c9e3c7b 100644 --- a/cmd/helm/root.go +++ b/cmd/helm/root.go @@ -45,11 +45,10 @@ Common actions from this point include: - helm list: list releases of charts Environment: - $HELM_HOME set an alternative location for Helm files. By default, these are stored in - "$XDG_CONFIG_DIR/helm" (defaults to ~/.config/helm) on Linux, - "%APPDATA%\helm" on Windows and "$HOME/Library/Preferences" on OSX. - NOTE: if you have old-style "~/.helm" directory, it will be used, but consider - moving it to a new home. + $HELM_HOME set an alternative location for Helm files. By default, these are stored in + "$XDG_CONFIG_DIR/helm" (typically ~/.config/helm) on Linux, + "%APPDATA%\helm" on Windows and "$HOME/Library/Preferences" on OSX. + NOTE: if you have old-style "~/.helm" directory, it will be used. $HELM_DRIVER set the backend storage driver. Values are: configmap, secret, memory $HELM_NO_PLUGINS disable plugins. Set HELM_NO_PLUGINS=1 to disable plugins. $KUBECONFIG set an alternative Kubernetes configuration file (default "~/.kube/config") @@ -67,7 +66,6 @@ func newRootCmd(c helm.Interface, actionConfig *action.Configuration, out io.Wri flags := cmd.PersistentFlags() settings.AddFlags(flags) - settings.AddHomeFlag(flags, helmpath.GetDefaultConfigHome(out)) flags.Parse(args) diff --git a/cmd/helm/root_test.go b/cmd/helm/root_test.go index 924ec8a7a..7885e41b4 100644 --- a/cmd/helm/root_test.go +++ b/cmd/helm/root_test.go @@ -17,8 +17,8 @@ limitations under the License. package main import ( + "k8s.io/helm/pkg/helm/helmpath" "os" - "path/filepath" "testing" "k8s.io/client-go/util/homedir" @@ -34,7 +34,7 @@ func TestRootCmd(t *testing.T) { { name: "defaults", args: "home", - home: filepath.Join(homedir.HomeDir(), ".config/helm"), + home: helmpath.GetDefaultConfigHome(), }, { name: "with --home set", diff --git a/pkg/helm/environment/environment.go b/pkg/helm/environment/environment.go index f4f3f1c7c..b90e3b7bc 100644 --- a/pkg/helm/environment/environment.go +++ b/pkg/helm/environment/environment.go @@ -48,11 +48,7 @@ func (s *EnvSettings) AddFlags(fs *pflag.FlagSet) { fs.StringVar(&s.KubeConfig, "kubeconfig", "", "path to the kubeconfig file") fs.StringVar(&s.KubeContext, "kube-context", "", "name of the kubeconfig context to use") fs.BoolVar(&s.Debug, "debug", false, "enable verbose output") -} - -// Binds home flag. -func (s *EnvSettings) AddHomeFlag(fs *pflag.FlagSet, defaultHomePath string) { - fs.StringVar((*string)(&s.Home), "home", defaultHomePath, "location of your Helm config. Overrides $HELM_HOME") + fs.StringVar((*string)(&s.Home), "home", helmpath.GetDefaultConfigHome(), "location of your Helm config. Overrides $HELM_HOME") } // Init sets values from the environment. diff --git a/pkg/helm/environment/environment_test.go b/pkg/helm/environment/environment_test.go index 3cba11827..3af9d8c29 100644 --- a/pkg/helm/environment/environment_test.go +++ b/pkg/helm/environment/environment_test.go @@ -40,8 +40,8 @@ func TestEnvSettings(t *testing.T) { }{ { name: "defaults", - home: "~/.helm", - plugins: helmpath.Home("~/.helm").Plugins(), + home: helmpath.GetDefaultConfigHome(), + plugins: helmpath.Home(helmpath.GetDefaultConfigHome()).Plugins(), ns: "", }, { @@ -83,7 +83,6 @@ func TestEnvSettings(t *testing.T) { settings := &EnvSettings{} settings.AddFlags(flags) - settings.AddHomeFlag(flags, "~/.helm") flags.Parse(strings.Split(tt.args, " ")) settings.Init(flags) diff --git a/pkg/helm/helmpath/default_helm_home_dir_test.go b/pkg/helm/helmpath/default_helm_home_dir_test.go index 2b2c30710..c593f54d1 100644 --- a/pkg/helm/helmpath/default_helm_home_dir_test.go +++ b/pkg/helm/helmpath/default_helm_home_dir_test.go @@ -14,7 +14,6 @@ package helmpath import ( - "os" "runtime" "testing" ) @@ -26,28 +25,40 @@ func StringEquals(t *testing.T, a, b string) { } } -func returns(what bool) func() bool { return func() bool { return what } } +type WithNewHome struct{ DefaultConfigHomePath } + +func (WithNewHome) xdgHomeExists() bool { return true } +func (WithNewHome) basicHomeExists() bool { return false } + +type WithOldHome struct{ DefaultConfigHomePath } + +func (WithOldHome) xdgHomeExists() bool { return false } +func (WithOldHome) basicHomeExists() bool { return true } + +type WithNoHome struct{ DefaultConfigHomePath } + +func (WithNoHome) xdgHomeExists() bool { return false } +func (WithNoHome) basicHomeExists() bool { return false } + +type WithAllHomes struct{ DefaultConfigHomePath } + +func (WithAllHomes) xdgHomeExists() bool { return true } +func (WithAllHomes) basicHomeExists() bool { return true } func TestGetDefaultConfigHome(t *testing.T) { - var _OldDefaultHelmHomeExists = OldDefaultHelmHomeExists - var _DefaultHelmHomeExists = DefaultHelmHomeExists + oldConfig := ConfigPath - OldDefaultHelmHomeExists = returns(false) - DefaultHelmHomeExists = returns(false) - StringEquals(t, GetDefaultConfigHome(os.Stdout), defaultHelmHome) + ConfigPath = WithNewHome{} + StringEquals(t, GetDefaultConfigHome(), defaultHelmHome) - OldDefaultHelmHomeExists = returns(true) - DefaultHelmHomeExists = returns(false) - StringEquals(t, GetDefaultConfigHome(os.Stdout), oldDefaultHelmHome) + ConfigPath = WithOldHome{} + StringEquals(t, GetDefaultConfigHome(), oldDefaultHelmHome) - OldDefaultHelmHomeExists = returns(false) - DefaultHelmHomeExists = returns(true) - StringEquals(t, GetDefaultConfigHome(os.Stdout), defaultHelmHome) + ConfigPath = WithNoHome{} + StringEquals(t, GetDefaultConfigHome(), defaultHelmHome) - OldDefaultHelmHomeExists = returns(true) - DefaultHelmHomeExists = returns(true) - StringEquals(t, GetDefaultConfigHome(os.Stdout), defaultHelmHome) + ConfigPath = WithAllHomes{} + StringEquals(t, GetDefaultConfigHome(), defaultHelmHome) - OldDefaultHelmHomeExists = _OldDefaultHelmHomeExists - DefaultHelmHomeExists = _DefaultHelmHomeExists + ConfigPath = oldConfig } diff --git a/pkg/helm/helmpath/default_home_dir.go b/pkg/helm/helmpath/default_home_dir.go index daea8c9fb..88c7143bb 100644 --- a/pkg/helm/helmpath/default_home_dir.go +++ b/pkg/helm/helmpath/default_home_dir.go @@ -17,47 +17,49 @@ limitations under the License. package helmpath import ( - "fmt" "github.com/casimir/xdg-go" - "io" "k8s.io/client-go/util/homedir" "os" "path/filepath" ) -// Old default helm home, it's old good ~/.helm -var oldDefaultHelmHome = filepath.Join(homedir.HomeDir(), ".helm") - // New default helm home, with different paths for different OS: // - %APPDATA%\helm on Windows // - ~/Library/Preferences/helm on OSX // - $XDG_CONFIG_DIR/helm (typically ~/.config/helm for linux) var defaultHelmHome = filepath.Join(xdg.ConfigHome(), "helm") -func DirExists(path string) bool { - osStat, err := os.Stat(path) - return err == nil && osStat.IsDir() +// Old default helm home, it's old good ~/.helm +var oldDefaultHelmHome = filepath.Join(homedir.HomeDir(), ".helm") + +type DefaultConfigHomePath interface { + xdgHomeExists() bool + basicHomeExists() bool } -// Check whether new default helm home exists -// TODO: improve me -var DefaultHelmHomeExists = func() bool { +type FSConfigHomePath struct{ DefaultConfigHomePath } + +// Checks whether $XDG_CONFIG_HOME/helm exists +func (FSConfigHomePath) xdgHomeExists() bool { return DirExists(defaultHelmHome) } -// Checks whether old-style ~/.helm exists -// TODO: improve me -var OldDefaultHelmHomeExists = func() bool { +// Checks whether ~/.helm exists +func (FSConfigHomePath) basicHomeExists() bool { return DirExists(oldDefaultHelmHome) } +var ConfigPath DefaultConfigHomePath = FSConfigHomePath{} + +func DirExists(path string) bool { + osStat, err := os.Stat(path) + return err == nil && osStat.IsDir() +} + // Get configuration home dir. -// -// Note: Temporal until all migrate to XDG Base Directory spec -func GetDefaultConfigHome(out io.Writer) string { - if DefaultHelmHomeExists() || !OldDefaultHelmHomeExists() { +func GetDefaultConfigHome() string { + if ConfigPath.xdgHomeExists() || !ConfigPath.basicHomeExists() { return defaultHelmHome } - fmt.Fprintf(out, "WARNING: using old-style configuration directory. Please, consider moving it to %s\n", defaultHelmHome) return oldDefaultHelmHome }