Do repo error-checking in AddRepo.Prepare [#26]

internal/helm/plan.go

@@ -4,7 +4,6 @@ import (
 	"fmt"
 	"github.com/pelotech/drone-helm3/internal/run"
 	"os"
-	"strings"
 )
 
 const (
@@ -162,14 +161,8 @@ func initKube(cfg Config) []Step {
 func addRepos(cfg Config) []Step {
 	steps := make([]Step, 0)
 	for _, repo := range cfg.AddRepos {
-		split := strings.SplitN(repo, "=", 2)
-		if len(split) != 2 {
-			fmt.Fprintf(cfg.Stderr, "Warning: skipping bad repo spec '%s'.\n", repo)
-			continue
-		}
 		steps = append(steps, &run.AddRepo{
-			Name: split[0],
+			Repo: repo,
-			URL:  split[1],
 		})
 	}
 

internal/helm/plan_test.go

@@ -293,45 +293,8 @@ func (suite *PlanTestSuite) TestAddRepos() {
 	first := steps[0].(*run.AddRepo)
 	second := steps[1].(*run.AddRepo)
 
-	suite.Equal(first.Name, "first")
+	suite.Equal(first.Repo, "first=https://add.repos/one")
-	suite.Equal(first.URL, "https://add.repos/one")
+	suite.Equal(second.Repo, "second=https://add.repos/two")
-	suite.Equal(second.Name, "second")
-	suite.Equal(second.URL, "https://add.repos/two")
-}
-
-func (suite *PlanTestSuite) TestAddNoRepos() {
-	cfg := Config{
-		AddRepos: []string{},
-	}
-	steps := addRepos(cfg)
-	suite.Equal(0, len(steps), "adding no repos should take zero steps")
-}
-
-func (suite *PlanTestSuite) TestAddReposWithMalformedRepoSpec() {
-	stderr := strings.Builder{}
-	cfg := Config{
-		AddRepos: []string{
-			"dwim",
-		},
-		Stderr: &stderr,
-	}
-	steps := addRepos(cfg)
-	suite.Equal(len(steps), 0)
-	suite.Equal("Warning: skipping bad repo spec 'dwim'.\n", stderr.String())
-}
-
-func (suite *PlanTestSuite) TestAddReposWithEqualsInURL() {
-	cfg := Config{
-		AddRepos: []string{
-			"samaritan=https://github.com/arthur_claypool/samaritan?version=2.1",
-		},
-		Stderr: &strings.Builder{},
-	}
-	steps := addRepos(cfg)
-	suite.Require().Equal(1, len(steps))
-	suite.Require().IsType(&run.AddRepo{}, steps[0])
-	addRepo := steps[0].(*run.AddRepo)
-	suite.Equal("https://github.com/arthur_claypool/samaritan?version=2.1", addRepo.URL)
 }
 
 func (suite *PlanTestSuite) TestLint() {

internal/run/addrepo.go

@@ -2,12 +2,12 @@ package run
 
 import (
 	"fmt"
+	"strings"
 )
 
 // AddRepo is an execution step that calls `helm repo add` when executed.
 type AddRepo struct {
-	Name string
+	Repo string
-	URL  string
 	cmd  cmd
 }
 
@@ -18,13 +18,17 @@ func (a *AddRepo) Execute(_ Config) error {
 
 // Prepare gets the AddRepo ready to execute.
 func (a *AddRepo) Prepare(cfg Config) error {
-	if a.Name == "" {
+	if a.Repo == "" {
-		return fmt.Errorf("repo name is required")
+		return fmt.Errorf("repo is required")
 	}
-	if a.URL == "" {
+	split := strings.SplitN(a.Repo, "=", 2)
-		return fmt.Errorf("repo URL is required")
+	if len(split) != 2 {
+		return fmt.Errorf("bad repo spec '%s'", a.Repo)
 	}
 
+	name := split[0]
+	url := split[1]
+
 	args := make([]string, 0)
 
 	if cfg.Namespace != "" {
@@ -34,7 +38,7 @@ func (a *AddRepo) Prepare(cfg Config) error {
 		args = append(args, "--debug")
 	}
 
-	args = append(args, "repo", "add", a.Name, a.URL)
+	args = append(args, "repo", "add", name, url)
 
 	a.cmd = command(helmBin, args...)
 	a.cmd.Stdout(cfg.Stdout)

internal/run/addrepo_test.go

@@ -46,8 +46,7 @@ func (suite *AddRepoTestSuite) TestPrepareAndExecute() {
 		Stderr: &stderr,
 	}
 	a := AddRepo{
-		Name: "edeath",
+		Repo: "edeath=https://github.com/n_marks/e-death",
-		URL:  "https://github.com/n_marks/e-death",
 	}
 
 	suite.mockCmd.EXPECT().
@@ -58,8 +57,8 @@ func (suite *AddRepoTestSuite) TestPrepareAndExecute() {
 		Times(1)
 
 	suite.Require().NoError(a.Prepare(cfg))
-	suite.Equal(suite.commandPath, helmBin)
+	suite.Equal(helmBin, suite.commandPath)
-	suite.Equal(suite.commandArgs, []string{"repo", "add", "edeath", "https://github.com/n_marks/e-death"})
+	suite.Equal([]string{"repo", "add", "edeath", "https://github.com/n_marks/e-death"}, suite.commandArgs)
 
 	suite.mockCmd.EXPECT().
 		Run().
@@ -69,23 +68,33 @@ func (suite *AddRepoTestSuite) TestPrepareAndExecute() {
 
 }
 
-func (suite *AddRepoTestSuite) TestRequiredFields() {
+func (suite *AddRepoTestSuite) TestPrepareRepoIsRequired() {
 	// These aren't really expected, but allowing them gives clearer test-failure messages
 	suite.mockCmd.EXPECT().Stdout(gomock.Any()).AnyTimes()
 	suite.mockCmd.EXPECT().Stderr(gomock.Any()).AnyTimes()
 	cfg := Config{}
-	a := AddRepo{
+	a := AddRepo{}
-		Name: "idgen",
-	}
 
 	err := a.Prepare(cfg)
-	suite.EqualError(err, "repo URL is required")
+	suite.EqualError(err, "repo is required")
+}
 
-	a.Name = ""
+func (suite *AddRepoTestSuite) TestPrepareMalformedRepo() {
-	a.URL = "https://github.com/n_marks/idgen"
+	a := AddRepo{
+		Repo: "dwim",
+	}
+	err := a.Prepare(Config{})
+	suite.EqualError(err, "bad repo spec 'dwim'")
+}
 
-	err = a.Prepare(cfg)
+func (suite *AddRepoTestSuite) TestPrepareWithEqualSignInURL() {
-	suite.EqualError(err, "repo name is required")
+	suite.mockCmd.EXPECT().Stdout(gomock.Any()).AnyTimes()
+	suite.mockCmd.EXPECT().Stderr(gomock.Any()).AnyTimes()
+	a := AddRepo{
+		Repo: "samaritan=https://github.com/arthur_claypool/samaritan?version=2.1",
+	}
+	suite.NoError(a.Prepare(Config{}))
+	suite.Contains(suite.commandArgs, "https://github.com/arthur_claypool/samaritan?version=2.1")
 }
 
 func (suite *AddRepoTestSuite) TestNamespaceFlag() {
@@ -95,8 +104,7 @@ func (suite *AddRepoTestSuite) TestNamespaceFlag() {
 		Namespace: "alliteration",
 	}
 	a := AddRepo{
-		Name: "edeath",
+		Repo: "edeath=https://github.com/theater_guy/e-death",
-		URL:  "https://github.com/theater_guy/e-death",
 	}
 
 	suite.NoError(a.Prepare(cfg))
@@ -124,8 +132,7 @@ func (suite *AddRepoTestSuite) TestDebugFlag() {
 		Stderr: &stderr,
 	}
 	a := AddRepo{
-		Name: "edeath",
+		Repo: "edeath=https://github.com/the_bug/e-death",
-		URL:  "https://github.com/the_bug/e-death",
 	}
 
 	suite.Require().NoError(a.Prepare(cfg))