Skip to content

Commit 95f6736

Browse files
authored
fix(librarian): subcommands now process args correctly (#1826)
Fixes: #1819
1 parent aa507b1 commit 95f6736

File tree

3 files changed

+132
-4
lines changed

3 files changed

+132
-4
lines changed

internal/librarian/librarian.go

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -55,12 +55,11 @@ func Run(ctx context.Context, arg ...string) error {
5555
CmdLibrarian.Flags.Usage()
5656
return fmt.Errorf("command not specified")
5757
}
58-
cmd, err := CmdLibrarian.Lookup(arg[0])
58+
cmd, arg, err := lookupCommand(CmdLibrarian, arg)
5959
if err != nil {
60-
CmdLibrarian.Flags.Usage()
6160
return err
6261
}
63-
if err := cmd.Parse(arg[1:]); err != nil {
62+
if err := cmd.Parse(arg); err != nil {
6463
// We expect that if cmd.Parse fails, it will already
6564
// have printed out a command-specific usage error,
6665
// so we don't need to display the general usage.
@@ -76,6 +75,24 @@ func Run(ctx context.Context, arg ...string) error {
7675
return cmd.Run(ctx, cmd.Config)
7776
}
7877

78+
// lookupCommand recursively looks up the command specified by the given arguments.
79+
// It returns the command, the remaining arguments, and an error if the command
80+
// is not found.
81+
func lookupCommand(cmd *cli.Command, args []string) (*cli.Command, []string, error) {
82+
if len(args) == 0 {
83+
return cmd, nil, nil
84+
}
85+
subcommand, err := cmd.Lookup(args[0])
86+
if err != nil {
87+
cmd.Flags.Usage()
88+
return nil, nil, err
89+
}
90+
if len(subcommand.Commands) > 0 {
91+
return lookupCommand(subcommand, args[1:])
92+
}
93+
return subcommand, args[1:], nil
94+
}
95+
7996
// GitHubClient is an abstraction over the GitHub client.
8097
type GitHubClient interface {
8198
GetRawContent(ctx context.Context, path, ref string) ([]byte, error)

internal/librarian/librarian_test.go

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626
"github.com/go-git/go-git/v5"
2727
"github.com/go-git/go-git/v5/plumbing/object"
2828

29+
"github.com/googleapis/librarian/internal/cli"
2930
"github.com/googleapis/librarian/internal/config"
3031
"github.com/googleapis/librarian/internal/gitrepo"
3132
"gopkg.in/yaml.v3"
@@ -239,3 +240,113 @@ type pathAndMessage struct {
239240
path string
240241
message string
241242
}
243+
244+
func TestLookupCommand(t *testing.T) {
245+
sub1sub1 := &cli.Command{
246+
Short: "sub1sub1 does something",
247+
UsageLine: "sub1sub1",
248+
Long: "sub1sub1 does something",
249+
}
250+
sub1 := &cli.Command{
251+
Short: "sub1 does something",
252+
UsageLine: "sub1",
253+
Long: "sub1 does something",
254+
Commands: []*cli.Command{sub1sub1},
255+
}
256+
sub2 := &cli.Command{
257+
Short: "sub2 does something",
258+
UsageLine: "sub2",
259+
Long: "sub2 does something",
260+
}
261+
root := &cli.Command{
262+
Short: "root does something",
263+
UsageLine: "root",
264+
Long: "root does something",
265+
Commands: []*cli.Command{
266+
sub1,
267+
sub2,
268+
},
269+
}
270+
root.Init()
271+
sub1.Init()
272+
sub2.Init()
273+
sub1sub1.Init()
274+
275+
testCases := []struct {
276+
name string
277+
cmd *cli.Command
278+
args []string
279+
wantCmd *cli.Command
280+
wantArgs []string
281+
wantErr bool
282+
}{
283+
{
284+
name: "no args",
285+
cmd: root,
286+
args: []string{},
287+
wantCmd: root,
288+
},
289+
{
290+
name: "find sub1",
291+
cmd: root,
292+
args: []string{"sub1"},
293+
wantCmd: sub1,
294+
},
295+
{
296+
name: "find sub2",
297+
cmd: root,
298+
args: []string{"sub2"},
299+
wantCmd: sub2,
300+
wantArgs: []string{},
301+
},
302+
{
303+
name: "find sub1sub1",
304+
cmd: root,
305+
args: []string{"sub1", "sub1sub1"},
306+
wantCmd: sub1sub1,
307+
wantArgs: []string{},
308+
},
309+
{
310+
name: "find sub1sub1 with args",
311+
cmd: root,
312+
args: []string{"sub1", "sub1sub1", "arg1"},
313+
wantCmd: sub1sub1,
314+
wantArgs: []string{"arg1"},
315+
},
316+
{
317+
name: "unknown command",
318+
cmd: root,
319+
args: []string{"unknown"},
320+
wantErr: true,
321+
},
322+
{
323+
name: "unknown subcommand",
324+
cmd: root,
325+
args: []string{"sub1", "unknown"},
326+
wantErr: true,
327+
},
328+
}
329+
330+
for _, tc := range testCases {
331+
t.Run(tc.name, func(t *testing.T) {
332+
gotCmd, gotArgs, err := lookupCommand(tc.cmd, tc.args)
333+
if (err != nil) != tc.wantErr {
334+
t.Errorf("lookupCommand() error = %v, wantErr %v", err, tc.wantErr)
335+
return
336+
}
337+
if gotCmd != tc.wantCmd {
338+
var gotName, wantName string
339+
if gotCmd != nil {
340+
gotName = gotCmd.Name()
341+
}
342+
if tc.wantCmd != nil {
343+
wantName = tc.wantCmd.Name()
344+
}
345+
t.Errorf("lookupCommand() gotCmd.Name() = %q, want %q", gotName, wantName)
346+
}
347+
if diff := cmp.Diff(tc.wantArgs, gotArgs); diff != "" {
348+
t.Errorf("lookupCommand() args mismatch (-want +got):\n%s", diff)
349+
}
350+
})
351+
}
352+
}

internal/librarian/release.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ var cmdRelease = &cli.Command{
2727

2828
func init() {
2929
cmdRelease.Init()
30-
CmdLibrarian.Commands = append(CmdLibrarian.Commands,
30+
cmdRelease.Commands = append(cmdRelease.Commands,
3131
cmdInit,
3232
cmdTagAndRelease,
3333
)

0 commit comments

Comments
 (0)