Refactor viper flag tests

Make the tests for the config table driven
Add a whitelist for the flags that viper can configure.

Date:      Tue Aug 18 16:41:05 2016 -0700

cmd/minikube/cmd/root.go

@@ -44,13 +44,18 @@ const (
 	showLibmachineLogs = "show-libmachine-logs"
 )
 
+var viperWhiteList = []string{
+	"v",
+	"alsologtostderr",
+	"log_dir",
+}
+
 // RootCmd represents the base command when called without any subcommands
 var RootCmd = &cobra.Command{
 	Use:   "minikube",
 	Short: "Minikube is a tool for managing local Kubernetes clusters.",
 	Long:  `Minikube is a CLI tool that provisions and manages single-node Kubernetes clusters optimized for development workflows.`,
 	PersistentPreRun: func(cmd *cobra.Command, args []string) {
-		setFlagsUsingViper()
 		for _, path := range dirs {
 			if err := os.MkdirAll(path, 0777); err != nil {
 				glog.Exitf("Error creating minikube directory: %s", err)
@@ -77,7 +82,8 @@ func Execute() {
 // Handle config values for flags used in external packages (e.g. glog)
 // by setting them directly, using values from viper when not passed in as args
 func setFlagsUsingViper() {
-	setFlagValues := func(a *pflag.Flag) {
+	for _, config := range viperWhiteList {
+		var a = pflag.Lookup(config)
 		viper.SetDefault(a.Name, a.DefValue)
 		// If the flag is set, override viper value
 		if a.Changed {
@@ -87,8 +93,6 @@ func setFlagsUsingViper() {
 		// then to values from the config.yml
 		a.Value.Set(viper.GetString(a.Name))
 	}
-
-	pflag.VisitAll(setFlagValues)
 }
 
 func init() {
@@ -101,17 +105,20 @@ func init() {
 // initConfig reads in config file and ENV variables if set.
 func initConfig() {
 	configPath := constants.MakeMiniPath("config")
-
-	// Bind all viper values to env variables
-	viper.SetEnvPrefix(constants.MinikubeEnvPrefix)
-	viper.AutomaticEnv()
-
 	viper.SetConfigName("config")
 	viper.AddConfigPath(configPath)
 	err := viper.ReadInConfig()
 	if err != nil {
 		glog.Warningf("Error reading config file at %s: %s", configPath, err)
 	}
+	setupViper()
+}
+
+func setupViper() {
+	viper.SetEnvPrefix(constants.MinikubeEnvPrefix)
+	viper.AutomaticEnv()
+
 	viper.SetDefault(config.WantUpdateNotification, true)
 	viper.SetDefault(config.ReminderWaitPeriodInHours, 24)
+	setFlagsUsingViper()
 }

cmd/minikube/cmd/root_test.go

@@ -17,7 +17,9 @@ limitations under the License.
 package cmd
 
 import (
+	"bytes"
 	"os"
+	"strings"
 	"testing"
 
 	"github.com/spf13/cobra"
@@ -27,6 +29,76 @@ import (
 	"k8s.io/minikube/pkg/minikube/tests"
 )
 
+var yamlExampleConfig = []byte(`v: 999
+alsologtostderr: true
+log_dir: "/etc/hosts"
+log-flush-frequency: "3s"
+`)
+
+const configName = ".test_minikube_config.yml"
+
+type configTest struct {
+	Name          string
+	EnvValue      string
+	ConfigValue   string
+	FlagValue     string
+	ExpectedValue string
+}
+
+var configTests = []configTest{
+	{
+		Name:          "v",
+		ExpectedValue: "0",
+	},
+	{
+		Name:          "v",
+		ConfigValue:   "999",
+		ExpectedValue: "999",
+	},
+	{
+		Name:          "v",
+		FlagValue:     "0",
+		ExpectedValue: "0",
+	},
+	{
+		Name:          "v",
+		EnvValue:      "123",
+		ExpectedValue: "123",
+	},
+	{
+		Name:          "v",
+		FlagValue:     "3",
+		ExpectedValue: "3",
+	},
+	// Flag should override config and env
+	{
+		Name:          "v",
+		FlagValue:     "3",
+		ConfigValue:   "222",
+		EnvValue:      "888",
+		ExpectedValue: "3",
+	},
+	// Env should override config
+	{
+		Name:          "v",
+		EnvValue:      "2",
+		ConfigValue:   "999",
+		ExpectedValue: "2",
+	},
+	// Config should not override flags not on whitelist
+	{
+		Name:          "log-flush-frequency",
+		ConfigValue:   "6s",
+		ExpectedValue: "5s",
+	},
+	// Env should not override flags not on whitelist
+	{
+		Name:          "log_backtrace_at",
+		EnvValue:      ":2",
+		ExpectedValue: ":0",
+	},
+}
+
 func runCommand(f func(*cobra.Command, []string)) {
 	cmd := cobra.Command{}
 	var args []string
@@ -48,55 +120,54 @@ func TestPreRunDirectories(t *testing.T) {
 	}
 }
 
+func initTestConfig(config string) {
+	viper.SetConfigType("yml")
+	r := bytes.NewReader([]byte(config))
+	viper.ReadConfig(r)
+}
+
+func TestViperConfig(t *testing.T) {
+	defer viper.Reset()
+	initTestConfig("v: 999")
+	if viper.GetString("v") != "999" {
+		t.Fatalf("Viper did not read test config file")
+	}
+}
+
 func getEnvVarName(name string) string {
-	return constants.MinikubeEnvPrefix + name
+	return constants.MinikubeEnvPrefix + "_" + strings.ToUpper(name)
 }
 
-func TestEnvVariable(t *testing.T) {
-	defer os.Unsetenv("WANTUPDATENOTIFICATION")
-	initConfig()
-	os.Setenv(getEnvVarName("WANTUPDATENOTIFICATION"), "true")
-	if !viper.GetBool("WantUpdateNotification") {
-		t.Fatalf("Viper did not respect environment variable")
+func setValues(tt configTest) {
+	if tt.FlagValue != "" {
+		pflag.Set(tt.Name, tt.FlagValue)
+	}
+	if tt.EnvValue != "" {
+		os.Setenv(getEnvVarName(tt.Name), tt.EnvValue)
+	}
+	if tt.ConfigValue != "" {
+		initTestConfig(tt.Name + ": " + tt.ConfigValue)
 	}
 }
 
-func cleanup() {
-	pflag.Set("v", "0")
-	pflag.Lookup("v").Changed = false
+func unsetValues(tt configTest) {
+	var f = pflag.Lookup(tt.Name)
+	f.Value.Set(f.DefValue)
+	f.Changed = false
+
+	os.Unsetenv(getEnvVarName(tt.Name))
+
+	viper.Reset()
 }
 
-func TestFlagShouldOverrideConfig(t *testing.T) {
-	defer cleanup()
-	viper.Set("v", "1337")
-	pflag.Set("v", "100")
-	setFlagsUsingViper()
-	if viper.GetInt("v") != 100 {
-		viper.Debug()
-		t.Fatal("Value from viper config overrode explicit flag value")
-	}
-}
-
-func TestConfigShouldOverrideDefault(t *testing.T) {
-	defer cleanup()
-	viper.Set("v", "1337")
-	setFlagsUsingViper()
-	if viper.GetInt("v") != 1337 {
-		viper.Debug()
-		t.Fatalf("Value from viper config did not override default flag value")
-	}
-}
-
-func TestFallbackToDefaultFlag(t *testing.T) {
-	setFlagsUsingViper()
-
-	if viper.GetInt("stderrthreshold") != 2 {
-		t.Logf("stderrthreshold %s", viper.GetInt("stderrthreshold"))
-		t.Fatalf("The default flag value was overwritten")
-	}
-
-	if viper.GetString("log-flush-frequency") != "5s" {
-		t.Logf("log flush frequency: %s", viper.GetString("log-flush-frequency"))
-		t.Fatalf("The default flag value was overwritten")
+func TestViperAndFlags(t *testing.T) {
+	for _, tt := range configTests {
+		setValues(tt)
+		setupViper()
+		var actual = pflag.Lookup(tt.Name).Value.String()
+		if actual != tt.ExpectedValue {
+			t.Errorf("pflag.Value(%s) => %s, wanted %s [%+v]", tt.Name, actual, tt.ExpectedValue, tt)
+		}
+		unsetValues(tt)
 	}
 }