Improve error handling in tests (#603)

* Assert there are no errors instead of printing the error
* Fix tests that had errors because they weren't written correctly
* Remove unnecessary prints in tests

cmd/commands_suite_test.go

@@ -23,12 +23,15 @@ var _ = BeforeSuite(func() {
 	logs.Logger = logs.NewLogger()
 })
 
-func executeAndProvideOutput(execute func()) string {
+func executeAndProvideOutput(execute func() error) (string, error) {
 	old := os.Stdout // keep backup of the real stdout
-	r, w, _ := os.Pipe()
+	r, w, err := os.Pipe()
+	if err != nil {
+		return "", err
+	}
 	os.Stdout = w
 
-	execute()
+	err = execute()
 
 	outC := make(chan string)
 	// copy the output in a separate goroutine so printing can't block indefinitely
@@ -41,11 +44,11 @@ func executeAndProvideOutput(execute func()) string {
 		outC <- buf.String()
 	}()
 
-	// back to normal state
-	w.Close()
 	os.Stdout = old // restoring the real stdout
+	// back to normal state
+	_ = w.Close()
 	out := <-outC
-	return out
+	return out, err
 }
 
 func createFileInTmpFolder(projectName string, path ...string) {

cmd/init_test.go

@@ -1,7 +1,6 @@
 package commands
 
 import (
-	"fmt"
 	"os"
 
 	. "github.com/onsi/ginkgo"
@@ -11,13 +10,10 @@ import (
 var _ = Describe("Init", func() {
 
 	BeforeEach(func() {
-		err := os.Mkdir(getTestPath("result"), os.ModePerm)
-		if err != nil {
-			fmt.Println("error occurred during dir creation")
-		}
+		Ω(os.MkdirAll(getTestPath("result"), os.ModePerm)).Should(Succeed())
 	})
 	AfterEach(func() {
-		os.RemoveAll(getTestPath("result"))
+		Ω(os.RemoveAll(getTestPath("result"))).Should(Succeed())
 	})
 	It("Sanity", func() {
 		initCmdSrc = getTestPath("mta")
@@ -30,16 +26,10 @@ var _ = Describe("Init", func() {
 var _ = Describe("Build", func() {
 
 	BeforeEach(func() {
-		err := os.Mkdir(getTestPath("result"), os.ModePerm)
-		if err != nil {
-			fmt.Println("error occurred during dir creation")
-		}
+		Ω(os.MkdirAll(getTestPath("result"), os.ModePerm)).Should(Succeed())
 	})
 	AfterEach(func() {
-		err := os.RemoveAll(getTestPath("result"))
-		if err != nil {
-			fmt.Println("error occurred during dir cleanup")
-		}
+		Ω(os.RemoveAll(getTestPath("result"))).Should(Succeed())
 	})
 	It("Failure - wrong platform", func() {
 		buildCmdSrc = getTestPath("mta")

cmd/module_test.go

@@ -26,10 +26,7 @@ var _ = Describe("Commands", func() {
 		buildModuleCmdTrg = getTestPath("result")
 		cleanupCmdTrg = getTestPath("result")
 		logs.Logger = logs.NewLogger()
-		err := os.Mkdir(mtadCmdTrg, os.ModePerm)
-		if err != nil {
-			fmt.Println(err)
-		}
+		Ω(os.MkdirAll(mtadCmdTrg, os.ModePerm)).Should(Succeed())
 	})
 
 	AfterEach(func() {

cmd/root_test.go

@@ -1,7 +1,6 @@
 package commands
 
 import (
-	"fmt"
 	"os"
 	"path/filepath"
 
@@ -38,12 +37,10 @@ var _ = Describe("Root", func() {
 
 	Describe("Execute", func() {
 		It("Sanity", func() {
-			out := executeAndProvideOutput(func() {
-				e := Execute()
-				if e != nil {
-					fmt.Println("error occurred during execute cmd process")
-				}
+			out, err := executeAndProvideOutput(func() error {
+				return Execute()
 			})
+			Ω(err).Should(Succeed())
 			Ω(out).Should(ContainSubstring("help"))
 			Ω(out).Should(ContainSubstring("version"))
 		})

integration/cloud_mta_build_tool_test.go

@@ -670,12 +670,8 @@ func getFileContentFromZip(path string, filename string) ([]byte, error) {
 			if err != nil {
 				return nil, err
 			}
-			defer fc.Close()
-			c, err := ioutil.ReadAll(fc)
-			if err != nil {
-				return nil, err
-			}
-			return c, nil
+			defer fc.Close() // If we got here there won't be another loop iteration
+			return ioutil.ReadAll(fc)
 		}
 	}
 	return nil, fmt.Errorf(`file "%s" not found`, filename)

internal/archive/fsops_test.go

@@ -56,12 +56,10 @@ var _ = Describe("FSOPS", func() {
 			Entry("DirectoryExists", getFullPath("testdata", "level2", "level3")),
 		)
 		It("Fails because file with the same name exists", func() {
-			err := CreateDirIfNotExist(getFullPath("testdata", "level2", "result"))
-			if err != nil {
-				fmt.Println("error occurred during directory creation")
-			}
-			file, _ := os.Create(getFullPath("testdata", "level2", "result", "file"))
-			file.Close()
+			Ω(CreateDirIfNotExist(getFullPath("testdata", "level2", "result"))).Should(Succeed())
+			file, err := os.Create(getFullPath("testdata", "level2", "result", "file"))
+			Ω(err).Should(Succeed())
+			Ω(file.Close()).Should(Succeed())
 			Ω(CreateDirIfNotExist(getFullPath("testdata", "level2", "result", "file"))).Should(HaveOccurred())
 		})
 	})
@@ -380,10 +378,7 @@ var _ = Describe("FSOPS", func() {
 		It("Sanity", func() {
 			sourcePath := getFullPath("testdata", "level2", "level3")
 			targetPath := getFullPath("testdata", "result")
-			err := CreateDirIfNotExist(targetPath)
-			if err != nil {
-				fmt.Println("error occurred during dir creation")
-			}
+			Ω(CreateDirIfNotExist(targetPath)).Should(Succeed())
 			files, _ := ioutil.ReadDir(sourcePath)
 			// Files wrapped to overwrite their methods
 			var filesWrapped []os.FileInfo
@@ -397,10 +392,7 @@ var _ = Describe("FSOPS", func() {
 		It("Sanity - copy in parallel", func() {
 			sourcePath := getFullPath("testdata", "level2", "level3")
 			targetPath := getFullPath("testdata", "result")
-			err := CreateDirIfNotExist(targetPath)
-			if err != nil {
-				fmt.Println("error occurred during dir creation")
-			}
+			Ω(CreateDirIfNotExist(targetPath)).Should(Succeed())
 			files, _ := ioutil.ReadDir(sourcePath)
 			// Files wrapped to overwrite their methods
 			var filesWrapped []os.FileInfo

internal/artifacts/assembly_test.go

@@ -69,17 +69,12 @@ func getFileContentFromZip(path string, filename string) ([]byte, error) {
 	for _, file := range zipFile.File {
 		if strings.Contains(file.Name, filename) {
 			fc, err = file.Open()
-
-			if err != nil {
-				return nil, err
-			}
-			c, err := ioutil.ReadAll(fc)
 			if err != nil {
 				return nil, err
 			}
-			return c, nil
+			defer fc.Close() // If we got here there won't be another loop iteration
+			return ioutil.ReadAll(fc)
 		}
 	}
-	fc.Close()
 	return nil, fmt.Errorf(`file "%s" not found`, filename)
 }

internal/artifacts/cleanup_test.go

@@ -2,7 +2,6 @@ package artifacts
 
 import (
 	"errors"
-	"fmt"
 	"os"
 
 	dir "github.com/SAP/cloud-mta-build-tool/internal/archive"
@@ -14,10 +13,7 @@ import (
 var _ = Describe("Cleanup", func() {
 
 	BeforeEach(func() {
-		err := dir.CreateDirIfNotExist(getTestPath("result", ".mtahtml5_mta_build_tmp"))
-		if err != nil {
-			fmt.Println("error occurred during directory creation")
-		}
+		Ω(dir.CreateDirIfNotExist(getTestPath("result", ".mtahtml5_mta_build_tmp"))).Should(Succeed())
 	})
 
 	AfterEach(func() {

internal/artifacts/manifest_test.go

@@ -1,7 +1,6 @@
 package artifacts
 
 import (
-	"fmt"
 	"os"
 	"strings"
 	"text/template"
@@ -105,8 +104,6 @@ var _ = Describe("manifest", func() {
 			golden := getFileContent(getTestPath("golden_manifestBuildResult.mf"))
 			v, _ := version.GetVersion()
 			golden = strings.Replace(golden, "{{cli_version}}", v.CliVersion, -1)
-			fmt.Println(actual)
-			fmt.Println(golden)
 			Ω(actual).Should(Equal(golden))
 		})
 		It("wrong content types configuration", func() {
@@ -131,8 +128,6 @@ var _ = Describe("manifest", func() {
 			golden := getFileContent(getTestPath("golden_assembly_manifest_no_paths.mf"))
 			v, _ := version.GetVersion()
 			golden = strings.Replace(golden, "{{cli_version}}", v.CliVersion, -1)
-			fmt.Println(actual)
-			fmt.Println(golden)
 			Ω(actual).Should(Equal(golden))
 		})
 		It("With resources", func() {
@@ -148,8 +143,6 @@ var _ = Describe("manifest", func() {
 			golden := getFileContent(getTestPath("golden_assembly_manifest.mf"))
 			v, _ := version.GetVersion()
 			golden = strings.Replace(golden, "{{cli_version}}", v.CliVersion, -1)
-			fmt.Println(actual)
-			fmt.Println(golden)
 			Ω(actual).Should(Equal(golden))
 		})
 		It("With missing module path", func() {