Add --tag to release upload for parity with release create

internal/cli/pr_checkout_test.go

@@ -84,16 +84,17 @@ 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 }
 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 }

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 <file> argument, got %d", len(args))
+		}
+		return flagTag, args[0], nil
+	}
+	if len(args) != releaseAssetArgs {
+		return "", "", fmt.Errorf("expected <tag> <file> (or --tag <tag> <file>), got %d argument(s)", len(args))
+	}
+	return args[0], args[1], nil
+}
+
 func releaseUploadCmd() *cobra.Command {
-	return &cobra.Command{
-		Use:   "upload <tag> <file>",
+	var flagTag string
+
+	cmd := &cobra.Command{
+		Use:   "upload (<tag> <file> | --tag <tag> <file>)",
 		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 <tag>)")
+	return cmd
 }
 
 func releaseDownloadCmd() *cobra.Command {

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)
+			}
+		})
+	}
+}