Return errors rather than dropping them during PR creation (#490)

* Return errors rather than dropping them during PR creation

* Generate error messages during PR creation and remove unused "fluxops" pkg

* remove unused code in log_tests.go and change setup test to use temp dir rather than user's actual home dir

Co-authored-by: Jerry Jackson <jerry@weave.works>

cmd/wego/app/add/cmd.go

@@ -31,8 +31,8 @@ var Cmd = &cobra.Command{
 	Use:   "add [--name <name>] [--url <url>] [--branch <branch>] [--path <path within repository>] [--private-key <keyfile>] <repository directory>",
 	Short: "Add a workload repository to a wego cluster",
 	Long: strings.TrimSpace(dedent.Dedent(`
-		Associates an additional application in a git repository with a wego cluster so that its contents may be managed via GitOps
-	`)),
+        Associates an additional application in a git repository with a wego cluster so that its contents may be managed via GitOps
+    `)),
 	Example: `
   # Add application to wego control from local git repository
   wego app add .
@@ -43,7 +43,7 @@ var Cmd = &cobra.Command{
   # Get status of podinfo application
   wego app status podinfo
 `,
-	RunE:    runCmd,
+	RunE:          runCmd,
 	SilenceUsage:  true,
 	SilenceErrors: true,
 	PostRun: func(cmd *cobra.Command, args []string) {

cmd/wego/version/cmd.go

@@ -2,11 +2,11 @@ package version
 
 import (
 	"fmt"
-	"strings"
 
 	log "github.com/sirupsen/logrus"
 	"github.com/weaveworks/go-checkpoint"
-	"github.com/weaveworks/weave-gitops/pkg/fluxops"
+	"github.com/weaveworks/weave-gitops/pkg/flux"
+	"github.com/weaveworks/weave-gitops/pkg/runner"
 
 	"github.com/spf13/cobra"
 )
@@ -72,14 +72,7 @@ func CheckpointParamsWithFlags(params *checkpoint.CheckParams, c *cobra.Command)
 	return p
 }
 func CheckFluxVersion() (string, error) {
-	fluxops.SetFluxHandler(&fluxops.QuietFluxHandler{})
-	output, err := fluxops.CallFlux("-v")
-	if err != nil {
-		return "", err
-	}
-
-	// change string format to match other version info
-	version := strings.ReplaceAll(string(output), "flux version ", "v")
-
-	return version, nil
+	cliRunner := &runner.CLIRunner{}
+	fluxClient := flux.New(cliRunner)
+	return fluxClient.GetVersion()
 }

pkg/flux/flux.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"fmt"
 	"os"
+	"strings"
 
 	"github.com/pkg/errors"
 	"github.com/weaveworks/weave-gitops/pkg/runner"
@@ -22,6 +23,7 @@ type Flux interface {
 	CreateHelmReleaseGitRepository(name string, source string, path string, namespace string) ([]byte, error)
 	CreateHelmReleaseHelmRepository(name string, chart string, namespace string) ([]byte, error)
 	CreateSecretGit(name string, url string, namespace string) ([]byte, error)
+	GetVersion() (string, error)
 	GetAllResourcesStatus(name string, namespace string) ([]byte, error)
 }
 
@@ -206,6 +208,16 @@ func (f *FluxClient) GetAllResourcesStatus(name string, namespace string) ([]byt
 	return out, nil
 }
 
+func (f *FluxClient) GetVersion() (string, error) {
+	out, err := f.runFluxCmd("-v")
+	if err != nil {
+		return "", err
+	}
+	// change string format to match our versioning standard
+	version := strings.ReplaceAll(string(out), "flux version ", "v")
+	return version, nil
+}
+
 func (f *FluxClient) runFluxCmd(args ...string) ([]byte, error) {
 	fluxPath, err := f.fluxPath()
 	if err != nil {

pkg/flux/logs_test.go

@@ -2,13 +2,14 @@ package flux
 
 import (
 	"fmt"
+	"io/ioutil"
+	"os"
 	"testing"
 
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
 
 	"github.com/stretchr/testify/require"
-	"github.com/weaveworks/weave-gitops/pkg/fluxops"
 	"github.com/weaveworks/weave-gitops/pkg/override"
 	"github.com/weaveworks/weave-gitops/pkg/shims"
 	"github.com/weaveworks/weave-gitops/pkg/utils"
@@ -85,20 +86,12 @@ func processStatus() ([]byte, []byte, error) {
 
 // Test Setup
 
-type localExitHandler struct {
-	action func(int)
-}
-
-func (h localExitHandler) Handle(code int) {
-	h.action(code)
-}
-
 type localHomeDirHandler struct {
-	action func() (string, error)
+	dir string
 }
 
 func (h localHomeDirHandler) Handle() (string, error) {
-	return h.action()
+	return h.dir, nil
 }
 
 func TestSetup(t *testing.T) {
@@ -110,48 +103,26 @@ func TestSetup(t *testing.T) {
 }
 
 func TestSetupFluxBin(t *testing.T) {
-	version.FluxVersion = "0.11.0"
-	SetupFluxBin()
-	homeDir, err := shims.UserHomeDir()
+	dir, err := ioutil.TempDir(t.TempDir(), "a-home-dir")
 	require.NoError(t, err)
-
-	fluxPath := fmt.Sprintf("%v/.wego/bin", homeDir)
-	require.DirExists(t, fluxPath)
-	binPath := fmt.Sprintf("%v/flux-%v", fluxPath, version.FluxVersion)
-	require.FileExists(t, binPath)
-
-	version.FluxVersion = "0.12.0"
-	SetupFluxBin()
-	require.NoFileExists(t, binPath)
-	binPath = fmt.Sprintf("%v/flux-%v", fluxPath, version.FluxVersion)
-	require.FileExists(t, binPath)
+	defer os.RemoveAll(dir)
+	override.WithOverrides(func() override.Result {
+		version.FluxVersion = "0.11.0"
+		SetupFluxBin()
+		homeDir, err := shims.UserHomeDir()
+		require.NoError(t, err)
+
+		fluxPath := fmt.Sprintf("%v/.wego/bin", homeDir)
+		require.DirExists(t, fluxPath)
+		binPath := fmt.Sprintf("%v/flux-%v", fluxPath, version.FluxVersion)
+		require.FileExists(t, binPath)
+
+		version.FluxVersion = "0.12.0"
+		SetupFluxBin()
+		require.NoFileExists(t, binPath)
+		binPath = fmt.Sprintf("%v/flux-%v", fluxPath, version.FluxVersion)
+		require.FileExists(t, binPath)
+		return override.Result{}
+	},
+		shims.OverrideHomeDir(localHomeDirHandler{dir: dir}))
 }
-
-var _ = Describe("Flux Setup Failure", func() {
-	It("Verify that exit is called with expected code", func() {
-		By("Executing a code path that contains checkError", func() {
-			exitCode := -1
-			_ = override.WithOverrides(
-				func() override.Result {
-					checkError(fmt.Errorf("An error"))
-					return override.Result{}
-				},
-				shims.OverrideExit(localExitHandler{action: func(code int) { exitCode = code }}))
-			Expect(exitCode).To(Equal(1))
-		})
-	})
-
-	It("Verify that os.UserHomeDir failures are handled correctly", func() {
-		By("Setting the shim to fail and invoking calls that will trigger it", func() {
-			res := override.WithOverrides(
-				func() override.Result {
-					out, err := fluxops.QuietInstall("flux-system")
-					return override.Result{Output: out, Err: err}
-				},
-				shims.OverrideExit(shims.IgnoreExitHandler{}),
-				shims.OverrideHomeDir(localHomeDirHandler{action: func() (string, error) { return "", fmt.Errorf("failed") }}))
-			Expect(res.Err).To(Not(BeNil()))
-		})
-	})
-
-})