diff --git a/internal/cli/pr_checkout_test.go b/internal/cli/pr_checkout_test.go index 6c90862..8d960f6 100644 --- a/internal/cli/pr_checkout_test.go +++ b/internal/cli/pr_checkout_test.go @@ -84,8 +84,9 @@ func (m *mockPRService) ListURL(_ string) string { // mockForge implements forges.Forge for testing. type mockForge struct { - prService *mockPRService - repoService forges.RepoService + prService *mockPRService + repoService forges.RepoService + releaseService forges.ReleaseService } func (m *mockForge) Repos() forges.RepoService { return m.repoService } @@ -93,7 +94,7 @@ func (m *mockForge) Issues() forges.IssueService { return nil } func (m *mockForge) PullRequests() forges.PullRequestService { return m.prService } func (m *mockForge) Labels() forges.LabelService { return nil } func (m *mockForge) Milestones() forges.MilestoneService { return nil } -func (m *mockForge) Releases() forges.ReleaseService { return nil } +func (m *mockForge) Releases() forges.ReleaseService { return m.releaseService } func (m *mockForge) CI() forges.CIService { return nil } func (m *mockForge) Branches() forges.BranchService { return nil } func (m *mockForge) DeployKeys() forges.DeployKeyService { return nil } diff --git a/internal/cli/release.go b/internal/cli/release.go index fb8d678..c10e245 100644 --- a/internal/cli/release.go +++ b/internal/cli/release.go @@ -312,14 +312,41 @@ func releaseDeleteCmd() *cobra.Command { return cmd } +// uploadTagAndFile resolves the release tag and file for "release upload". The tag comes from --tag with the file as +// the only argument, or from the first of two positional arguments. tagChanged must be +// cmd.Flags().Changed("tag"): an explicitly empty --tag is an error, not an unset flag, so an +// unset script variable cannot silently select positional mode. +func uploadTagAndFile(flagTag string, tagChanged bool, args []string) (tag, file string, err error) { + if tagChanged { + if flagTag == "" { + return "", "", fmt.Errorf("--tag value must not be empty") + } + if len(args) != 1 { + return "", "", fmt.Errorf("with --tag, expected one argument, got %d", len(args)) + } + return flagTag, args[0], nil + } + if len(args) != releaseAssetArgs { + return "", "", fmt.Errorf("expected (or --tag ), got %d argument(s)", len(args)) + } + return args[0], args[1], nil +} + func releaseUploadCmd() *cobra.Command { - return &cobra.Command{ - Use: "upload ", + var flagTag string + + cmd := &cobra.Command{ + Use: "upload ( | --tag )", Short: "Upload an asset to a release", - Args: cobra.ExactArgs(releaseAssetArgs), + Args: func(cmd *cobra.Command, args []string) error { + _, _, err := uploadTagAndFile(flagTag, cmd.Flags().Changed("tag"), args) + return err + }, RunE: func(cmd *cobra.Command, args []string) error { - tag := args[0] - filePath := args[1] + tag, filePath, err := uploadTagAndFile(flagTag, cmd.Flags().Changed("tag"), args) + if err != nil { + return err + } forge, owner, repoName, _, err := resolve.Repo(flagRepo, flagForgeType) if err != nil { @@ -349,6 +376,9 @@ func releaseUploadCmd() *cobra.Command { return nil }, } + + cmd.Flags().StringVar(&flagTag, "tag", "", "Tag name of the release (alternative to the positional )") + return cmd } func releaseDownloadCmd() *cobra.Command { diff --git a/internal/cli/release_test.go b/internal/cli/release_test.go index ee682ad..1527cc5 100644 --- a/internal/cli/release_test.go +++ b/internal/cli/release_test.go @@ -1,9 +1,16 @@ package cli import ( + "context" + "io" + "os" + "path/filepath" "testing" "github.com/spf13/cobra" + + "github.com/git-pkgs/forge" + "github.com/git-pkgs/forge/internal/resolve" ) const flagTypeBool = "bool" @@ -68,3 +75,148 @@ func TestReleaseSubcommands(t *testing.T) { } } } + +// mockReleaseService implements forges.ReleaseService, recording UploadAsset calls. +type mockReleaseService struct { + uploadCalls int + uploadTag string + uploadBody []byte +} + +func (m *mockReleaseService) List(_ context.Context, _, _ string, _ forges.ListReleaseOpts) ([]forges.Release, error) { + return nil, nil +} + +func (m *mockReleaseService) Get(_ context.Context, _, _, _ string) (*forges.Release, error) { + return nil, nil +} + +func (m *mockReleaseService) GetLatest(_ context.Context, _, _ string) (*forges.Release, error) { + return nil, nil +} + +func (m *mockReleaseService) Create(_ context.Context, _, _ string, _ forges.CreateReleaseOpts) (*forges.Release, error) { + return nil, nil +} + +func (m *mockReleaseService) Update(_ context.Context, _, _, _ string, _ forges.UpdateReleaseOpts) (*forges.Release, error) { + return nil, nil +} + +func (m *mockReleaseService) Delete(_ context.Context, _, _, _ string) error { + return nil +} + +func (m *mockReleaseService) UploadAsset(_ context.Context, _, _, tag string, file *os.File) (*forges.ReleaseAsset, error) { + m.uploadCalls++ + m.uploadTag = tag + body, err := io.ReadAll(file) + if err != nil { + return nil, err + } + m.uploadBody = body + return &forges.ReleaseAsset{Name: filepath.Base(file.Name()), Size: len(body)}, nil +} + +func (m *mockReleaseService) DownloadAsset(_ context.Context, _, _ string, _ int64) (io.ReadCloser, error) { + return nil, nil +} + +func TestReleaseUploadCommand(t *testing.T) { + const contents = "fake-zip-bytes" + + tests := []struct { + name string + args []string + wantTag string + wantErr bool + }{ + {name: "positional tag and file", args: []string{"v1.0.0", "{file}"}, wantTag: "v1.0.0"}, + {name: "tag flag and file", args: []string{"--tag", "v1.0.0", "{file}"}, wantTag: "v1.0.0"}, + {name: "tag flag with positional tag too", args: []string{"--tag", "v1.0.0", "v1.0.0", "{file}"}, wantErr: true}, + {name: "empty tag flag with positional tag", args: []string{"--tag", "", "v1.0.0", "{file}"}, wantErr: true}, + {name: "empty tag flag without positional tag", args: []string{"--tag", "", "{file}"}, wantErr: true}, + {name: "no tag at all", args: []string{"{file}"}, wantErr: true}, + {name: "tag flag without a file", args: []string{"--tag", "v1.0.0"}, wantErr: true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + file := filepath.Join(t.TempDir(), "app.zip") + if err := os.WriteFile(file, []byte(contents), 0644); err != nil { + t.Fatalf("writing test file: %v", err) + } + args := make([]string, len(tt.args)) + for i, a := range tt.args { + if a == "{file}" { + a = file + } + args[i] = a + } + + releases := &mockReleaseService{} + resolve.SetTestForge(&mockForge{releaseService: releases}, "owner", "repo", "example.com") + t.Cleanup(resolve.ResetTestForge) + + cmd := releaseUploadCmd() + cmd.SetArgs(args) + err := cmd.Execute() + if tt.wantErr { + if err == nil { + t.Fatalf("expected error, got success (tag %q uploaded)", releases.uploadTag) + } + if releases.uploadCalls != 0 { + t.Errorf("UploadAsset calls = %d, want 0", releases.uploadCalls) + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if releases.uploadCalls != 1 { + t.Fatalf("UploadAsset calls = %d, want 1", releases.uploadCalls) + } + if releases.uploadTag != tt.wantTag { + t.Errorf("uploaded tag = %q, want %q", releases.uploadTag, tt.wantTag) + } + if string(releases.uploadBody) != contents { + t.Errorf("uploaded body = %q, want %q", releases.uploadBody, contents) + } + }) + } +} + +func TestUploadTagAndFile(t *testing.T) { + tests := []struct { + name string + flagTag string + tagChanged bool + args []string + wantTag string + wantFile string + wantErr bool + }{ + {name: "positional tag and file", args: []string{"v1.0.0", "app.zip"}, wantTag: "v1.0.0", wantFile: "app.zip"}, + {name: "tag flag and file", flagTag: "v1.0.0", tagChanged: true, args: []string{"app.zip"}, wantTag: "v1.0.0", wantFile: "app.zip"}, + {name: "tag flag with positional tag too", flagTag: "v1.0.0", tagChanged: true, args: []string{"v1.0.0", "app.zip"}, wantErr: true}, + {name: "empty tag flag with positional tag", tagChanged: true, args: []string{"v1.0.0", "app.zip"}, wantErr: true}, + {name: "empty tag flag without positional tag", tagChanged: true, args: []string{"app.zip"}, wantErr: true}, + {name: "no tag at all", args: []string{"app.zip"}, wantErr: true}, + {name: "no arguments", wantErr: true}, + {name: "tag flag without a file", flagTag: "v1.0.0", tagChanged: true, wantErr: true}, + {name: "too many positional arguments", args: []string{"v1.0.0", "a.zip", "b.zip"}, wantErr: true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tag, file, err := uploadTagAndFile(tt.flagTag, tt.tagChanged, tt.args) + if (err != nil) != tt.wantErr { + t.Fatalf("uploadTagAndFile(%q, %v, %v) error = %v, wantErr %v", tt.flagTag, tt.tagChanged, tt.args, err, tt.wantErr) + } + if tag != tt.wantTag || file != tt.wantFile { + t.Errorf("uploadTagAndFile(%q, %v, %v) = (%q, %q), want (%q, %q)", + tt.flagTag, tt.tagChanged, tt.args, tag, file, tt.wantTag, tt.wantFile) + } + }) + } +}