SUP-2130 Move pipeline positional argument resolver to resolvers pack…

…age (#228)

* Move positional pipeline resolver to resolver package

* Update build command output to error if doesn't resolve pipeline

internal/io/pending_command.go

@@ -12,10 +12,11 @@ type PendingOutput string
 // Pending is used to show a loading spinner while a long running function runs to perform some action and output
 // information
 type Pending struct {
-	spinner  spinner.Model
-	quitting bool
+	Err      error
 	fn       tea.Cmd
 	output   string
+	quitting bool
+	spinner  spinner.Model
 }
 
 // Init implements tea.Model.
@@ -46,6 +47,7 @@ func (p Pending) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
 	case error: // an error occurred somewhere. show the error and exit
 		p.quitting = true
 		p.output = "Error: " + msg.Error()
+		p.Err = msg
 		return p, tea.Quit
 	default: // start/update the spinner
 		var cmd tea.Cmd

internal/pipeline/resolver/cli.go

@@ -0,0 +1,66 @@
+package resolver
+
+import (
+	"fmt"
+	"net/url"
+	"strings"
+
+	"github.com/buildkite/cli/v3/internal/config"
+	"github.com/buildkite/cli/v3/internal/pipeline"
+)
+
+func ResolveFromPositionalArgument(args []string, index int, conf *config.Config) PipelineResolverFn {
+	return func() (*pipeline.Pipeline, error) {
+		// if args does not have values, skip this resolver
+		if len(args) < 1 {
+			return nil, nil
+		}
+		// if the index is out of bounds
+		if len(args) < index {
+			return nil, nil
+		}
+
+		org, name := parsePipelineArg(args[index], conf)
+
+		// if we get here, we should be able to parse the value and return an error if not
+		// this is because a user has explicitly given an input value for us to use - we shoulnt ignore it on error
+		if org == "" || name == "" {
+			return nil, fmt.Errorf("Not able to parse the input pipeline argument: \"%s\"", args[index])
+		}
+
+		return &pipeline.Pipeline{Name: name, Org: org}, nil
+	}
+}
+
+// parsePipelineArg resolve an input string in varying formats to an organization and pipeline pair
+// some example input formats are:
+// - a web URL: https://buildkite.com/<org>/<pipeline slug>/builds/...
+// - a slug: <org>/<pipeline slug>
+// - a pipeline slug by itself
+func parsePipelineArg(arg string, conf *config.Config) (org, pipeline string) {
+	pipelineIsURL := strings.Contains(arg, ":")
+	pipelineIsSlug := !pipelineIsURL && strings.Contains(arg, "/")
+
+	if pipelineIsURL {
+		url, err := url.Parse(arg)
+		if err != nil {
+			return "", ""
+		}
+		// eg: url.Path = /buildkite/buildkite-cli
+		part := strings.Split(url.Path, "/")
+		if len(part) < 3 {
+			return "", ""
+		}
+		org, pipeline = part[1], part[2]
+	} else if pipelineIsSlug {
+		part := strings.Split(arg, "/")
+		if len(part) < 2 {
+			return "", ""
+		}
+		org, pipeline = part[0], part[1]
+	} else {
+		org = conf.Organization
+		pipeline = arg
+	}
+	return org, pipeline
+}

pkg/cmd/build/build_test.go → internal/pipeline/resolver/cli_test.go

@@ -1,9 +1,10 @@
-package build
+package resolver_test
 
 import (
 	"testing"
 
 	"github.com/buildkite/cli/v3/internal/config"
+	"github.com/buildkite/cli/v3/internal/pipeline/resolver"
 )
 
 func TestParsePipelineArg(t *testing.T) {
@@ -37,13 +38,33 @@ func TestParsePipelineArg(t *testing.T) {
 			c := config.Config{
 				Organization: "testing",
 			}
-			org, agent := parsePipelineArg(testcase.url, &c)
-			if org != testcase.org {
+			f := resolver.ResolveFromPositionalArgument([]string{testcase.url}, 0, &c)
+			pipeline, err := f()
+			if err != nil {
+				t.Error(err)
+			}
+			if pipeline.Org != testcase.org {
 				t.Error("parsed organization slug did not match expected")
 			}
-			if agent != testcase.pipeline {
+			if pipeline.Name != testcase.pipeline {
 				t.Error("parsed pipeline name did not match expected")
 			}
 		})
 	}
+
+	t.Run("Returns error if failed parsing", func(t *testing.T) {
+		t.Parallel()
+
+		c := config.Config{
+			Organization: "testing",
+		}
+		f := resolver.ResolveFromPositionalArgument([]string{"https://buildkite.com/"}, 0, &c)
+		pipeline, err := f()
+		if err == nil {
+			t.Error("Should have failed parsing pipeline")
+		}
+		if pipeline != nil {
+			t.Error("No pipeline should be returned")
+		}
+	})
 }

pkg/cmd/build/build.go

@@ -2,11 +2,8 @@ package build
 
 import (
 	"fmt"
-	"net/url"
-	"strings"
 
 	"github.com/MakeNowJust/heredoc"
-	"github.com/buildkite/cli/v3/internal/config"
 	"github.com/buildkite/cli/v3/pkg/cmd/factory"
 	"github.com/buildkite/cli/v3/pkg/cmd/validation"
 	"github.com/charmbracelet/lipgloss"
@@ -21,14 +18,14 @@ func NewCmdBuild(f *factory.Factory) *cobra.Command {
 		Long:  "Work with Buildkite pipeline builds.",
 		Example: heredoc.Doc(`
 			# To create a new build
-			$ bk build new -m "Build from cli" -c "HEAD" -b "main" 
+			$ bk build new -m "Build from cli" -c "HEAD" -b "main"
 		`),
 		PersistentPreRunE: validation.CheckValidConfiguration(f.Config),
 		Annotations: map[string]string{
 			"help:arguments": heredoc.Doc(`
 				A pipeline is passed as an argument. It can be supplied in any of the following formats:
 				- "PIPELINE_SLUG"
-				- "ORGANIZATION_SLUG/PIPELINE_SLUG" 
+				- "ORGANIZATION_SLUG/PIPELINE_SLUG"
 			`),
 		},
 	}
@@ -41,29 +38,6 @@ func NewCmdBuild(f *factory.Factory) *cobra.Command {
 	return &cmd
 }
 
-func parsePipelineArg(arg string, conf *config.Config) (string, string) {
-	var org, pipeline string
-	pipelineIsURL := strings.Contains(arg, ":")
-	pipelineIsSlug := !pipelineIsURL && strings.Contains(arg, "/")
-
-	if pipelineIsURL {
-		url, err := url.Parse(arg)
-		if err != nil {
-			return "", ""
-		}
-		// eg: url.Path = /buildkite/buildkite-cli
-		part := strings.Split(url.Path, "/")
-		org, pipeline = part[1], part[2]
-	} else if pipelineIsSlug {
-		part := strings.Split(arg, "/")
-		org, pipeline = part[0], part[1]
-	} else {
-		org = conf.Organization
-		pipeline = arg
-	}
-	return org, pipeline
-}
-
 func openBuildInBrowser(openInWeb bool, webUrl string) error {
 
 	if openInWeb {

pkg/cmd/build/cancel.go

@@ -30,7 +30,7 @@ func NewCmdBuildCancel(f *factory.Factory) *cobra.Command {
 		RunE: func(cmd *cobra.Command, args []string) error {
 			buildId := args[0]
 			resolvers := resolver.NewAggregateResolver(
-				pipelineResolverPositionArg(args[1:], f.Config),
+				resolver.ResolveFromPositionalArgument(args, 1, f.Config),
 				resolver.ResolveFromPath("", f.Config.Organization, f.RestAPIClient),
 			)
 			var pipeline pipeline.Pipeline
@@ -44,10 +44,13 @@ func NewCmdBuildCancel(f *factory.Factory) *cobra.Command {
 				return io.PendingOutput(fmt.Sprintf("Resolved pipeline to: %s", pipeline.Name))
 			}, "Resolving pipeline")
 			p := tea.NewProgram(r)
-			_, err := p.Run()
+			finalModel, err := p.Run()
 			if err != nil {
 				return err
 			}
+			if finalModel.(io.Pending).Err != nil {
+				return finalModel.(io.Pending).Err
+			}
 			return cancelBuild(pipeline.Org, pipeline.Name, buildId, web, f)
 		},
 	}

pkg/cmd/build/new.go

@@ -4,7 +4,6 @@ import (
 	"fmt"
 
 	"github.com/MakeNowJust/heredoc"
-	"github.com/buildkite/cli/v3/internal/config"
 	"github.com/buildkite/cli/v3/internal/io"
 	"github.com/buildkite/cli/v3/internal/pipeline"
 	"github.com/buildkite/cli/v3/internal/pipeline/resolver"
@@ -34,7 +33,7 @@ func NewCmdBuildNew(f *factory.Factory) *cobra.Command {
 		`),
 		RunE: func(cmd *cobra.Command, args []string) error {
 			resolvers := resolver.NewAggregateResolver(
-				pipelineResolverPositionArg(args, f.Config),
+				resolver.ResolveFromPositionalArgument(args, 0, f.Config),
 				resolver.ResolveFromPath("", f.Config.Organization, f.RestAPIClient),
 			)
 			var pipeline pipeline.Pipeline
@@ -48,10 +47,13 @@ func NewCmdBuildNew(f *factory.Factory) *cobra.Command {
 				return io.PendingOutput(fmt.Sprintf("Resolved pipeline to: %s", pipeline.Name))
 			}, "Resolving pipeline")
 			p := tea.NewProgram(r)
-			_, err := p.Run()
+			finalModel, err := p.Run()
 			if err != nil {
 				return err
 			}
+			if finalModel.(io.Pending).Err != nil {
+				return finalModel.(io.Pending).Err
+			}
 			return newBuild(pipeline.Org, pipeline.Name, f, message, commit, branch, web)
 		},
 	}
@@ -64,24 +66,6 @@ func NewCmdBuildNew(f *factory.Factory) *cobra.Command {
 	return &cmd
 }
 
-func pipelineResolverPositionArg(args []string, conf *config.Config) resolver.PipelineResolverFn {
-	return func() (*pipeline.Pipeline, error) {
-		// if args does not have values, skip this resolver
-		if len(args) < 1 {
-			return nil, nil
-		}
-
-		org, name := parsePipelineArg(args[0], conf)
-		// if we could not parse the pipeline from the arg then return no pipeline or error, to pass indicate to pass to
-		// the next resolver in the chain
-		if org == "" || name == "" {
-			return nil, nil
-		}
-
-		return &pipeline.Pipeline{Name: name, Org: org}, nil
-	}
-}
-
 func newBuild(org string, pipeline string, f *factory.Factory, message string, commit string, branch string, web bool) error {
 	l := io.NewPendingCommand(func() tea.Msg {
 

pkg/cmd/build/rebuild.go

@@ -30,7 +30,7 @@ func NewCmdBuildRebuild(f *factory.Factory) *cobra.Command {
 		RunE: func(cmd *cobra.Command, args []string) error {
 			buildId := args[0]
 			resolvers := resolver.NewAggregateResolver(
-				pipelineResolverPositionArg(args[1:], f.Config),
+				resolver.ResolveFromPositionalArgument(args, 1, f.Config),
 				resolver.ResolveFromPath("", f.Config.Organization, f.RestAPIClient),
 			)
 			var pipeline pipeline.Pipeline
@@ -44,10 +44,13 @@ func NewCmdBuildRebuild(f *factory.Factory) *cobra.Command {
 				return io.PendingOutput(fmt.Sprintf("Resolved pipeline to: %s", pipeline.Name))
 			}, "Resolving pipeline")
 			p := tea.NewProgram(r)
-			_, err := p.Run()
+			finalModel, err := p.Run()
 			if err != nil {
 				return err
 			}
+			if finalModel.(io.Pending).Err != nil {
+				return finalModel.(io.Pending).Err
+			}
 			return rebuild(pipeline.Org, pipeline.Name, buildId, web, f)
 		},
 	}

pkg/cmd/build/view.go

@@ -39,7 +39,7 @@ func NewCmdBuildView(f *factory.Factory) *cobra.Command {
 			var buildAnnotations = make([]buildkite.Annotation, 0)
 			buildId := args[0]
 			resolvers := resolver.NewAggregateResolver(
-				pipelineResolverPositionArg(args[1:], f.Config),
+				resolver.ResolveFromPositionalArgument(args, 1, f.Config),
 				resolver.ResolveFromPath("", f.Config.Organization, f.RestAPIClient),
 			)
 
@@ -55,10 +55,13 @@ func NewCmdBuildView(f *factory.Factory) *cobra.Command {
 				return io.PendingOutput(fmt.Sprintf("Resolved pipeline to: %s", pipeline.Name))
 			}, "Resolving pipeline")
 			p := tea.NewProgram(r)
-			_, err := p.Run()
+			finalModel, err := p.Run()
 			if err != nil {
 				return err
 			}
+			if finalModel.(io.Pending).Err != nil {
+				return finalModel.(io.Pending).Err
+			}
 
 			l := io.NewPendingCommand(func() tea.Msg {
 				var buildUrl string