From 49aa858b89934b380ad1c7826cd97373d0229dc0 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Thu, 22 Dec 2022 16:19:38 +0100 Subject: [PATCH] Run command must always take a single argument (#156) --- bundle/run/runner.go | 55 +++++++++-------------------- bundle/run/runner_test.go | 74 ++++++--------------------------------- cmd/bundle/run.go | 13 ++++--- 3 files changed, 33 insertions(+), 109 deletions(-) diff --git a/bundle/run/runner.go b/bundle/run/runner.go index f97f19e0..e9c1aadd 100644 --- a/bundle/run/runner.go +++ b/bundle/run/runner.go @@ -24,55 +24,34 @@ type Runner interface { Run(ctx context.Context) error } -// Collect collects a list of runners given a list of arguments. +// Find locates a runner matching the specified argument. // // Its behavior is as follows: -// 1. If no arguments are specified, it returns a runner for the only resource in the bundle. -// 2. If multiple arguments are specified, for each argument: -// 2.1. Try to find a resource with identical to the argument. -// 2.2. Try to find a resource with . identical to the argument. +// 1. Try to find a resource with identical to the argument. +// 2. Try to find a resource with . identical to the argument. // // If an argument resolves to multiple resources, it returns an error. -func Collect(b *bundle.Bundle, args []string) ([]Runner, error) { +func Find(b *bundle.Bundle, arg string) (Runner, error) { keyOnly, keyWithType := ResourceKeys(b) if len(keyWithType) == 0 { return nil, fmt.Errorf("bundle defines no resources") } - var out []Runner - - // If the bundle contains only a single resource, we know what to run. - if len(args) == 0 { - if len(keyWithType) != 1 { - return nil, fmt.Errorf("bundle defines multiple resources; please specify resource to run") - } - for _, runners := range keyWithType { - if len(runners) != 1 { - // This invariant is covered by [ResourceKeys]. - panic("length of []run.Runner must be 1") - } - out = append(out, runners[0]) - } - return out, nil - } - - for _, arg := range args { - runners, ok := keyOnly[arg] + runners, ok := keyOnly[arg] + if !ok { + runners, ok = keyWithType[arg] if !ok { - runners, ok = keyWithType[arg] - if !ok { - return nil, fmt.Errorf("no such resource: %s", arg) - } + return nil, fmt.Errorf("no such resource: %s", arg) } - if len(runners) != 1 { - var keys []string - for _, runner := range runners { - keys = append(keys, runner.Key()) - } - return nil, fmt.Errorf("ambiguous: %s (can resolve to all of %s)", arg, strings.Join(keys, ", ")) - } - out = append(out, runners[0]) } - return out, nil + if len(runners) != 1 { + var keys []string + for _, runner := range runners { + keys = append(keys, runner.Key()) + } + return nil, fmt.Errorf("ambiguous: %s (can resolve to all of %s)", arg, strings.Join(keys, ", ")) + } + + return runners[0], nil } diff --git a/bundle/run/runner_test.go b/bundle/run/runner_test.go index e284d979..da2f59d6 100644 --- a/bundle/run/runner_test.go +++ b/bundle/run/runner_test.go @@ -9,18 +9,18 @@ import ( "github.com/stretchr/testify/assert" ) -func TestCollectNoResources(t *testing.T) { +func TestFindNoResources(t *testing.T) { b := &bundle.Bundle{ Config: config.Root{ Resources: config.Resources{}, }, } - _, err := Collect(b, []string{"foo"}) + _, err := Find(b, "foo") assert.ErrorContains(t, err, "bundle defines no resources") } -func TestCollectNoArg(t *testing.T) { +func TestFindSingleArg(t *testing.T) { b := &bundle.Bundle{ Config: config.Root{ Resources: config.Resources{ @@ -31,28 +31,11 @@ func TestCollectNoArg(t *testing.T) { }, } - out, err := Collect(b, []string{}) + _, err := Find(b, "foo") assert.NoError(t, err) - assert.Len(t, out, 1) } -func TestCollectNoArgMultipleResources(t *testing.T) { - b := &bundle.Bundle{ - Config: config.Root{ - Resources: config.Resources{ - Jobs: map[string]*resources.Job{ - "foo": {}, - "bar": {}, - }, - }, - }, - } - - _, err := Collect(b, []string{}) - assert.ErrorContains(t, err, "bundle defines multiple resources") -} - -func TestCollectSingleArg(t *testing.T) { +func TestFindSingleArgNotFound(t *testing.T) { b := &bundle.Bundle{ Config: config.Root{ Resources: config.Resources{ @@ -63,27 +46,11 @@ func TestCollectSingleArg(t *testing.T) { }, } - out, err := Collect(b, []string{"foo"}) - assert.NoError(t, err) - assert.Len(t, out, 1) -} - -func TestCollectSingleArgNotFound(t *testing.T) { - b := &bundle.Bundle{ - Config: config.Root{ - Resources: config.Resources{ - Jobs: map[string]*resources.Job{ - "foo": {}, - }, - }, - }, - } - - _, err := Collect(b, []string{"bar"}) + _, err := Find(b, "bar") assert.ErrorContains(t, err, "no such resource: bar") } -func TestCollectSingleArgAmbiguous(t *testing.T) { +func TestFindSingleArgAmbiguous(t *testing.T) { b := &bundle.Bundle{ Config: config.Root{ Resources: config.Resources{ @@ -97,11 +64,11 @@ func TestCollectSingleArgAmbiguous(t *testing.T) { }, } - _, err := Collect(b, []string{"key"}) + _, err := Find(b, "key") assert.ErrorContains(t, err, "ambiguous: ") } -func TestCollectSingleArgWithType(t *testing.T) { +func TestFindSingleArgWithType(t *testing.T) { b := &bundle.Bundle{ Config: config.Root{ Resources: config.Resources{ @@ -112,27 +79,6 @@ func TestCollectSingleArgWithType(t *testing.T) { }, } - out, err := Collect(b, []string{"jobs.key"}) + _, err := Find(b, "jobs.key") assert.NoError(t, err) - assert.Len(t, out, 1) -} - -func TestCollectMultipleArg(t *testing.T) { - b := &bundle.Bundle{ - Config: config.Root{ - Resources: config.Resources{ - Jobs: map[string]*resources.Job{ - "foo": {}, - "bar": {}, - }, - Pipelines: map[string]*resources.Pipeline{ - "qux": {}, - }, - }, - }, - } - - out, err := Collect(b, []string{"foo", "bar", "qux"}) - assert.NoError(t, err) - assert.Len(t, out, 3) } diff --git a/cmd/bundle/run.go b/cmd/bundle/run.go index 0185e904..93ba1823 100644 --- a/cmd/bundle/run.go +++ b/cmd/bundle/run.go @@ -9,9 +9,10 @@ import ( ) var runCmd = &cobra.Command{ - Use: "run [flags] KEY...", + Use: "run [flags] KEY", Short: "Run a workload (e.g. a job or a pipeline)", + Args: cobra.ExactArgs(1), PreRunE: ConfigureBundle, RunE: func(cmd *cobra.Command, args []string) error { b := bundle.Get(cmd.Context()) @@ -24,16 +25,14 @@ var runCmd = &cobra.Command{ return err } - runners, err := run.Collect(b, args) + runner, err := run.Find(b, args[0]) if err != nil { return err } - for _, runner := range runners { - err = runner.Run(cmd.Context()) - if err != nil { - return err - } + err = runner.Run(cmd.Context()) + if err != nil { + return err } return nil