Improve code readability

In general, remove as many hard-coded text as possible:
- Add function IsEnvSet to check when env variables are set;
- Add variables for fetures minimum versions;
- Add constants for sandbox script names;
- Add constants for most used error messages;
- Add comparison functions for testing in a separate package;
- Replace manual path composition with path.Join;

README.md

@@ -3,7 +3,7 @@
 [DBdeployer](https://github.com/datacharmer/dbdeployer) is a tool that deploys MySQL database servers easily.
 This is a port of [MySQL-Sandbox](https://github.com/datacharmer/mysql-sandbox), originally written in Perl, and re-designed from the ground up in [Go](https://golang.org). See the [features comparison](https://github.com/datacharmer/dbdeployer/blob/master/docs/features.md) for more detail.
 
-Documentation updated for version 1.13.0 (27-Oct-2018 16:12 UTC)
+Documentation updated for version 1.14.0 (11-Nov-2018 07:34 UTC)
 
 [![Build Status](https://travis-ci.org/datacharmer/dbdeployer.svg "Travis CI status")](https://travis-ci.org/datacharmer/dbdeployer)
 
@@ -15,7 +15,7 @@ Get the one for your O.S. from [dbdeployer releases](https://github.com/datachar
 
 For example:
 
-    $ VERSION=1.13.0
+    $ VERSION=1.14.0
     $ OS=linux
     $ origin=https://github.com/datacharmer/dbdeployer/releases/download/$VERSION
     $ wget $origin/dbdeployer-$VERSION.$OS.tar.gz
@@ -50,7 +50,7 @@ For example:
 The program doesn't have any dependencies. Everything is included in the binary. Calling *dbdeployer* without arguments or with ``--help`` will show the main help screen.
 
     $ dbdeployer --version
-    dbdeployer version 1.13.0
+    dbdeployer version 1.14.0
 
 
     $ dbdeployer -h
@@ -906,18 +906,18 @@ Should you need to compile your own binaries for dbdeployer, follow these steps:
 1. Make sure you have go installed in your system, and that the ``$GOPATH`` variable is set.
 2. Run ``go get -u github.com/datacharmer/dbdeployer``.  This will import all the code that is needed to build dbdeployer.
 3. Change directory to ``$GOPATH/src/github.com/datacharmer/dbdeployer``.
-4. Run ``./scripts/build.sh {linux|OSX} 1.13.0``
-5. If you need the docs enabled binaries (see the section "Generating additional documentation") run ``MKDOCS=1 ./scripts/build.sh {linux|OSX} 1.13.0``
+4. Run ``./scripts/build.sh {linux|OSX} 1.14.0``
+5. If you need the docs enabled binaries (see the section "Generating additional documentation") run ``MKDOCS=1 ./scripts/build.sh {linux|OSX} 1.14.0``
 
 ## Generating additional documentation
 
 Between this file and [the API API list](https://github.com/datacharmer/dbdeployer/blob/master/docs/API/API-1.1.md), you have all the existing documentation for dbdeployer.
 Should you need additional formats, though, dbdeployer is able to generate them on-the-fly. Tou will need the docs-enabled binaries: in the distribution list, you will find:
 
-* dbdeployer-1.13.0-docs.linux.tar.gz
-* dbdeployer-1.13.0-docs.osx.tar.gz
-* dbdeployer-1.13.0.linux.tar.gz
-* dbdeployer-1.13.0.osx.tar.gz
+* dbdeployer-1.14.0-docs.linux.tar.gz
+* dbdeployer-1.14.0-docs.osx.tar.gz
+* dbdeployer-1.14.0.linux.tar.gz
+* dbdeployer-1.14.0.osx.tar.gz
 
 The executables containing ``-docs`` in their name have the same capabilities of the regular ones, but in addition they can run the *hidden* command ``tree``, with alias ``docs``.
 

abbreviations/abbreviations.go

@@ -69,7 +69,7 @@ func debugPrint(descr string, v interface{}) {
 }
 
 func LoadAbbreviations() {
-	if os.Getenv("SKIP_ABBR") != "" {
+	if common.IsEnvSet("SKIP_ABBR") {
 		fmt.Printf("# Abbreviations suppressed by env variable SKIP_ABBR\n")
 		return
 	}
@@ -78,7 +78,7 @@ func LoadAbbreviations() {
 	var variables = make(common.StringMap)
 	var verboseAbbr bool = true
 	var replacementsUsed bool = false
-	if os.Getenv("SILENT_ABBR") != "" {
+	if common.IsEnvSet("SILENT_ABBR") {
 		verboseAbbr = false
 	}
 	if userDefinedFile != "" {
@@ -176,7 +176,7 @@ func LoadAbbreviations() {
 }
 
 func init() {
-	if os.Getenv("DEBUG_ABBR") != "" {
+	if common.IsEnvSet("DEBUG_ABBR") {
 		debugAbbr = true
 	}
 }

abbreviations/abbreviations_test.go

@@ -17,30 +17,11 @@ package abbreviations
 
 import (
 	"github.com/datacharmer/dbdeployer/common"
+	"github.com/datacharmer/dbdeployer/compare"
 	"os"
 	"testing"
 )
 
-func okEqualSlices(t *testing.T, found, expected []string) {
-
-	if len(expected) != len(found) {
-		t.Logf("not ok - slice Found has %d elements, while slice Expected has %d\n", len(found), len(expected))
-		t.Logf("Found: %v", found)
-		t.Logf("Expected: %v", expected)
-		t.Fail()
-		return
-	}
-	for N := 0; N < len(found); N++ {
-		if found[N] == expected[N] {
-			t.Logf("ok - element %d of Found and the same in Expected are equal [%v]\n", N, found[N])
-		} else {
-			t.Logf("not ok - element %d of Found differs from the corresponding one in Expected. "+
-				"Expected '%s' - found: '%s'\n", N, expected[N], found[N])
-			t.Fail()
-		}
-	}
-}
-
 func TestLoadAbbreviations(t *testing.T) {
 	userDefinedFile = "/tmp/abbreviations.txt"
 	type abbrData struct {
@@ -76,9 +57,9 @@ func TestLoadAbbreviations(t *testing.T) {
 	for _, d := range data {
 		os.Args = d.commandLine
 		LoadAbbreviations()
-		okEqualSlices(t, os.Args, d.expected)
+		compare.OkEqualStringSlices(t, os.Args, d.expected)
 	}
 
 	os.Args = saveArgs
-	okEqualSlices(t, os.Args, saveArgs)
+	compare.OkEqualStringSlices(t, os.Args, saveArgs)
 }

cmd/admin.go

@@ -18,76 +18,78 @@ package cmd
 import (
 	"fmt"
 	"github.com/datacharmer/dbdeployer/common"
+	"github.com/datacharmer/dbdeployer/defaults"
 	"github.com/datacharmer/dbdeployer/sandbox"
 	"github.com/spf13/cobra"
 	"os"
+	"path"
 )
 
 func UnpreserveSandbox(sandboxDir, sandboxName string) {
-	fullPath := sandboxDir + "/" + sandboxName
+	fullPath := path.Join(sandboxDir, sandboxName)
 	if !common.DirExists(fullPath) {
-		common.Exitf(1, "Directory '%s' not found", fullPath)
+		common.Exitf(1, defaults.ErrDirectoryNotFound, fullPath)
 	}
-	preserve := fullPath + "/no_clear_all"
+	preserve := path.Join(fullPath, defaults.ScriptNoClearAll)
 	if !common.ExecExists(preserve) {
-		preserve = fullPath + "/no_clear"
+		preserve = path.Join(fullPath, defaults.ScriptNoClear)
 	}
 	if !common.ExecExists(preserve) {
 		fmt.Printf("Sandbox %s is not locked\n", sandboxName)
 		return
 	}
 	isMultiple := true
-	clear := fullPath + "/clear_all"
+	clear := path.Join(fullPath, defaults.ScriptClearAll)
 	if !common.ExecExists(clear) {
-		clear = fullPath + "/clear"
+		clear = path.Join(fullPath, defaults.ScriptClear)
 		isMultiple = false
 	}
 	if !common.ExecExists(clear) {
-		common.Exitf(1, "Executable '%s' not found", clear)
+		common.Exitf(1, defaults.ErrExecutableNotFound, clear)
 	}
-	noClear := fullPath + "/no_clear"
+	noClear := path.Join(fullPath, defaults.ScriptNoClear)
 	if isMultiple {
-		noClear = fullPath + "/no_clear_all"
+		noClear = path.Join(fullPath, defaults.ScriptNoClearAll)
 	}
 	err := os.Remove(clear)
-	common.ErrCheckExitf(err, 1, "Error while removing %s \n%s", clear, err)
+	common.ErrCheckExitf(err, 1, defaults.ErrWhileRemoving, clear, err)
 	err = os.Rename(noClear, clear)
-	common.ErrCheckExitf(err, 1, "Error while renaming  script\n%s", err)
+	common.ErrCheckExitf(err, 1, defaults.ErrWhileRenamingScript, err)
 	fmt.Printf("Sandbox %s unlocked\n", sandboxName)
 }
 
 func PreserveSandbox(sandboxDir, sandboxName string) {
-	fullPath := sandboxDir + "/" + sandboxName
+	fullPath := path.Join(sandboxDir, sandboxName)
 	if !common.DirExists(fullPath) {
-		common.Exitf(1, "Directory '%s' not found", fullPath)
+		common.Exitf(1, defaults.ErrDirectoryNotFound, fullPath)
 	}
-	preserve := fullPath + "/no_clear_all"
+	preserve := path.Join(fullPath, defaults.ScriptNoClearAll)
 	if !common.ExecExists(preserve) {
-		preserve = fullPath + "/no_clear"
+		preserve = path.Join(fullPath, defaults.ScriptNoClear)
 	}
 	if common.ExecExists(preserve) {
 		fmt.Printf("Sandbox %s is already locked\n", sandboxName)
 		return
 	}
 	isMultiple := true
-	clear := fullPath + "/clear_all"
+	clear := path.Join(fullPath, defaults.ScriptClearAll)
 	if !common.ExecExists(clear) {
-		clear = fullPath + "/clear"
+		clear = path.Join(fullPath, defaults.ScriptClear)
 		isMultiple = false
 	}
 	if !common.ExecExists(clear) {
-		common.Exitf(1, "Executable '%s' not found", clear)
+		common.Exitf(1, defaults.ErrExecutableNotFound, clear)
 	}
-	noClear := fullPath + "/no_clear"
-	clearCmd := "clear"
-	noClearCmd := "no_clear"
+	noClear := path.Join(fullPath, defaults.ScriptNoClear)
+	clearCmd := defaults.ScriptClear
+	noClearCmd := defaults.ScriptNoClear
 	if isMultiple {
-		noClear = fullPath + "/no_clear_all"
-		clearCmd = "clear_all"
-		noClearCmd = "no_clear_all"
+		noClear = path.Join(fullPath, defaults.ScriptNoClearAll)
+		clearCmd = defaults.ScriptClearAll
+		noClearCmd = defaults.ScriptNoClearAll
 	}
 	err := os.Rename(clear, noClear)
-	common.ErrCheckExitf(err, 1, "Error while renaming script.\n%s", err)
+	common.ErrCheckExitf(err, 1, defaults.ErrWhileRenamingScript, err)
 	template := sandbox.SingleTemplates["sb_locked_template"].Contents
 	var data = common.StringMap{
 		"TemplateName": "sb_locked_template",
@@ -156,25 +158,25 @@ func UpgradeSandbox(sandboxDir, oldSandbox, newSandbox string) {
 		"8.0": "8.0",
 	}
 	err := os.Chdir(sandboxDir)
-	common.ErrCheckExitf(err, 1, "Error: can't change directory to %s", sandboxDir)
-	scripts := []string{"start", "stop", "my"}
+	common.ErrCheckExitf(err, 1, "can't change directory to %s", sandboxDir)
+	scripts := []string{defaults.ScriptStart, defaults.ScriptStop, defaults.ScriptMy}
 	for _, dir := range []string{oldSandbox, newSandbox} {
 		if !common.DirExists(dir) {
-			common.Exitf(1, "Error: Directory %s not found in %s", dir, sandboxDir)
+			common.Exitf(1, defaults.ErrDirectoryNotFoundInUpper, dir, sandboxDir)
 		}
 		for _, script := range scripts {
-			if !common.ExecExists(dir + "/" + script) {
-				common.Exit(1, fmt.Sprintf("Error: script %s not found in %s", script, dir),
+			if !common.ExecExists(path.Join(dir, script)) {
+				common.Exit(1, fmt.Sprintf(defaults.ErrScriptNotFoundInUpper, script, dir),
 					"The upgrade only works between SINGLE deployments")
 			}
 		}
 	}
 	newSbdesc := common.ReadSandboxDescription(newSandbox)
 	oldSbdesc := common.ReadSandboxDescription(oldSandbox)
-	mysqlUpgrade := newSbdesc.Basedir + "/bin/mysql_upgrade"
+	mysqlUpgrade := path.Join(newSbdesc.Basedir, "bin", "mysql_upgrade")
 	if !common.ExecExists(mysqlUpgrade) {
-		common.WriteString("", newSandbox+"/no_upgrade")
-		common.Exit(0, "mysql_upgrade not found in %s. Upgrade is not possible", newSbdesc.Basedir)
+		common.WriteString("", path.Join(newSandbox, "no_upgrade"))
+		common.Exitf(0, "mysql_upgrade not found in %s. Upgrade is not possible", newSbdesc.Basedir)
 	}
 	newVersionList := common.VersionToList(newSbdesc.Version)
 	newMajor := newVersionList[0]
@@ -187,10 +189,10 @@ func UpgradeSandbox(sandboxDir, oldSandbox, newSandbox string) {
 	newUpgradeVersion := fmt.Sprintf("%d.%d", newVersionList[0], newVersionList[1])
 	oldUpgradeVersion := fmt.Sprintf("%d.%d", oldVersionList[0], oldVersionList[1])
 	if oldMajor == 10 || newMajor == 10 {
-		common.Exit(1, "Upgrade from and to MariaDB is not supported")
+		common.Exit(1, "upgrade from and to MariaDB is not supported")
 	}
 	if common.GreaterOrEqualVersion(oldSbdesc.Version, newVersionList) {
-		common.Exitf(1, "Version %s must be greater than %s", newUpgradeVersion, oldUpgradeVersion)
+		common.Exitf(1, "version %s must be greater than %s", newUpgradeVersion, oldUpgradeVersion)
 	}
 	canBeUpgraded := false
 	if oldMajor < newMajor {
@@ -205,30 +207,30 @@ func UpgradeSandbox(sandboxDir, oldSandbox, newSandbox string) {
 		}
 	}
 	if !canBeUpgraded {
-		common.Exitf(1, "Version %s can only be upgraded to %s or to the same version with a higher revision", oldUpgradeVersion, possibleUpgrades[oldUpgradeVersion])
+		common.Exitf(1, "version '%s' can only be upgraded to '%s' or to the same version with a higher revision", oldUpgradeVersion, possibleUpgrades[oldUpgradeVersion])
 	}
-	newSandboxOldData := newSandbox + "/data-" + newSandbox
+	newSandboxOldData := path.Join(newSandbox, defaults.DataDirName+"-"+newSandbox)
 	if common.DirExists(newSandboxOldData) {
-		common.Exitf(1, "Sandbox %s is already the upgrade from an older version", newSandbox)
+		common.Exitf(1, "sandbox '%s' is already the upgrade from an older version", newSandbox)
 	}
-	err, _ = common.RunCmd(oldSandbox + "/stop")
-	common.ErrCheckExitf(err, 1, "Error while stopping sandbox %s", oldSandbox)
-	err, _ = common.RunCmd(newSandbox + "/stop")
-	common.ErrCheckExitf(err, 1, "Error while stopping sandbox %s", newSandbox)
-	mvArgs := []string{newSandbox + "/data", newSandboxOldData}
+	err, _ = common.RunCmd(path.Join(oldSandbox, defaults.ScriptStop))
+	common.ErrCheckExitf(err, 1, defaults.ErrWhileStoppingSandbox, oldSandbox)
+	err, _ = common.RunCmd(path.Join(newSandbox, defaults.ScriptStop))
+	common.ErrCheckExitf(err, 1, defaults.ErrWhileStoppingSandbox, newSandbox)
+	mvArgs := []string{path.Join(newSandbox, defaults.DataDirName), newSandboxOldData}
 	err, _ = common.RunCmdWithArgs("mv", mvArgs)
-	common.ErrCheckExitf(err, 1, "Error while moving data directory in sandbox %s", newSandbox)
+	common.ErrCheckExitf(err, 1, "error while moving data directory in sandbox %s", newSandbox)
 
-	mvArgs = []string{oldSandbox + "/data", newSandbox + "/data"}
+	mvArgs = []string{path.Join(oldSandbox, defaults.DataDirName), path.Join(newSandbox, defaults.DataDirName)}
 	err, _ = common.RunCmdWithArgs("mv", mvArgs)
-	common.ErrCheckExitf(err, 1, "Error while moving data directory from sandbox %s to %s", oldSandbox, newSandbox)
+	common.ErrCheckExitf(err, 1, "error while moving data directory from sandbox %s to %s", oldSandbox, newSandbox)
 	fmt.Printf("Data directory %s/data moved to %s/data \n", oldSandbox, newSandbox)
 
-	err, _ = common.RunCmd(newSandbox + "/start")
-	common.ErrCheckExitf(err, 1, "Error while starting sandbox %s", newSandbox)
+	err, _ = common.RunCmd(path.Join(newSandbox, defaults.ScriptStart))
+	common.ErrCheckExitf(err, 1, defaults.ErrWhileStartingSandbox, newSandbox)
 	upgradeArgs := []string{"sql_upgrade"}
-	err, _ = common.RunCmdWithArgs(newSandbox+"/my", upgradeArgs)
-	common.ErrCheckExitf(err, 1, "Error while running mysql_upgrade in %s", newSandbox)
+	err, _ = common.RunCmdWithArgs(path.Join(newSandbox, defaults.ScriptMy), upgradeArgs)
+	common.ErrCheckExitf(err, 1, "error while running mysql_upgrade in %s", newSandbox)
 	fmt.Println("")
 	fmt.Printf("The data directory from %s/data is preserved in %s\n", newSandbox, newSandboxOldData)
 	fmt.Printf("The data directory from %s/data is now used in %s/data\n", oldSandbox, newSandbox)

cmd/defaults.go

@@ -55,7 +55,7 @@ func ExportDefaults(cmd *cobra.Command, args []string) {
 	}
 	filename := args[0]
 	if common.FileExists(filename) {
-		common.Exitf(1, "File %s already exists. Will not overwrite", filename)
+		common.Exitf(1, "file '%s' already exists. Will not overwrite", filename)
 	}
 	defaults.WriteDefaultsFile(filename, defaults.Defaults())
 	fmt.Printf("# Defaults exported to file %s\n", filename)

cmd/delete.go

@@ -24,6 +24,7 @@ import (
 	"github.com/datacharmer/dbdeployer/sandbox"
 	"github.com/spf13/cobra"
 	"os"
+	"path"
 )
 
 func DeleteSandbox(cmd *cobra.Command, args []string) {
@@ -37,7 +38,7 @@ func DeleteSandbox(cmd *cobra.Command, args []string) {
 	sandboxName := args[0]
 	confirm, _ := flags.GetBool(defaults.ConfirmLabel)
 	runConcurrently, _ := flags.GetBool(defaults.ConcurrentLabel)
-	if os.Getenv("RUN_CONCURRENTLY") != "" {
+	if common.IsEnvSet("RUN_CONCURRENTLY") {
 		runConcurrently = true
 	}
 	skipConfirm, _ := flags.GetBool(defaults.SkipConfirmLabel)
@@ -102,7 +103,7 @@ func DeleteSandbox(cmd *cobra.Command, args []string) {
 	}
 	concurrent.RunParallelTasksByPriority(execLists)
 	for _, sb := range deletionList {
-		fullPath := sandboxDir + "/" + sb.SandboxName
+		fullPath := path.Join(sandboxDir, sb.SandboxName)
 		if !sb.Locked {
 			defaults.DeleteFromCatalog(fullPath)
 		}

cmd/deploy.go

@@ -22,6 +22,7 @@ import (
 	"github.com/spf13/cobra"
 	"math/rand"
 	"os"
+	"path"
 )
 
 var deployCmd = &cobra.Command{
@@ -31,7 +32,7 @@ var deployCmd = &cobra.Command{
 }
 
 func init() {
-	myloginCnf := os.Getenv("HOME") + "/.mylogin.cnf"
+	myloginCnf := path.Join(os.Getenv("HOME"), ".mylogin.cnf")
 	if common.FileExists(myloginCnf) {
 		// dbdeployer is not compatible with .mylogin.cnf,
 		// as it bypasses --defaults-file and --no-defaults.