-
Notifications
You must be signed in to change notification settings - Fork 365
fix: disallow --name flag when adding multiple workflows at once #28195
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -337,3 +337,15 @@ func TestAddCommandArgs(t *testing.T) { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| err = cmd.Args(cmd, []string{"workflow1", "workflow2"}) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| require.NoError(t, err, "Should not error with multiple arguments") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // TestAddMultipleWorkflowsNameFlag verifies that --name is not allowed when multiple workflows are specified. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| func TestAddMultipleWorkflowsNameFlag(t *testing.T) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| cmd := NewAddCommand(validateEngineStub) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Simulate calling the command with --name and multiple workflow arguments | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| cmd.SetArgs([]string{"workflow1", "workflow2", "--name", "custom-name"}) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| err := cmd.Execute() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| require.Error(t, err, "Should error when --name is used with multiple workflows") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| assert.Contains(t, err.Error(), "--name flag cannot be used when adding multiple workflows", "Error should mention --name restriction") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+343
to
+350
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| cmd := NewAddCommand(validateEngineStub) | |
| // Simulate calling the command with --name and multiple workflow arguments | |
| cmd.SetArgs([]string{"workflow1", "workflow2", "--name", "custom-name"}) | |
| err := cmd.Execute() | |
| require.Error(t, err, "Should error when --name is used with multiple workflows") | |
| assert.Contains(t, err.Error(), "--name flag cannot be used when adding multiple workflows", "Error should mention --name restriction") | |
| tests := []struct { | |
| name string | |
| args []string | |
| }{ | |
| { | |
| name: "non-empty name with multiple workflow arguments", | |
| args: []string{"workflow1", "workflow2", "--name", "custom-name"}, | |
| }, | |
| { | |
| name: "explicitly empty name with multiple workflow arguments", | |
| args: []string{"workflow1", "workflow2", "--name="}, | |
| }, | |
| } | |
| for _, tt := range tests { | |
| t.Run(tt.name, func(t *testing.T) { | |
| cmd := NewAddCommand(validateEngineStub) | |
| // Simulate calling the command with --name and multiple workflow arguments | |
| cmd.SetArgs(tt.args) | |
| err := cmd.Execute() | |
| require.Error(t, err, "Should error when --name is used with multiple workflows") | |
| assert.Contains(t, err.Error(), "--name flag cannot be used when adding multiple workflows", "Error should mention --name restriction") | |
| }) | |
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The validation only triggers when
nameFlag != ""andlen(args) > 1, which still allows using--namein cases where a single argument resolves to multiple workflows (e.g., local wildcard patterns) and also allows--name=""since the value remains empty. To fully enforce “flag cannot be used when adding multiple workflows”, consider checkingcmd.Flags().Changed("name")and validating against the number of resolved workflows (or pre-expanding local wildcards) rather than onlylen(args)and a non-empty string value.