PT-2236 - pt-secure-collect, pt-pg-summary do not follow PT standard (#652)

* PT-2236 - pt-secure-collect, pt-pg-summary do not follow PT standard for option --version

- Redirected UsageWriter to os.Stdout for kingpin, so --version output
  goes to STDOUT, not to STDERR
- Adjusted text, printed by the --version flag
- Added test cases to check how --version flag works
- Adjusted test cases, so they use TOOLNAME constant

* PT-2236 - pt-secure-collect, pt-pg-summary do not follow PT standard for option --version

Run go mod tidy as requested by Artem Gavrilov

* PT-2236 - pt-secure-collect, pt-pg-summary do not follow PT standard for option --version

Renamed const TOOLNAME to toolname to follow Go coding standards

src/go/pt-k8s-debug-collector/main.go

@@ -10,7 +10,7 @@ import (
 )
 
 const (
-	TOOLNAME = "pt-k8s-debug-collector"
+	toolname = "pt-k8s-debug-collector"
 )
 
 // We do not set anything here, these variables are defined by the Makefile
@@ -38,7 +38,7 @@ func main() {
 	flag.Parse()
 
 	if version {
-		fmt.Println(TOOLNAME)
+		fmt.Println(toolname)
 		fmt.Printf("Version %s\n", Version)
 		fmt.Printf("Build: %s using %s\n", Build, GoVersion)
 		fmt.Printf("Commit: %s\n", Commit)

src/go/pt-k8s-debug-collector/main_test.go

@@ -164,14 +164,14 @@ func TestResourceOption(t *testing.T) {
 Option --version
 */
 func TestVersionOption(t *testing.T) {
-	out, err := exec.Command("../../../bin/pt-k8s-debug-collector", "--version").Output()
+	out, err := exec.Command("../../../bin/"+toolname, "--version").Output()
 	if err != nil {
-		t.Errorf("error executing pt-k8s-debug-collector --version: %s", err.Error())
+		t.Errorf("error executing %s --version: %s", toolname, err.Error())
 	}
 	// We are using MustCompile here, because hard-coded RE should not fail
-	re := regexp.MustCompile(TOOLNAME + `\n.*Version v?\d+\.\d+\.\d+\n`)
+	re := regexp.MustCompile(toolname + `\n.*Version v?\d+\.\d+\.\d+\n`)
 	if !re.Match(out) {
-		t.Errorf("pt-k8s-debug-collector --version returns wrong result:\n%s", out)
+		t.Errorf("%s --version returns wrong result:\n%s", toolname, out)
 	}
 }
 

src/go/pt-mongodb-index-check/main.go

@@ -42,7 +42,7 @@ type response struct {
 }
 
 const (
-	TOOLNAME = "pt-mongodb-index-check"
+	toolname = "pt-mongodb-index-check"
 )
 
 // We do not set anything here, these variables are defined by the Makefile
@@ -58,7 +58,7 @@ func main() {
 	kongctx := kong.Parse(&args, kong.UsageOnError())
 
 	if kongctx.Command() == "version" {
-		fmt.Println(TOOLNAME)
+		fmt.Println(toolname)
 		fmt.Printf("Version %s\n", Version)
 		fmt.Printf("Build: %s using %s\n", Build, GoVersion)
 		fmt.Printf("Commit: %s\n", Commit)

src/go/pt-mongodb-query-digest/main.go

@@ -30,7 +30,7 @@ import (
 )
 
 const (
-	TOOLNAME = "pt-mongodb-query-digest"
+	toolname = "pt-mongodb-query-digest"
 
 	DEFAULT_AUTHDB          = "admin"
 	DEFAULT_HOST            = "localhost:27017"
@@ -90,16 +90,16 @@ func main() {
 	log.SetLevel(logLevel)
 
 	if opts.Version {
-		fmt.Println(TOOLNAME)
+		fmt.Println(toolname)
 		fmt.Printf("Version %s\n", Version)
 		fmt.Printf("Build: %s using %s\n", Build, GoVersion)
 		fmt.Printf("Commit: %s\n", Commit)
 		return
 	}
 
-	conf := config.DefaultConfig(TOOLNAME)
+	conf := config.DefaultConfig(toolname)
 	if !conf.GetBool("no-version-check") && !opts.NoVersionCheck {
-		advice, err := versioncheck.CheckUpdates(TOOLNAME, Version)
+		advice, err := versioncheck.CheckUpdates(toolname, Version)
 		if err != nil {
 			log.Infof("cannot check version updates: %s", err.Error())
 		} else if advice != "" {
@@ -361,7 +361,7 @@ func getClientOptions(opts *cliOptions) (*options.ClientOptions, error) {
 
 func getHeaders(opts *cliOptions) []string {
 	h := []string{
-		fmt.Sprintf("%s - %s\n", TOOLNAME, time.Now().Format(time.RFC1123Z)),
+		fmt.Sprintf("%s - %s\n", toolname, time.Now().Format(time.RFC1123Z)),
 		fmt.Sprintf("Host: %s\n", opts.Host),
 		fmt.Sprintf("Skipping profiled queries on these collections: %v\n", opts.SkipCollections),
 	}

src/go/pt-mongodb-query-digest/main_test.go

@@ -106,7 +106,7 @@ func TestParseArgs(t *testing.T) {
 		want *cliOptions
 	}{
 		{
-			args: []string{TOOLNAME}, // arg[0] is the command itself
+			args: []string{toolname}, // arg[0] is the command itself
 			want: &cliOptions{
 				Host:            "mongodb://" + DEFAULT_HOST,
 				LogLevel:        DEFAULT_LOGLEVEL,
@@ -117,11 +117,11 @@ func TestParseArgs(t *testing.T) {
 			},
 		},
 		{
-			args: []string{TOOLNAME, "zapp.brannigan.net:27018/samples", "--help"},
+			args: []string{toolname, "zapp.brannigan.net:27018/samples", "--help"},
 			want: nil,
 		},
 		{
-			args: []string{TOOLNAME, "zapp.brannigan.net:27018/samples"},
+			args: []string{toolname, "zapp.brannigan.net:27018/samples"},
 			want: &cliOptions{
 				Host:            "mongodb://zapp.brannigan.net:27018/samples",
 				LogLevel:        DEFAULT_LOGLEVEL,

src/go/pt-mongodb-summary/main.go

@@ -35,7 +35,7 @@ import (
 )
 
 const (
-	TOOLNAME = "pt-mongodb-summary"
+	toolname = "pt-mongodb-summary"
 
 	DefaultAuthDB             = "admin"
 	DefaultHost               = "mongodb://localhost:27017"
@@ -194,7 +194,7 @@ func main() {
 	log.SetLevel(logLevel)
 
 	if opts.Version {
-		fmt.Println(TOOLNAME)
+		fmt.Println(toolname)
 		fmt.Printf("Version %s\n", Version)
 		fmt.Printf("Build: %s using %s\n", Build, GoVersion)
 		fmt.Printf("Commit: %s\n", Commit)
@@ -202,9 +202,9 @@ func main() {
 		return
 	}
 
-	conf := config.DefaultConfig(TOOLNAME)
+	conf := config.DefaultConfig(toolname)
 	if !conf.GetBool("no-version-check") && !opts.NoVersionCheck {
-		advice, err := versioncheck.CheckUpdates(TOOLNAME, Version)
+		advice, err := versioncheck.CheckUpdates(toolname, Version)
 		if err != nil {
 			log.Infof("cannot check version updates: %s", err.Error())
 		} else if advice != "" {

src/go/pt-mongodb-summary/main_test.go

@@ -101,7 +101,7 @@ func TestParseArgs(t *testing.T) {
 		want *cliOptions
 	}{
 		{
-			args: []string{TOOLNAME}, // arg[0] is the command itself
+			args: []string{toolname}, // arg[0] is the command itself
 			want: &cliOptions{
 				Host:               DefaultHost,
 				LogLevel:           DefaultLogLevel,
@@ -112,7 +112,7 @@ func TestParseArgs(t *testing.T) {
 			},
 		},
 		{
-			args: []string{TOOLNAME, "zapp.brannigan.net:27018/samples", "--help"},
+			args: []string{toolname, "zapp.brannigan.net:27018/samples", "--help"},
 			want: nil,
 		},
 	}

src/go/pt-pg-summary/main.go

@@ -17,6 +17,10 @@ import (
 	"github.com/percona/percona-toolkit/src/go/pt-pg-summary/templates"
 )
 
+const (
+	toolname = "pt-pg-summary"
+)
+
 // We do not set anything here, these variables are defined by the Makefile
 var (
 	Build     string //nolint
@@ -211,10 +215,11 @@ func safeConnString(opts connOpts, dbName string) string {
 }
 
 func parseCommandLineOpts(args []string) (cliOptions, error) {
-	app := kingpin.New("pt-pg-summary", "Percona Toolkit - PostgreSQL Summary")
+	app := kingpin.New(toolname, "Percona Toolkit - PostgreSQL Summary")
+	app.UsageWriter(os.Stdout)
 	// version, commit and date will be set at build time by the compiler -ldflags param
-	app.Version(fmt.Sprintf("%s version %s\nGIT commit %s\nDate: %s\nGo version: %s",
+	app.Version(fmt.Sprintf("%s\nVersion %s\nBuild: %s using %s\nCommit: %s",
-		app.Name, Version, Commit, Build, GoVersion))
+		app.Name, Version, Build, GoVersion, Commit))
 	opts := cliOptions{app: app}
 
 	app.Flag("ask-pass", "Prompt for a password when connecting to PostgreSQL").

src/go/pt-pg-summary/main_test.go

@@ -3,6 +3,8 @@ package main
 import (
 	"fmt"
 	"os"
+	"os/exec"
+	"regexp"
 	"testing"
 
 	"github.com/sirupsen/logrus"
@@ -122,3 +124,18 @@ func TestCollectPerDatabaseInfo(t *testing.T) {
 		})
 	}
 }
+
+/*
+Option --version
+*/
+func TestVersionOption(t *testing.T) {
+	out, err := exec.Command("../../../bin/"+toolname, "--version").Output()
+	if err != nil {
+		t.Errorf("error executing %s --version: %s", toolname, err.Error())
+	}
+	// We are using MustCompile here, because hard-coded RE should not fail
+	re := regexp.MustCompile(toolname + `\n.*Version v?\d+\.\d+\.\d+\n`)
+	if !re.Match(out) {
+		t.Errorf("%s --version returns wrong result:\n%s", toolname, out)
+	}
+}

src/go/pt-secure-collect/main.go

@@ -67,7 +67,7 @@ type myDefaults struct {
 }
 
 const (
-	TOOLNAME = "pt-secure-collect"
+	toolname = "pt-secure-collect"
 
 	decryptCmd       = "decrypt"
 	encryptCmd       = "encrypt"
@@ -172,15 +172,17 @@ func processCliParams(baseTempPath string, usageWriter io.Writer) (*cliOptions,
 	}
 	msg += "\n "
 
-	app := kingpin.New(TOOLNAME, msg)
+	app := kingpin.New(toolname, msg)
 	if usageWriter != nil {
 		app.UsageWriter(usageWriter)
 		app.Terminate(nil)
+	} else {
+		app.UsageWriter(os.Stdout)
 	}
 
 	// Add support for --version flag
-	app.Version(TOOLNAME + "\nVersion " + Version + "\nBuild: " + Build + " using " + GoVersion +
+	app.Version(toolname + "\nVersion " + Version + "\nBuild: " + Build + " using " + GoVersion +
-		" Go version: " + GoVersion)
+		"\nCommit:" + Commit)
 
 	opts := &cliOptions{
 		CollectCommand:  app.Command(collectCmd, "Collect, sanitize, pack and encrypt data from pt-tools."),

src/go/pt-secure-collect/main_test.go

@@ -4,7 +4,9 @@ import (
 	"bufio"
 	"bytes"
 	"os"
+	"os/exec"
 	"reflect"
+	"regexp"
 	"testing"
 )
 
@@ -38,3 +40,18 @@ func TestProcessCliParams(t *testing.T) {
 
 func TestCollect(t *testing.T) {
 }
+
+/*
+Option --version
+*/
+func TestVersionOption(t *testing.T) {
+	out, err := exec.Command("../../../bin/"+toolname, "--version").Output()
+	if err != nil {
+		t.Errorf("error executing %s --version: %s", toolname, err.Error())
+	}
+	// We are using MustCompile here, because hard-coded RE should not fail
+	re := regexp.MustCompile(toolname + `\n.*Version v?\d+\.\d+\.\d+\n`)
+	if !re.Match(out) {
+		t.Errorf("%s --version returns wrong result:\n%s", toolname, out)
+	}
+}