diff --git a/cli/cmd/args_test.go b/cli/cmd/args_test.go new file mode 100644 index 00000000..4dd5fa9b --- /dev/null +++ b/cli/cmd/args_test.go @@ -0,0 +1,106 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package cmd + +import ( + "bytes" + "testing" + + "github.com/spf13/cobra" + "github.com/spf13/pflag" +) + +// execute drives the CLI through rootCmd.Execute. Calling ValidateArgs +// directly is not enough: it passes on the non-runnable plugin parent +// while the real CLI exits 0 (#345). +func execute(t *testing.T, args ...string) error { + t.Helper() + out := &bytes.Buffer{} + rootCmd.SetOut(out) + rootCmd.SetErr(out) + rootCmd.SetArgs(args) + err := rootCmd.Execute() + resetFlags(rootCmd) + return err +} + +// Flag values persist across Execute calls, so reset them between cases. +func resetFlags(cmd *cobra.Command) { + cmd.Flags().Visit(func(f *pflag.Flag) { + _ = f.Value.Set(f.DefValue) + f.Changed = false + }) + for _, sub := range cmd.Commands() { + resetFlags(sub) + } +} + +func TestArgumentValidation(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + + tests := []struct { + name string + args []string + wantErr bool + }{ + // The five reproductions from #345. + {name: "plugin unknown subcommand", args: []string{"plugin", "bogus"}, wantErr: true}, + {name: "plugin help routed as args", args: []string{"plugin", "help", "list"}, wantErr: true}, + {name: "plugin install extra args", args: []string{"plugin", "install", "a", "b", "c"}, wantErr: true}, + {name: "plugin install no name no --all", args: []string{"plugin", "install"}, wantErr: true}, + {name: "convert trailing arg", args: []string{"convert", "--from", "x", "--input", "y", "extra"}, wantErr: true}, + {name: "plugin bare", args: []string{"plugin"}, wantErr: true}, + {name: "plugin install --all with name", args: []string{"plugin", "install", "--all", "foo"}, wantErr: true}, + // Valid invocations keep working. + {name: "plugin list", args: []string{"plugin", "list"}}, + {name: "plugin install by name", args: []string{"plugin", "install", "foo"}}, + {name: "plugin install --all", args: []string{"plugin", "install", "--all"}}, + {name: "convert flags only", args: []string{"convert", "--from", "x", "--input", "y"}}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := execute(t, tt.args...) + if (err != nil) != tt.wantErr { + t.Fatalf("Execute(%q) error = %v, wantErr %v", tt.args, err, tt.wantErr) + } + }) + } +} + +// Every command needs an Args validator, and every parent needs to be +// runnable, or cobra silently accepts arbitrary arguments (#345). +func TestEveryCommandValidatesArgs(t *testing.T) { + var walk func(cmd *cobra.Command) + walk = func(cmd *cobra.Command) { + for _, sub := range cmd.Commands() { + if sub.Name() == "help" || sub.Name() == "completion" { + continue + } + if sub.HasSubCommands() { + if !sub.Runnable() { + t.Errorf("%s: parent is not runnable, unknown subcommands would exit 0", sub.CommandPath()) + } + walk(sub) + } + if sub.Args == nil { + t.Errorf("%s: no Args validator", sub.CommandPath()) + } + } + } + walk(rootCmd) +} diff --git a/cli/cmd/convert.go b/cli/cmd/convert.go index 1d0734fc..1d20d78f 100644 --- a/cli/cmd/convert.go +++ b/cli/cmd/convert.go @@ -25,6 +25,7 @@ import ( var convertCmd = &cobra.Command{ Use: "convert --from --input | --to --input ", Short: "Convert a semantic model between Ossie and a platform format", + Args: cobra.NoArgs, RunE: runConvert, } diff --git a/cli/cmd/plugin/install.go b/cli/cmd/plugin/install.go index 7a77f235..38476898 100644 --- a/cli/cmd/plugin/install.go +++ b/cli/cmd/plugin/install.go @@ -17,6 +17,7 @@ package plugin import ( + "errors" "fmt" "github.com/spf13/cobra" @@ -25,6 +26,7 @@ import ( var installCmd = &cobra.Command{ Use: "install [name[@version] | url]", Short: "Install a plugin from the registry or a URL", + Args: cobra.MaximumNArgs(1), RunE: runPluginInstall, } @@ -33,6 +35,17 @@ func init() { } func runPluginInstall(cmd *cobra.Command, args []string) error { + all, err := cmd.Flags().GetBool("all") + if err != nil { + return err + } + hasName := len(args) == 1 + if all && hasName { + return errors.New("--all cannot be combined with a plugin name") + } + if !all && !hasName { + return errors.New("requires a plugin name or --all") + } fmt.Fprintln(cmd.OutOrStdout(), "not yet implemented") return nil } diff --git a/cli/cmd/plugin/plugin.go b/cli/cmd/plugin/plugin.go index b1f217bc..647e0c82 100644 --- a/cli/cmd/plugin/plugin.go +++ b/cli/cmd/plugin/plugin.go @@ -16,13 +16,25 @@ package plugin -import "github.com/spf13/cobra" +import ( + "errors" + + "github.com/spf13/cobra" +) // Cmd is the parent "ossie plugin" command. It is exported so cmd/root.go can -// register it. Invoking it bare prints help. +// register it. +// +// The RunE matters: cobra never reaches ValidateArgs on a non-runnable +// command, so without it a bare "ossie plugin" or an unknown subcommand +// prints help and exits 0 (#345). var Cmd = &cobra.Command{ Use: "plugin", Short: "Manage Ossie plugins", + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, args []string) error { + return errors.New("a subcommand is required") + }, } func init() {