Don't prompt for password if explicitly set to blank string. Fixes #73

This commit fixes edge cases where the behavior of Skeema's password option
did not exactly match the behavior of the MySQL command-line client. In
particular, this caused problems with Travis CI's default ~/.my.cnf file.

These command-line invocations now correctly use no password, rather than
prompting from a TTY:
* --password=
* --password=''
* --password=""

Ditto for these option file lines:
* password=
* password=''
* password=""

In other words, Skeema only prompts for a password on STDIN if no equals sign
is found immediately after the password option.

skeema.go

@@ -16,7 +16,7 @@ schema to the filesystem, and apply online schema changes by modifying files.`
 
 // Globals overridden by GoReleaser's ldflags
 var (
-	version = "1.2.2-dev"
+	version = "1.2.3-dev"
 	commit  = "unknown"
 	date    = "unknown"
 )

util/config.go

@@ -31,7 +31,7 @@ func AddGlobalOptions(cmd *mybase.Command) {
 
 	// Visible global options
 	cmd.AddOption(mybase.StringOption("user", 'u', "root", "Username to connect to database host"))
-	cmd.AddOption(mybase.StringOption("password", 'p', "<no password>", "Password for database user; supply with no value to prompt").ValueOptional())
+	cmd.AddOption(mybase.StringOption("password", 'p', "", "Password for database user; omit value to prompt from TTY (default no password)").ValueOptional())
 	cmd.AddOption(mybase.StringOption("host-wrapper", 'H', "", "External bin to shell out to for host lookup; see manual for template vars"))
 	cmd.AddOption(mybase.StringOption("temp-schema", 't', "_skeema_tmp", "Name of temporary schema for intermediate operations, created and dropped each run unless --reuse-temp-schema"))
 	cmd.AddOption(mybase.StringOption("connect-options", 'o', "", "Comma-separated session options to set upon connecting to each database instance"))
@@ -107,14 +107,14 @@ func ProcessSpecialGlobalOptions(cfg *mybase.Config) error {
 	}
 
 	// Special handling for password option: if not supplied at all, check env
-	// var instead. Or if supplied but with no value (empty string, instead of
-	// its default of "<no password>"), prompt on STDIN like mysql client does.
+	// var instead. Or if supplied but with no equals sign or value, prompt on
+	// STDIN like mysql client does.
 	if !cfg.Supplied("password") {
 		if val := os.Getenv("MYSQL_PWD"); val != "" {
 			cfg.CLI.OptionValues["password"] = val
 			cfg.MarkDirty()
 		}
-	} else if cfg.Get("password") == "" {
+	} else if !cfg.SuppliedWithValue("password") {
 		var err error
 		cfg.CLI.OptionValues["password"], err = PromptPassword()
 		cfg.MarkDirty()

util/config_test.go

@@ -20,8 +20,8 @@ func TestAddGlobalConfigFiles(t *testing.T) {
 	// Expectation: global config files not existing isn't fatal
 	cfg := mybase.ParseFakeCLI(t, cmdSuite, "skeema diff")
 	AddGlobalConfigFiles(cfg)
-	if actualPassword := cfg.Get("password"); actualPassword != "<no password>" {
-		t.Errorf("Expected password to be unchanged from default; instead found %s", actualPassword)
+	if cfg.Supplied("password") || cfg.Changed("password") {
+		t.Errorf("Expected password to be unsupplied and unchanged from default; instead found %q", cfg.GetRaw("password"))
 	}
 
 	os.MkdirAll("fake-etc", 0777)
@@ -65,8 +65,8 @@ func TestAddGlobalConfigFiles(t *testing.T) {
 	if actualUser := cfg.Get("user"); actualUser != "two" {
 		t.Errorf("Expected user in fake-home/.my.cnf to take precedence; instead found %s", actualUser)
 	}
-	if actualPassword := cfg.Get("password"); actualPassword != "<no password>" {
-		t.Errorf("Expected password to be unchanged from default; instead found %s", actualPassword)
+	if cfg.Supplied("password") || cfg.Changed("password") {
+		t.Errorf("Expected password to be unsupplied and unchanged from default; instead found %q", cfg.GetRaw("password"))
 	}
 }
 
@@ -127,7 +127,8 @@ func TestPasswordOption(t *testing.T) {
 		t.Errorf("Expected password to be howdyplanet, instead found %s", cfg.Get("password"))
 	}
 
-	// ProcessSpecialGlobalOptions should error if STDIN isn't TTY
+	// ProcessSpecialGlobalOptions should error with valueless password if STDIN
+	// isn't a TTY. Test bare "password" (no =) on both CLI and config file.
 	oldStdin := os.Stdin
 	defer func() {
 		os.Stdin = oldStdin
@@ -140,6 +141,29 @@ func TestPasswordOption(t *testing.T) {
 	if err := ProcessSpecialGlobalOptions(cfg); err == nil {
 		t.Error("Expected ProcessSpecialGlobalOptions to return an error for non-TTY STDIN, but it did not")
 	}
+	cfg = mybase.ParseFakeCLI(t, cmdSuite, "skeema diff --password", fakeFileSource)
+	if err := ProcessSpecialGlobalOptions(cfg); err == nil {
+		t.Error("Expected ProcessSpecialGlobalOptions to return an error for non-TTY STDIN, but it did not")
+	}
+	fakeFileSource["password"] = ""
+	cfg = mybase.ParseFakeCLI(t, cmdSuite, "skeema diff", fakeFileSource)
+	if err := ProcessSpecialGlobalOptions(cfg); err == nil {
+		t.Error("Expected ProcessSpecialGlobalOptions to return an error for non-TTY STDIN, but it did not")
+	}
+
+	// Setting password to an empty string explicitly should not trigger TTY prompt
+	// (note: STDIN intentionally still points to a file here, from test above)
+	cfg = mybase.ParseFakeCLI(t, cmdSuite, "skeema diff --password=")
+	if err := ProcessSpecialGlobalOptions(cfg); err != nil {
+		t.Errorf("Unexpected error from ProcessSpecialGlobalOptions: %v", err)
+	}
+	if cfg.Changed("password") {
+		t.Error("Password unexpectedly considered changed from default")
+	}
+	cfg = mybase.ParseFakeCLI(t, cmdSuite, "skeema diff --password=''")
+	if err := ProcessSpecialGlobalOptions(cfg); err != nil {
+		t.Errorf("Unexpected error from ProcessSpecialGlobalOptions: %v", err)
+	}
 }
 
 func TestSplitConnectOptions(t *testing.T) {